From 3f9186e03ce59a380e5b107c8a71b6e9611dd267 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Wed, 29 Aug 2018 15:39:55 +0200 Subject: src: rework (mostly internal) functions to use Maybes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rework all affected functions to use Maybes, thus improving error handling substantially in internal functions, API functions as well as utilities. Co-authored-by: Michaël Zasso PR-URL: https://github.com/nodejs/node/pull/21935 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- src/spawn_sync.cc | 125 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 71 insertions(+), 54 deletions(-) (limited to 'src/spawn_sync.cc') diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index ba29aaeef03..77449b9569d 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -37,7 +37,11 @@ using v8::FunctionCallbackInfo; using v8::HandleScope; using v8::Integer; using v8::Isolate; +using v8::Just; using v8::Local; +using v8::Maybe; +using v8::MaybeLocal; +using v8::Nothing; using v8::Null; using v8::Number; using v8::Object; @@ -372,7 +376,8 @@ void SyncProcessRunner::Spawn(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); env->PrintSyncTrace(); SyncProcessRunner p(env); - Local result = p.Run(args[0]); + Local result; + if (!p.Run(args[0]).ToLocal(&result)) return; args.GetReturnValue().Set(result); } @@ -430,22 +435,21 @@ Environment* SyncProcessRunner::env() const { return env_; } - -Local SyncProcessRunner::Run(Local options) { +MaybeLocal SyncProcessRunner::Run(Local options) { EscapableHandleScope scope(env()->isolate()); CHECK_EQ(lifecycle_, kUninitialized); - TryInitializeAndRunLoop(options); + Maybe r = TryInitializeAndRunLoop(options); CloseHandlesAndDeleteLoop(); + if (r.IsNothing()) return MaybeLocal(); Local result = BuildResultObject(); return scope.Escape(result); } - -void SyncProcessRunner::TryInitializeAndRunLoop(Local options) { +Maybe SyncProcessRunner::TryInitializeAndRunLoop(Local options) { int r; // There is no recovery from failure inside TryInitializeAndRunLoop - the @@ -454,18 +458,24 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local options) { lifecycle_ = kInitialized; uv_loop_ = new uv_loop_t; - if (uv_loop_ == nullptr) - return SetError(UV_ENOMEM); + if (uv_loop_ == nullptr) { + SetError(UV_ENOMEM); + return Just(false); + } CHECK_EQ(uv_loop_init(uv_loop_), 0); - r = ParseOptions(options); - if (r < 0) - return SetError(r); + if (!ParseOptions(options).To(&r)) return Nothing(); + if (r < 0) { + SetError(r); + return Just(false); + } if (timeout_ > 0) { r = uv_timer_init(uv_loop_, &uv_timer_); - if (r < 0) - return SetError(r); + if (r < 0) { + SetError(r); + return Just(false); + } uv_unref(reinterpret_cast(&uv_timer_)); @@ -477,22 +487,28 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local options) { // which implicitly stops it, so there is no risk that the timeout callback // runs when the process didn't start. r = uv_timer_start(&uv_timer_, KillTimerCallback, timeout_, 0); - if (r < 0) - return SetError(r); + if (r < 0) { + SetError(r); + return Just(false); + } } uv_process_options_.exit_cb = ExitCallback; r = uv_spawn(uv_loop_, &uv_process_, &uv_process_options_); - if (r < 0) - return SetError(r); + if (r < 0) { + SetError(r); + return Just(false); + } uv_process_.data = this; for (uint32_t i = 0; i < stdio_count_; i++) { SyncProcessStdioPipe* h = stdio_pipes_[i].get(); if (h != nullptr) { r = h->Start(); - if (r < 0) - return SetPipeError(r); + if (r < 0) { + SetPipeError(r); + return Just(false); + } } } @@ -503,6 +519,7 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local options) { // If we get here the process should have exited. CHECK_GE(exit_status_, 0); + return Just(true); } @@ -724,46 +741,41 @@ Local SyncProcessRunner::BuildOutputArray() { return scope.Escape(js_output); } - -int SyncProcessRunner::ParseOptions(Local js_value) { +Maybe SyncProcessRunner::ParseOptions(Local js_value) { HandleScope scope(env()->isolate()); int r; - if (!js_value->IsObject()) - return UV_EINVAL; + if (!js_value->IsObject()) return Just(UV_EINVAL); Local context = env()->context(); Local js_options = js_value.As(); Local js_file = js_options->Get(context, env()->file_string()).ToLocalChecked(); - r = CopyJsString(js_file, &file_buffer_); - if (r < 0) - return r; + if (!CopyJsString(js_file, &file_buffer_).To(&r)) return Nothing(); + if (r < 0) return Just(r); uv_process_options_.file = file_buffer_; Local js_args = js_options->Get(context, env()->args_string()).ToLocalChecked(); - r = CopyJsStringArray(js_args, &args_buffer_); - if (r < 0) - return r; + if (!CopyJsStringArray(js_args, &args_buffer_).To(&r)) return Nothing(); + if (r < 0) return Just(r); uv_process_options_.args = reinterpret_cast(args_buffer_); Local js_cwd = js_options->Get(context, env()->cwd_string()).ToLocalChecked(); if (IsSet(js_cwd)) { - r = CopyJsString(js_cwd, &cwd_buffer_); - if (r < 0) - return r; + if (!CopyJsString(js_cwd, &cwd_buffer_).To(&r)) return Nothing(); + if (r < 0) return Just(r); uv_process_options_.cwd = cwd_buffer_; } Local js_env_pairs = js_options->Get(context, env()->env_pairs_string()).ToLocalChecked(); if (IsSet(js_env_pairs)) { - r = CopyJsStringArray(js_env_pairs, &env_buffer_); - if (r < 0) - return r; + if (!CopyJsStringArray(js_env_pairs, &env_buffer_).To(&r)) + return Nothing(); + if (r < 0) return Just(r); uv_process_options_.env = reinterpret_cast(env_buffer_); } @@ -827,10 +839,9 @@ int SyncProcessRunner::ParseOptions(Local js_value) { Local js_stdio = js_options->Get(context, env()->stdio_string()).ToLocalChecked(); r = ParseStdioOptions(js_stdio); - if (r < 0) - return r; + if (r < 0) return Just(r); - return 0; + return Just(0); } @@ -970,9 +981,8 @@ bool SyncProcessRunner::IsSet(Local value) { return !value->IsUndefined() && !value->IsNull(); } - -int SyncProcessRunner::CopyJsString(Local js_value, - const char** target) { +Maybe SyncProcessRunner::CopyJsString(Local js_value, + const char** target) { Isolate* isolate = env()->isolate(); Local js_string; size_t size, written; @@ -980,12 +990,14 @@ int SyncProcessRunner::CopyJsString(Local js_value, if (js_value->IsString()) js_string = js_value.As(); - else - js_string = js_value->ToString(env()->isolate()->GetCurrentContext()) - .ToLocalChecked(); + else if (!js_value->ToString(env()->isolate()->GetCurrentContext()) + .ToLocal(&js_string)) + return Nothing(); // Include space for null terminator byte. - size = StringBytes::StorageSize(isolate, js_string, UTF8) + 1; + if (!StringBytes::StorageSize(isolate, js_string, UTF8).To(&size)) + return Nothing(); + size += 1; buffer = new char[size]; @@ -993,12 +1005,11 @@ int SyncProcessRunner::CopyJsString(Local js_value, buffer[written] = '\0'; *target = buffer; - return 0; + return Just(0); } - -int SyncProcessRunner::CopyJsStringArray(Local js_value, - char** target) { +Maybe SyncProcessRunner::CopyJsStringArray(Local js_value, + char** target) { Isolate* isolate = env()->isolate(); Local js_array; uint32_t length; @@ -1006,8 +1017,7 @@ int SyncProcessRunner::CopyJsStringArray(Local js_value, char** list; char* buffer; - if (!js_value->IsArray()) - return UV_EINVAL; + if (!js_value->IsArray()) return Just(UV_EINVAL); Local context = env()->context(); js_array = js_value.As()->Clone().As(); @@ -1025,15 +1035,22 @@ int SyncProcessRunner::CopyJsStringArray(Local js_value, for (uint32_t i = 0; i < length; i++) { auto value = js_array->Get(context, i).ToLocalChecked(); - if (!value->IsString()) + if (!value->IsString()) { + Local string; + if (!value->ToString(env()->isolate()->GetCurrentContext()) + .ToLocal(&string)) + return Nothing(); js_array ->Set(context, i, value->ToString(env()->isolate()->GetCurrentContext()) .ToLocalChecked()) .FromJust(); + } - data_size += StringBytes::StorageSize(isolate, value, UTF8) + 1; + Maybe maybe_size = StringBytes::StorageSize(isolate, value, UTF8); + if (maybe_size.IsNothing()) return Nothing(); + data_size += maybe_size.FromJust() + 1; data_size = ROUND_UP(data_size, sizeof(void*)); } @@ -1057,7 +1074,7 @@ int SyncProcessRunner::CopyJsStringArray(Local js_value, list[length] = nullptr; *target = buffer; - return 0; + return Just(0); } -- cgit v1.2.3