src: fix bad logic in uid/gid checks

Pointed out by Coverity.  Introduced in commits 3546383 ("process_wrap:
avoid leaking memory when throwing due to invalid arguments") and
fa4eb47 ("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: #7374
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>

src/process_wrap.cc

@@ -120,25 +120,19 @@ class ProcessWrap : public HandleWrap {
     // options.uid
     Local<Value> uid_v = js_options->Get(env->uid_string());
     if (uid_v->IsInt32()) {
-      int32_t uid = uid_v->Int32Value();
-      if (uid & ~((uv_uid_t) ~0)) {
-        return env->ThrowRangeError("options.uid is out of range");
-      }
+      const int32_t uid = uid_v->Int32Value(env->context()).FromJust();
       options.flags |= UV_PROCESS_SETUID;
-      options.uid = (uv_uid_t) uid;
+      options.uid = static_cast<uv_uid_t>(uid);
     } else if (!uid_v->IsUndefined() && !uid_v->IsNull()) {
       return env->ThrowTypeError("options.uid should be a number");
     }
 
     // options.gid
     Local<Value> gid_v = js_options->Get(env->gid_string());
     if (gid_v->IsInt32()) {
-      int32_t gid = gid_v->Int32Value();
-      if (gid & ~((uv_gid_t) ~0)) {
-        return env->ThrowRangeError("options.gid is out of range");
-      }
+      const int32_t gid = gid_v->Int32Value(env->context()).FromJust();
       options.flags |= UV_PROCESS_SETGID;
-      options.gid = (uv_gid_t) gid;
+      options.gid = static_cast<uv_gid_t>(gid);
     } else if (!gid_v->IsUndefined() && !gid_v->IsNull()) {
       return env->ThrowTypeError("options.gid should be a number");
     }

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();

src/spawn_sync.h

@@ -173,7 +173,6 @@ class SyncProcessRunner {
   inline int AddStdioInheritFD(uint32_t child_fd, int inherit_fd);
 
   static bool IsSet(Local<Value> value);
-  template <typename t> static bool CheckRange(Local<Value> js_value);
   int CopyJsString(Local<Value> js_value, const char** target);
   int CopyJsStringArray(Local<Value> js_value, char** target);