Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: remove calls to deprecated v8 functions (ToString) #21935

Closed
wants to merge 3 commits into from
Closed

src: remove calls to deprecated v8 functions (ToString) #21935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4a2f922
src: remove calls to deprecated v8 functions (ToString)
targos Aug 29, 2018
96fd305
src: rework (mostly internal) functions to use Maybes
targos Aug 29, 2018
d096c49
fixup! add using statements
targos Aug 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/node_buffer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,9 @@ MaybeLocal<Object> New(Isolate* isolate,
enum encoding enc) {
EscapableHandleScope scope(isolate);

const size_t length = StringBytes::Size(isolate, string, enc);
size_t length;
if (!StringBytes::Size(isolate, string, enc).To(&length))
return Local<Object>();
size_t actual = 0;
char* data = nullptr;

Expand Down Expand Up @@ -828,7 +830,8 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
const size_t haystack_length = (enc == UCS2) ?
ts_obj_length &~ 1 : ts_obj_length; // NOLINT(whitespace/operators)

const size_t needle_length = StringBytes::Size(isolate, needle, enc);
size_t needle_length;
if (!StringBytes::Size(isolate, needle, enc).To(&needle_length)) return;

int64_t opt_offset = IndexOfOffset(haystack_length,
offset_i64,
Expand Down Expand Up @@ -868,7 +871,7 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {

if (IsBigEndian()) {
StringBytes::InlineDecoder decoder;
decoder.Decode(env, needle, args[3], UCS2);
if (decoder.Decode(env, needle, args[3], UCS2).IsNothing()) return;
const uint16_t* decoded_string =
reinterpret_cast<const uint16_t*>(decoder.out());

Expand Down
9 changes: 6 additions & 3 deletions src/node_crypto.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3055,7 +3055,8 @@ void CipherBase::Update(const FunctionCallbackInfo<Value>& args) {
// Only copy the data if we have to, because it's a string
if (args[0]->IsString()) {
StringBytes::InlineDecoder decoder;
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8))
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8)
.FromMaybe(false))
return;
r = cipher->Update(decoder.out(), decoder.size(), &out, &out_len);
} else {
Expand Down Expand Up @@ -3241,7 +3242,8 @@ void Hmac::HmacUpdate(const FunctionCallbackInfo<Value>& args) {
bool r = false;
if (args[0]->IsString()) {
StringBytes::InlineDecoder decoder;
if (decoder.Decode(env, args[0].As<String>(), args[1], UTF8)) {
if (decoder.Decode(env, args[0].As<String>(), args[1], UTF8)
.FromMaybe(false)) {
r = hmac->HmacUpdate(decoder.out(), decoder.size());
}
} else {
Expand Down Expand Up @@ -3348,7 +3350,8 @@ void Hash::HashUpdate(const FunctionCallbackInfo<Value>& args) {
bool r = true;
if (args[0]->IsString()) {
StringBytes::InlineDecoder decoder;
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8)) {
if (!decoder.Decode(env, args[0].As<String>(), args[1], UTF8)
.FromMaybe(false)) {
args.GetReturnValue().Set(false);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/node_encoding.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ ssize_t DecodeBytes(Isolate* isolate,
enum encoding encoding) {
HandleScope scope(isolate);

return StringBytes::Size(isolate, val, encoding);
return StringBytes::Size(isolate, val, encoding).FromMaybe(-1);
}

// Returns number of bytes written.
Expand Down
5 changes: 3 additions & 2 deletions src/node_file.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1819,7 +1819,7 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {

if (is_async) { // write(fd, string, pos, enc, req)
CHECK_NOT_NULL(req_wrap_async);
len = StringBytes::StorageSize(env->isolate(), value, enc);
if (!StringBytes::StorageSize(env->isolate(), value, enc).To(&len)) return;
FSReqBase::FSReqBuffer& stack_buffer =
req_wrap_async->Init("write", len, enc);
// StorageSize may return too large a char, so correct the actual length
Expand Down Expand Up @@ -1847,7 +1847,8 @@ static void WriteString(const FunctionCallbackInfo<Value>& args) {
FSReqWrapSync req_wrap_sync;
FSReqBase::FSReqBuffer stack_buffer;
if (buf == nullptr) {
len = StringBytes::StorageSize(env->isolate(), value, enc);
if (!StringBytes::StorageSize(env->isolate(), value, enc).To(&len))
return;
stack_buffer.AllocateSufficientStorage(len + 1);
// StorageSize may return too large a char, so correct the actual length
// by the write size
Expand Down
131 changes: 77 additions & 54 deletions src/spawn_sync.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -372,7 +376,8 @@ void SyncProcessRunner::Spawn(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
env->PrintSyncTrace();
SyncProcessRunner p(env);
Local<Value> result = p.Run(args[0]);
Local<Value> result;
if (!p.Run(args[0]).ToLocal(&result)) return;
args.GetReturnValue().Set(result);
}

Expand Down Expand Up @@ -430,22 +435,21 @@ Environment* SyncProcessRunner::env() const {
return env_;
}


Local<Object> SyncProcessRunner::Run(Local<Value> options) {
MaybeLocal<Object> SyncProcessRunner::Run(Local<Value> options) {
EscapableHandleScope scope(env()->isolate());

CHECK_EQ(lifecycle_, kUninitialized);

TryInitializeAndRunLoop(options);
Maybe<bool> r = TryInitializeAndRunLoop(options);
CloseHandlesAndDeleteLoop();
if (r.IsNothing()) return MaybeLocal<Object>();

Local<Object> result = BuildResultObject();

return scope.Escape(result);
}


void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
Maybe<bool> SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {
int r;

// There is no recovery from failure inside TryInitializeAndRunLoop - the
Expand All @@ -454,18 +458,24 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> 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<bool>();
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_handle_t*>(&uv_timer_));

Expand All @@ -477,22 +487,28 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> 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);
}
}
}

Expand All @@ -503,6 +519,7 @@ void SyncProcessRunner::TryInitializeAndRunLoop(Local<Value> options) {

// If we get here the process should have exited.
CHECK_GE(exit_status_, 0);
return Just(true);
}


Expand Down Expand Up @@ -724,46 +741,41 @@ Local<Array> SyncProcessRunner::BuildOutputArray() {
return scope.Escape(js_output);
}


int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
Maybe<int> SyncProcessRunner::ParseOptions(Local<Value> js_value) {
HandleScope scope(env()->isolate());
int r;

if (!js_value->IsObject())
return UV_EINVAL;
if (!js_value->IsObject()) return Just<int>(UV_EINVAL);

Local<Context> context = env()->context();
Local<Object> js_options = js_value.As<Object>();

Local<Value> js_file =
js_options->Get(context, env()->file_string()).ToLocalChecked();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, I know that you are a huuuuge fan of PR scope creep, but ... feel free to take care of other such places in the functions you're already touching as well, if you like?

r = CopyJsString(js_file, &file_buffer_);
if (r < 0)
return r;
if (!CopyJsString(js_file, &file_buffer_).To(&r)) return Nothing<int>();
if (r < 0) return Just(r);
uv_process_options_.file = file_buffer_;

Local<Value> 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<int>();
if (r < 0) return Just(r);
uv_process_options_.args = reinterpret_cast<char**>(args_buffer_);

Local<Value> 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<int>();
if (r < 0) return Just(r);
uv_process_options_.cwd = cwd_buffer_;
}

Local<Value> 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<int>();
if (r < 0) return Just(r);

uv_process_options_.env = reinterpret_cast<char**>(env_buffer_);
}
Expand Down Expand Up @@ -827,10 +839,9 @@ int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
Local<Value> 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);
}


Expand Down Expand Up @@ -970,43 +981,43 @@ bool SyncProcessRunner::IsSet(Local<Value> value) {
return !value->IsUndefined() && !value->IsNull();
}


int SyncProcessRunner::CopyJsString(Local<Value> js_value,
const char** target) {
Maybe<int> SyncProcessRunner::CopyJsString(Local<Value> js_value,
const char** target) {
Isolate* isolate = env()->isolate();
Local<String> js_string;
size_t size, written;
char* buffer;

if (js_value->IsString())
js_string = js_value.As<String>();
else
js_string = js_value->ToString(env()->isolate());
else if (!js_value->ToString(env()->isolate()->GetCurrentContext())
.ToLocal(&js_string))
return Nothing<int>();

// 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<int>();
size += 1;

buffer = new char[size];

written = StringBytes::Write(isolate, buffer, -1, js_string, UTF8);
buffer[written] = '\0';

*target = buffer;
return 0;
return Just(0);
}


int SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
char** target) {
Maybe<int> SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
char** target) {
Isolate* isolate = env()->isolate();
Local<Array> js_array;
uint32_t length;
size_t list_size, data_size, data_offset;
char** list;
char* buffer;

if (!js_value->IsArray())
return UV_EINVAL;
if (!js_value->IsArray()) return Just<int>(UV_EINVAL);

Local<Context> context = env()->context();
js_array = js_value.As<Array>()->Clone().As<Array>();
Expand All @@ -1024,10 +1035,22 @@ int SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
for (uint32_t i = 0; i < length; i++) {
auto value = js_array->Get(context, i).ToLocalChecked();

if (!value->IsString())
js_array->Set(context, i, value->ToString(env()->isolate())).FromJust();
if (!value->IsString()) {
Local<String> string;
if (!value->ToString(env()->isolate()->GetCurrentContext())
.ToLocal(&string))
return Nothing<int>();
js_array
->Set(context,
i,
value->ToString(env()->isolate()->GetCurrentContext())
.ToLocalChecked())
.FromJust();
}

data_size += StringBytes::StorageSize(isolate, value, UTF8) + 1;
Maybe<size_t> maybe_size = StringBytes::StorageSize(isolate, value, UTF8);
if (maybe_size.IsNothing()) return Nothing<int>();
data_size += maybe_size.FromJust() + 1;
data_size = ROUND_UP(data_size, sizeof(void*));
}

Expand All @@ -1051,7 +1074,7 @@ int SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
list[length] = nullptr;

*target = buffer;
return 0;
return Just(0);
}


Expand Down
Loading