Skip to content

Commit 2903030

Browse files
trevnorrischrisdickinson
trevnorris
authored and
chrisdickinson
committed
buffer: switch API to return MaybeLocal<T>
Instead of aborting in case of internal failure, return an empty Local<Object>. Using the MaybeLocal<T> API, users must check their return values. PR-URL: #1825 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent ad91c1d commit 2903030

9 files changed

Lines changed: 164 additions & 132 deletions

File tree

‎src/js_stream.cc‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,11 @@ int JSStream::DoWrite(WriteWrap* w,
8888
HandleScope scope(env()->isolate());
8989

9090
Local<Array> bufs_arr = Array::New(env()->isolate(), count);
91-
for (size_t i = 0; i < count; i++)
92-
bufs_arr->Set(i, Buffer::New(env(), bufs[i].base, bufs[i].len));
91+
Local<Object> buf;
92+
for (size_t i = 0; i < count; i++) {
93+
buf = Buffer::New(env(), bufs[i].base, bufs[i].len).ToLocalChecked();
94+
bufs_arr->Set(i, buf);
95+
}
9396

9497
Local<Value> argv[] = {
9598
w->object(),
@@ -134,11 +137,13 @@ void JSStream::DoAlloc(const FunctionCallbackInfo<Value>& args) {
134137

135138
uv_buf_t buf;
136139
wrap->OnAlloc(args[0]->Int32Value(), &buf);
137-
args.GetReturnValue().Set(Buffer::New(wrap->env(),
138-
buf.base,
139-
buf.len,
140-
FreeCallback,
141-
nullptr));
140+
Local<Object> vbuf = Buffer::New(
141+
wrap->env(),
142+
buf.base,
143+
buf.len,
144+
FreeCallback,
145+
nullptr).ToLocalChecked();
146+
return args.GetReturnValue().Set(vbuf);
142147
}
143148

144149

‎src/node_buffer.cc‎

Lines changed: 71 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ using v8::HandleScope;
6868
using v8::Isolate;
6969
using v8::Local;
7070
using v8::Maybe;
71+
using v8::MaybeLocal;
7172
using v8::Number;
7273
using v8::Object;
7374
using v8::Persistent;
@@ -228,7 +229,9 @@ size_t Length(Handle<Object> obj) {
228229
}
229230

230231

231-
Local<Object> New(Isolate* isolate, Handle<String> string, enum encoding enc) {
232+
MaybeLocal<Object> New(Isolate* isolate,
233+
Local<String> string,
234+
enum encoding enc) {
232235
EscapableHandleScope scope(isolate);
233236

234237
size_t length = StringBytes::Size(isolate, string, enc);
@@ -245,19 +248,26 @@ Local<Object> New(Isolate* isolate, Handle<String> string, enum encoding enc) {
245248
CHECK_NE(data, nullptr);
246249
}
247250

248-
Local<Object> buf = Use(isolate, data, actual);
249-
return scope.Escape(buf);
251+
Local<Object> buf;
252+
if (Use(isolate, data, actual).ToLocal(&buf))
253+
return scope.Escape(buf);
254+
255+
// Object failed to be created. Clean up resources.
256+
free(data);
257+
return Local<Object>();
250258
}
251259

252260

253-
Local<Object> New(Isolate* isolate, size_t length) {
261+
MaybeLocal<Object> New(Isolate* isolate, size_t length) {
254262
EscapableHandleScope handle_scope(isolate);
255-
Local<Object> obj = Buffer::New(Environment::GetCurrent(isolate), length);
256-
return handle_scope.Escape(obj);
263+
Local<Object> obj;
264+
if (Buffer::New(Environment::GetCurrent(isolate), length).ToLocal(&obj))
265+
return handle_scope.Escape(obj);
266+
return Local<Object>();
257267
}
258268

259269

260-
Local<Object> New(Environment* env, size_t length) {
270+
MaybeLocal<Object> New(Environment* env, size_t length) {
261271
EscapableHandleScope scope(env->isolate());
262272

263273
if (using_old_buffer) {
@@ -280,13 +290,12 @@ Local<Object> New(Environment* env, size_t length) {
280290
void* data;
281291
if (length > 0) {
282292
data = malloc(length);
283-
// NOTE: API change. Must check .IsEmpty() on the return object to see if
284-
// the data was able to be allocated.
285293
if (data == nullptr)
286294
return Local<Object>();
287295
} else {
288296
data = nullptr;
289297
}
298+
290299
Local<ArrayBuffer> ab =
291300
ArrayBuffer::New(env->isolate(),
292301
data,
@@ -295,25 +304,27 @@ Local<Object> New(Environment* env, size_t length) {
295304
Local<Uint8Array> ui = Uint8Array::New(ab, 0, length);
296305
Maybe<bool> mb =
297306
ui->SetPrototype(env->context(), env->buffer_prototype_object());
298-
if (!mb.FromMaybe(false)) {
299-
FatalError("node::Buffer::New(Environment*, size_t)",
300-
"Could not set Object prototype");
301-
UNREACHABLE();
302-
}
303-
return scope.Escape(ui);
307+
if (mb.FromMaybe(false))
308+
return scope.Escape(ui);
309+
310+
// Object failed to be created. Clean up resources.
311+
free(data);
312+
return Local<Object>();
304313
}
305314

306315

307-
Local<Object> New(Isolate* isolate, const char* data, size_t length) {
316+
MaybeLocal<Object> New(Isolate* isolate, const char* data, size_t length) {
308317
Environment* env = Environment::GetCurrent(isolate);
309318
EscapableHandleScope handle_scope(env->isolate());
310-
Local<Object> obj = Buffer::New(env, data, length);
311-
return handle_scope.Escape(obj);
319+
Local<Object> obj;
320+
if (Buffer::New(env, data, length).ToLocal(&obj))
321+
return handle_scope.Escape(obj);
322+
return Local<Object>();
312323
}
313324

314325

315326
// Make a copy of "data". Why this isn't called "Copy", we'll never know.
316-
Local<Object> New(Environment* env, const char* data, size_t length) {
327+
MaybeLocal<Object> New(Environment* env, const char* data, size_t length) {
317328
EscapableHandleScope scope(env->isolate());
318329

319330
if (using_old_buffer) {
@@ -347,8 +358,6 @@ Local<Object> New(Environment* env, const char* data, size_t length) {
347358
if (length > 0) {
348359
CHECK_NE(data, nullptr);
349360
new_data = malloc(length);
350-
// NOTE: API change. Must check .IsEmpty() on the return object to see if
351-
// the data was able to be allocated.
352361
if (new_data == nullptr)
353362
return Local<Object>();
354363
memcpy(new_data, data, length);
@@ -364,33 +373,34 @@ Local<Object> New(Environment* env, const char* data, size_t length) {
364373
Local<Uint8Array> ui = Uint8Array::New(ab, 0, length);
365374
Maybe<bool> mb =
366375
ui->SetPrototype(env->context(), env->buffer_prototype_object());
367-
if (!mb.FromMaybe(false)) {
368-
FatalError("node::Buffer::New(Environment*, char*, size_t)",
369-
"Could not set Object prototype");
370-
UNREACHABLE();
371-
}
376+
if (mb.FromMaybe(false))
377+
return scope.Escape(ui);
372378

373-
return scope.Escape(ui);
379+
// Object failed to be created. Clean up resources.
380+
free(new_data);
381+
return Local<Object>();
374382
}
375383

376384

377-
Local<Object> New(Isolate* isolate,
378-
char* data,
379-
size_t length,
380-
FreeCallback callback,
381-
void* hint) {
385+
MaybeLocal<Object> New(Isolate* isolate,
386+
char* data,
387+
size_t length,
388+
FreeCallback callback,
389+
void* hint) {
382390
Environment* env = Environment::GetCurrent(isolate);
383391
EscapableHandleScope handle_scope(env->isolate());
384-
Local<Object> obj = Buffer::New(env, data, length, callback, hint);
385-
return handle_scope.Escape(obj);
392+
Local<Object> obj;
393+
if (Buffer::New(env, data, length, callback, hint).ToLocal(&obj))
394+
return handle_scope.Escape(obj);
395+
return Local<Object>();
386396
}
387397

388398

389-
Local<Object> New(Environment* env,
390-
char* data,
391-
size_t length,
392-
FreeCallback callback,
393-
void* hint) {
399+
MaybeLocal<Object> New(Environment* env,
400+
char* data,
401+
size_t length,
402+
FreeCallback callback,
403+
void* hint) {
394404
EscapableHandleScope scope(env->isolate());
395405

396406
if (using_old_buffer) {
@@ -410,26 +420,26 @@ Local<Object> New(Environment* env,
410420
Local<Uint8Array> ui = Uint8Array::New(ab, 0, length);
411421
Maybe<bool> mb =
412422
ui->SetPrototype(env->context(), env->buffer_prototype_object());
413-
if (!mb.FromMaybe(false)) {
414-
FatalError("node::Buffer::New(Environment*, char*, size_t,"
415-
" FreeCallback, void*)",
416-
"Could not set Object prototype");
417-
UNREACHABLE();
418-
}
423+
424+
if (!mb.FromMaybe(false))
425+
return Local<Object>();
426+
419427
CallbackInfo::New(env->isolate(), ui, callback, hint);
420428
return scope.Escape(ui);
421429
}
422430

423431

424-
Local<Object> Use(Isolate* isolate, char* data, size_t length) {
432+
MaybeLocal<Object> Use(Isolate* isolate, char* data, size_t length) {
425433
Environment* env = Environment::GetCurrent(isolate);
426434
EscapableHandleScope handle_scope(env->isolate());
427-
Local<Object> obj = Buffer::Use(env, data, length);
428-
return handle_scope.Escape(obj);
435+
Local<Object> obj;
436+
if (Buffer::Use(env, data, length).ToLocal(&obj))
437+
return handle_scope.Escape(obj);
438+
return Local<Object>();
429439
}
430440

431441

432-
Local<Object> Use(Environment* env, char* data, size_t length) {
442+
MaybeLocal<Object> Use(Environment* env, char* data, size_t length) {
433443
EscapableHandleScope scope(env->isolate());
434444

435445
if (using_old_buffer) {
@@ -457,12 +467,9 @@ Local<Object> Use(Environment* env, char* data, size_t length) {
457467
Local<Uint8Array> ui = Uint8Array::New(ab, 0, length);
458468
Maybe<bool> mb =
459469
ui->SetPrototype(env->context(), env->buffer_prototype_object());
460-
if (!mb.FromMaybe(false)) {
461-
FatalError("node::Buffer::Use(Environment*, char*, size_t)",
462-
"Could not set Object prototype");
463-
UNREACHABLE();
464-
}
465-
return scope.Escape(ui);
470+
if (mb.FromMaybe(false))
471+
return scope.Escape(ui);
472+
return Local<Object>();
466473
}
467474

468475

@@ -495,8 +502,9 @@ void Create(const FunctionCallbackInfo<Value>& args) {
495502
Local<Uint8Array> ui = Uint8Array::New(ab, 0, length);
496503
Maybe<bool> mb =
497504
ui->SetPrototype(env->context(), env->buffer_prototype_object());
498-
if (mb.FromMaybe(false))
499-
args.GetReturnValue().Set(ui);
505+
if (!mb.FromMaybe(false))
506+
return env->ThrowError("Unable to set Object prototype");
507+
args.GetReturnValue().Set(ui);
500508
}
501509

502510

@@ -507,8 +515,9 @@ void CreateFromString(const FunctionCallbackInfo<Value>& args) {
507515
enum encoding enc = ParseEncoding(args.GetIsolate(),
508516
args[1].As<String>(),
509517
UTF8);
510-
Local<Object> buf = New(args.GetIsolate(), args[0].As<String>(), enc);
511-
args.GetReturnValue().Set(buf);
518+
Local<Object> buf;
519+
if (New(args.GetIsolate(), args[0].As<String>(), enc).ToLocal(&buf))
520+
args.GetReturnValue().Set(buf);
512521
}
513522

514523

@@ -529,8 +538,9 @@ void Slice(const FunctionCallbackInfo<Value>& args) {
529538
Local<Uint8Array> ui = Uint8Array::New(ab, start, size);
530539
Maybe<bool> mb =
531540
ui->SetPrototype(env->context(), env->buffer_prototype_object());
532-
if (mb.FromMaybe(false))
533-
args.GetReturnValue().Set(ui);
541+
if (!mb.FromMaybe(false))
542+
env->ThrowError("Unable to set Object prototype");
543+
args.GetReturnValue().Set(ui);
534544
}
535545

536546

‎src/node_buffer.h‎

Lines changed: 34 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -24,50 +24,52 @@ NODE_EXTERN size_t Length(v8::Handle<v8::Value> val);
2424
NODE_EXTERN size_t Length(v8::Handle<v8::Object> val);
2525

2626
// public constructor
27-
NODE_EXTERN v8::Local<v8::Object> New(v8::Isolate* isolate, size_t length);
27+
NODE_EXTERN v8::MaybeLocal<v8::Object> New(v8::Isolate* isolate, size_t length);
2828
NODE_DEPRECATED("Use New(isolate, ...)",
29-
inline v8::Local<v8::Object> New(size_t length) {
29+
inline v8::MaybeLocal<v8::Object> New(size_t length) {
3030
return New(v8::Isolate::GetCurrent(), length);
3131
})
3232
// public constructor from string
33-
NODE_EXTERN v8::Local<v8::Object> New(v8::Isolate* isolate,
34-
v8::Handle<v8::String> string,
35-
enum encoding enc = UTF8);
33+
NODE_EXTERN v8::MaybeLocal<v8::Object> New(v8::Isolate* isolate,
34+
v8::Handle<v8::String> string,
35+
enum encoding enc = UTF8);
3636
NODE_DEPRECATED("Use New(isolate, ...)",
37-
inline v8::Local<v8::Object> New(v8::Handle<v8::String> string,
38-
enum encoding enc = UTF8) {
37+
inline v8::MaybeLocal<v8::Object> New(
38+
v8::Handle<v8::String> string,
39+
enum encoding enc = UTF8) {
3940
return New(v8::Isolate::GetCurrent(), string, enc);
4041
})
4142
// public constructor - data is copied
4243
// TODO(trevnorris): should be something like Copy()
43-
NODE_EXTERN v8::Local<v8::Object> New(v8::Isolate* isolate,
44-
const char* data,
45-
size_t len);
44+
NODE_EXTERN v8::MaybeLocal<v8::Object> New(v8::Isolate* isolate,
45+
const char* data,
46+
size_t len);
4647
NODE_DEPRECATED("Use New(isolate, ...)",
47-
inline v8::Local<v8::Object> New(const char* data, size_t len) {
48+
inline v8::MaybeLocal<v8::Object> New(const char* data,
49+
size_t len) {
4850
return New(v8::Isolate::GetCurrent(), data, len);
4951
})
5052
// public constructor - data is used, callback is passed data on object gc
51-
NODE_EXTERN v8::Local<v8::Object> New(v8::Isolate* isolate,
52-
char* data,
53-
size_t length,
54-
FreeCallback callback,
55-
void* hint);
53+
NODE_EXTERN v8::MaybeLocal<v8::Object> New(v8::Isolate* isolate,
54+
char* data,
55+
size_t length,
56+
FreeCallback callback,
57+
void* hint);
5658
NODE_DEPRECATED("Use New(isolate, ...)",
57-
inline v8::Local<v8::Object> New(char* data,
58-
size_t length,
59-
FreeCallback callback,
60-
void* hint) {
59+
inline v8::MaybeLocal<v8::Object> New(char* data,
60+
size_t length,
61+
FreeCallback callback,
62+
void* hint) {
6163
return New(v8::Isolate::GetCurrent(), data, length, callback, hint);
6264
})
6365

6466
// public constructor - data is used.
6567
// TODO(trevnorris): should be New() for consistency
66-
NODE_EXTERN v8::Local<v8::Object> Use(v8::Isolate* isolate,
67-
char* data,
68-
size_t len);
68+
NODE_EXTERN v8::MaybeLocal<v8::Object> Use(v8::Isolate* isolate,
69+
char* data,
70+
size_t len);
6971
NODE_DEPRECATED("Use Use(isolate, ...)",
70-
inline v8::Local<v8::Object> Use(char* data, size_t len) {
72+
inline v8::MaybeLocal<v8::Object> Use(char* data, size_t len) {
7173
return Use(v8::Isolate::GetCurrent(), data, len);
7274
})
7375

@@ -90,14 +92,14 @@ static inline bool IsWithinBounds(size_t off, size_t len, size_t max) {
9092
// src/node_internals.h due to a circular dependency issue with
9193
// the smalloc.h and node_internals.h headers.
9294
#if defined(NODE_WANT_INTERNALS)
93-
v8::Local<v8::Object> New(Environment* env, size_t size);
94-
v8::Local<v8::Object> New(Environment* env, const char* data, size_t len);
95-
v8::Local<v8::Object> New(Environment* env,
96-
char* data,
97-
size_t length,
98-
FreeCallback callback,
99-
void* hint);
100-
v8::Local<v8::Object> Use(Environment* env, char* data, size_t length);
95+
v8::MaybeLocal<v8::Object> New(Environment* env, size_t size);
96+
v8::MaybeLocal<v8::Object> New(Environment* env, const char* data, size_t len);
97+
v8::MaybeLocal<v8::Object> New(Environment* env,
98+
char* data,
99+
size_t length,
100+
FreeCallback callback,
101+
void* hint);
102+
v8::MaybeLocal<v8::Object> Use(Environment* env, char* data, size_t length);
101103
#endif // defined(NODE_WANT_INTERNALS)
102104

103105
} // namespace Buffer

0 commit comments

Comments
 (0)