From f3e921aee4ce304f43056a3a5aade328ca70fd11 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Tue, 18 Oct 2016 15:04:41 -0400 Subject: [PATCH] src: remove redundant spawn/spawnSync type checks This commit removes C++ checks from spawn() and spawnSync() that are duplicates of the JavaScript type checking. Fixes: https://github.com/nodejs/node/issues/8096 Fixes: https://github.com/nodejs/node/issues/8539 Refs: https://github.com/nodejs/node/issues/9722 PR-URL: https://github.com/nodejs/node/pull/8312 Reviewed-By: Ben Noordhuis --- src/process_wrap.cc | 20 +++++++------------- src/spawn_sync.cc | 19 +++++-------------- 2 files changed, 12 insertions(+), 27 deletions(-) diff --git a/src/process_wrap.cc b/src/process_wrap.cc index 3dcde0962af080..8c8e4704be4f82 100644 --- a/src/process_wrap.cc +++ b/src/process_wrap.cc @@ -121,35 +121,29 @@ class ProcessWrap : public HandleWrap { // options.uid Local uid_v = js_options->Get(env->uid_string()); - if (uid_v->IsInt32()) { + if (!uid_v->IsUndefined() && !uid_v->IsNull()) { + CHECK(uid_v->IsInt32()); const int32_t uid = uid_v->Int32Value(env->context()).FromJust(); options.flags |= UV_PROCESS_SETUID; options.uid = static_cast(uid); - } else if (!uid_v->IsUndefined() && !uid_v->IsNull()) { - return env->ThrowTypeError("options.uid should be a number"); } // options.gid Local gid_v = js_options->Get(env->gid_string()); - if (gid_v->IsInt32()) { + if (!gid_v->IsUndefined() && !gid_v->IsNull()) { + CHECK(gid_v->IsInt32()); const int32_t gid = gid_v->Int32Value(env->context()).FromJust(); options.flags |= UV_PROCESS_SETGID; options.gid = static_cast(gid); - } else if (!gid_v->IsUndefined() && !gid_v->IsNull()) { - return env->ThrowTypeError("options.gid should be a number"); } // TODO(bnoordhuis) is this possible to do without mallocing ? // options.file Local file_v = js_options->Get(env->file_string()); - node::Utf8Value file(env->isolate(), - file_v->IsString() ? file_v : Local()); - if (file.length() > 0) { - options.file = *file; - } else { - return env->ThrowTypeError("Bad argument"); - } + CHECK(file_v->IsString()); + node::Utf8Value file(env->isolate(), file_v); + options.file = *file; // options.args Local argv_v = js_options->Get(env->args_string()); diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 689f605e1446cd..68e78147578ff1 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -734,8 +734,7 @@ int SyncProcessRunner::ParseOptions(Local js_value) { } Local js_uid = js_options->Get(env()->uid_string()); if (IsSet(js_uid)) { - if (!js_uid->IsInt32()) - return UV_EINVAL; + CHECK(js_uid->IsInt32()); const int32_t uid = js_uid->Int32Value(env()->context()).FromJust(); uv_process_options_.uid = static_cast(uid); uv_process_options_.flags |= UV_PROCESS_SETUID; @@ -743,8 +742,7 @@ int SyncProcessRunner::ParseOptions(Local js_value) { Local js_gid = js_options->Get(env()->gid_string()); if (IsSet(js_gid)) { - if (!js_gid->IsInt32()) - return UV_EINVAL; + CHECK(js_gid->IsInt32()); const int32_t gid = js_gid->Int32Value(env()->context()).FromJust(); uv_process_options_.gid = static_cast(gid); uv_process_options_.flags |= UV_PROCESS_SETGID; @@ -760,28 +758,21 @@ int SyncProcessRunner::ParseOptions(Local js_value) { Local js_timeout = js_options->Get(env()->timeout_string()); if (IsSet(js_timeout)) { - if (!js_timeout->IsNumber()) - return UV_EINVAL; + CHECK(js_timeout->IsNumber()); int64_t timeout = js_timeout->IntegerValue(); - if (timeout < 0) - return UV_EINVAL; timeout_ = static_cast(timeout); } Local js_max_buffer = js_options->Get(env()->max_buffer_string()); if (IsSet(js_max_buffer)) { - if (!js_max_buffer->IsUint32()) - return UV_EINVAL; + CHECK(js_max_buffer->IsUint32()); max_buffer_ = js_max_buffer->Uint32Value(); } Local js_kill_signal = js_options->Get(env()->kill_signal_string()); if (IsSet(js_kill_signal)) { - if (!js_kill_signal->IsInt32()) - return UV_EINVAL; + CHECK(js_kill_signal->IsInt32()); kill_signal_ = js_kill_signal->Int32Value(); - if (kill_signal_ == 0) - return UV_EINVAL; } Local js_stdio = js_options->Get(env()->stdio_string());