src: fix bad logic in uid/gid checks

Pointed out by Coverity.  Introduced in commits 3546383c ("process_wrap:
avoid leaking memory when throwing due to invalid arguments") and
fa4eb47c ("bindings: add spawn_sync bindings").

The return statements inside the if blocks were dead code because their
guard conditions always evaluated to false.  Remove them.

PR-URL: https://github.com/nodejs/node/pull/7374
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

1 files changed, 7 insertions, 26 deletions

diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc
index 2d6012761c8..79f10a0ea25 100644
--- a/src/spawn_sync.cc
+++ b/src/spawn_sync.cc
@@ -729,17 +729,19 @@ int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
   }
   Local<Value> js_uid = js_options->Get(env()->uid_string());
   if (IsSet(js_uid)) {
-    if (!CheckRange<uv_uid_t>(js_uid))
+    if (!js_uid->IsInt32())
       return UV_EINVAL;
-    uv_process_options_.uid = static_cast<uv_gid_t>(js_uid->Int32Value());
+    const int32_t uid = js_uid->Int32Value(env()->context()).FromJust();
+    uv_process_options_.uid = static_cast<uv_uid_t>(uid);
     uv_process_options_.flags |= UV_PROCESS_SETUID;
   }
 
   Local<Value> js_gid = js_options->Get(env()->gid_string());
   if (IsSet(js_gid)) {
-    if (!CheckRange<uv_gid_t>(js_gid))
+    if (!js_gid->IsInt32())
       return UV_EINVAL;
-    uv_process_options_.gid = static_cast<uv_gid_t>(js_gid->Int32Value());
+    const int32_t gid = js_gid->Int32Value(env()->context()).FromJust();
+    uv_process_options_.gid = static_cast<uv_gid_t>(gid);
     uv_process_options_.flags |= UV_PROCESS_SETGID;
   }
 
@@ -763,7 +765,7 @@ int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
 
   Local<Value> js_max_buffer = js_options->Get(env()->max_buffer_string());
   if (IsSet(js_max_buffer)) {
-    if (!CheckRange<uint32_t>(js_max_buffer))
+    if (!js_max_buffer->IsUint32())
       return UV_EINVAL;
     max_buffer_ = js_max_buffer->Uint32Value();
   }
@@ -915,27 +917,6 @@ bool SyncProcessRunner::IsSet(Local<Value> value) {
 }
 
 
-template <typename t>
-bool SyncProcessRunner::CheckRange(Local<Value> js_value) {
-  if ((t) -1 > 0) {
-    // Unsigned range check.
-    if (!js_value->IsUint32())
-      return false;
-    if (js_value->Uint32Value() & ~((t) ~0))
-      return false;
-
-  } else {
-    // Signed range check.
-    if (!js_value->IsInt32())
-      return false;
-    if (js_value->Int32Value() & ~((t) ~0))
-      return false;
-  }
-
-  return true;
-}
-
-
 int SyncProcessRunner::CopyJsString(Local<Value> js_value,
                                     const char** target) {
   Isolate* isolate = env()->isolate();