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

Jump to bottom

buffer: make Buffer binding and prototype setup more straight-forward #25292

Closed
wants to merge 2 commits into from
Closed

buffer: make Buffer binding and prototype setup more straight-forward #25292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa6b492
buffer: move initialization of buffer prototype into node.js
joyeecheung Dec 31, 2018
8d5c144
buffer: move Buffer prototype wiring into internal/buffer.js
joyeecheung Dec 31, 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
26 changes: 10 additions & 16 deletions lib/buffer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ const {
swap32: _swap32,
swap64: _swap64,
kMaxLength,
kStringMaxLength
kStringMaxLength,
zeroFill: bindingZeroFill
} = internalBinding('buffer');
const {
getOwnNonIndexProperties,
Expand Down Expand Up @@ -72,21 +73,14 @@ const {
} = require('internal/errors').codes;
const { validateString } = require('internal/validators');

const internalBuffer = require('internal/buffer');

const { setupBufferJS } = internalBuffer;

const bindingObj = {};
const {
FastBuffer,
addBufferPrototypeMethods
} = require('internal/buffer');

class FastBuffer extends Uint8Array {}
FastBuffer.prototype.constructor = Buffer;
internalBuffer.FastBuffer = FastBuffer;

Buffer.prototype = FastBuffer.prototype;

for (const [name, method] of Object.entries(internalBuffer.readWrites)) {
Buffer.prototype[name] = method;
}
addBufferPrototypeMethods(Buffer.prototype);

const constants = Object.defineProperties({}, {
MAX_LENGTH: {
Expand All @@ -104,11 +98,11 @@ const constants = Object.defineProperties({}, {
Buffer.poolSize = 8 * 1024;
var poolSize, poolOffset, allocPool;

setupBufferJS(Buffer.prototype, bindingObj);

// A toggle used to access the zero fill setting of the array buffer allocator
// in C++.
// |zeroFill| can be undefined when running inside an isolate where we
// do not own the ArrayBuffer allocator. Zero fill is always on in that case.
const zeroFill = bindingObj.zeroFill || [0];
const zeroFill = bindingZeroFill || [0];

function createUnsafeBuffer(size) {
return new FastBuffer(createUnsafeArrayBuffer(size));
Expand Down
13 changes: 9 additions & 4 deletions lib/internal/bootstrap/node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,16 +593,21 @@ function setupGlobalVariables() {
}
});

// This, as side effect, removes `setupBufferJS` from the buffer binding,
// and exposes it on `internal/buffer`.
NativeModule.require('internal/buffer');
const { Buffer } = NativeModule.require('buffer');
const bufferBinding = internalBinding('buffer');

// Only after this point can C++ use Buffer::New()
bufferBinding.setBufferPrototype(Buffer.prototype);
delete bufferBinding.setBufferPrototype;
delete bufferBinding.zeroFill;

Object.defineProperty(global, 'Buffer', {
value: NativeModule.require('buffer').Buffer,
value: Buffer,
enumerable: false,
writable: true,
configurable: true
});

process.domain = null;
process._exiting = false;
}
Expand Down
119 changes: 72 additions & 47 deletions lib/internal/buffer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,25 @@
'use strict';

const binding = internalBinding('buffer');
const {
ERR_BUFFER_OUT_OF_BOUNDS,
ERR_INVALID_ARG_TYPE,
ERR_OUT_OF_RANGE
} = require('internal/errors').codes;
const { validateNumber } = require('internal/validators');
const { setupBufferJS } = binding;

// Remove from the binding so that function is only available as exported here.
// (That is, for internal use only.)
delete binding.setupBufferJS;
const {
asciiSlice,
base64Slice,
latin1Slice,
hexSlice,
ucs2Slice,
utf8Slice,
asciiWrite,
base64Write,
latin1Write,
hexWrite,
ucs2Write,
utf8Write
} = internalBinding('buffer');

// Temporary buffers to convert numbers.
const float32Array = new Float32Array(1);
Expand Down Expand Up @@ -777,46 +785,63 @@ function writeFloatBackwards(val, offset = 0) {
return offset;
}

// FastBuffer wil be inserted here by lib/buffer.js
class FastBuffer extends Uint8Array {}

function addBufferPrototypeMethods(proto) {
proto.readUIntLE = readUIntLE;
Copy link
Member Author

Choose a reason for hiding this comment

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

Another way to do this is to put all these to the prototype of FastBuffer, I am not sure if there are any concerns about edge cases though

Copy link
Member

Choose a reason for hiding this comment

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

I am fine either way.

proto.readUInt32LE = readUInt32LE;
proto.readUInt16LE = readUInt16LE;
proto.readUInt8 = readUInt8;
proto.readUIntBE = readUIntBE;
proto.readUInt32BE = readUInt32BE;
proto.readUInt16BE = readUInt16BE;
proto.readIntLE = readIntLE;
proto.readInt32LE = readInt32LE;
proto.readInt16LE = readInt16LE;
proto.readInt8 = readInt8;
proto.readIntBE = readIntBE;
proto.readInt32BE = readInt32BE;
proto.readInt16BE = readInt16BE;

proto.writeUIntLE = writeUIntLE;
proto.writeUInt32LE = writeUInt32LE;
proto.writeUInt16LE = writeUInt16LE;
proto.writeUInt8 = writeUInt8;
proto.writeUIntBE = writeUIntBE;
proto.writeUInt32BE = writeUInt32BE;
proto.writeUInt16BE = writeUInt16BE;
proto.writeIntLE = writeIntLE;
proto.writeInt32LE = writeInt32LE;
proto.writeInt16LE = writeInt16LE;
proto.writeInt8 = writeInt8;
proto.writeIntBE = writeIntBE;
proto.writeInt32BE = writeInt32BE;
proto.writeInt16BE = writeInt16BE;

proto.readFloatLE = bigEndian ? readFloatBackwards : readFloatForwards;
proto.readFloatBE = bigEndian ? readFloatForwards : readFloatBackwards;
proto.readDoubleLE = bigEndian ? readDoubleBackwards : readDoubleForwards;
proto.readDoubleBE = bigEndian ? readDoubleForwards : readDoubleBackwards;
proto.writeFloatLE = bigEndian ? writeFloatBackwards : writeFloatForwards;
proto.writeFloatBE = bigEndian ? writeFloatForwards : writeFloatBackwards;
proto.writeDoubleLE = bigEndian ? writeDoubleBackwards : writeDoubleForwards;
proto.writeDoubleBE = bigEndian ? writeDoubleForwards : writeDoubleBackwards;

proto.asciiSlice = asciiSlice;
proto.base64Slice = base64Slice;
proto.latin1Slice = latin1Slice;
proto.hexSlice = hexSlice;
proto.ucs2Slice = ucs2Slice;
proto.utf8Slice = utf8Slice;
proto.asciiWrite = asciiWrite;
proto.base64Write = base64Write;
proto.latin1Write = latin1Write;
proto.hexWrite = hexWrite;
proto.ucs2Write = ucs2Write;
proto.utf8Write = utf8Write;
}

module.exports = {
setupBufferJS,
// Container to export all read write functions.
readWrites: {
readUIntLE,
readUInt32LE,
readUInt16LE,
readUInt8,
readUIntBE,
readUInt32BE,
readUInt16BE,
readIntLE,
readInt32LE,
readInt16LE,
readInt8,
readIntBE,
readInt32BE,
readInt16BE,
writeUIntLE,
writeUInt32LE,
writeUInt16LE,
writeUInt8,
writeUIntBE,
writeUInt32BE,
writeUInt16BE,
writeIntLE,
writeInt32LE,
writeInt16LE,
writeInt8,
writeIntBE,
writeInt32BE,
writeInt16BE,
readFloatLE: bigEndian ? readFloatBackwards : readFloatForwards,
readFloatBE: bigEndian ? readFloatForwards : readFloatBackwards,
readDoubleLE: bigEndian ? readDoubleBackwards : readDoubleForwards,
readDoubleBE: bigEndian ? readDoubleForwards : readDoubleBackwards,
writeFloatLE: bigEndian ? writeFloatBackwards : writeFloatForwards,
writeFloatBE: bigEndian ? writeFloatForwards : writeFloatBackwards,
writeDoubleLE: bigEndian ? writeDoubleBackwards : writeDoubleForwards,
writeDoubleBE: bigEndian ? writeDoubleForwards : writeDoubleBackwards
}
FastBuffer,
addBufferPrototypeMethods
};
56 changes: 28 additions & 28 deletions src/node_buffer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1057,38 +1057,12 @@ static void EncodeUtf8String(const FunctionCallbackInfo<Value>& args) {
}


// pass Buffer object to load prototype methods
void SetupBufferJS(const FunctionCallbackInfo<Value>& args) {
void SetBufferPrototype(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

CHECK(args[0]->IsObject());
Local<Object> proto = args[0].As<Object>();
env->set_buffer_prototype_object(proto);

env->SetMethodNoSideEffect(proto, "asciiSlice", StringSlice<ASCII>);
env->SetMethodNoSideEffect(proto, "base64Slice", StringSlice<BASE64>);
env->SetMethodNoSideEffect(proto, "latin1Slice", StringSlice<LATIN1>);
env->SetMethodNoSideEffect(proto, "hexSlice", StringSlice<HEX>);
env->SetMethodNoSideEffect(proto, "ucs2Slice", StringSlice<UCS2>);
env->SetMethodNoSideEffect(proto, "utf8Slice", StringSlice<UTF8>);

env->SetMethod(proto, "asciiWrite", StringWrite<ASCII>);
env->SetMethod(proto, "base64Write", StringWrite<BASE64>);
env->SetMethod(proto, "latin1Write", StringWrite<LATIN1>);
env->SetMethod(proto, "hexWrite", StringWrite<HEX>);
env->SetMethod(proto, "ucs2Write", StringWrite<UCS2>);
env->SetMethod(proto, "utf8Write", StringWrite<UTF8>);

if (auto zero_fill_field = env->isolate_data()->zero_fill_field()) {
CHECK(args[1]->IsObject());
auto binding_object = args[1].As<Object>();
auto array_buffer = ArrayBuffer::New(env->isolate(),
zero_fill_field,
sizeof(*zero_fill_field));
auto name = FIXED_ONE_BYTE_STRING(env->isolate(), "zeroFill");
auto value = Uint32Array::New(array_buffer, 0, 1);
CHECK(binding_object->Set(env->context(), name, value).FromJust());
}
}


Expand All @@ -1098,7 +1072,7 @@ void Initialize(Local<Object> target,
void* priv) {
Environment* env = Environment::GetCurrent(context);

env->SetMethod(target, "setupBufferJS", SetupBufferJS);
env->SetMethod(target, "setBufferPrototype", SetBufferPrototype);
env->SetMethodNoSideEffect(target, "createFromString", CreateFromString);

env->SetMethodNoSideEffect(target, "byteLengthUtf8", ByteLengthUtf8);
Expand All @@ -1123,6 +1097,32 @@ void Initialize(Local<Object> target,
target->Set(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "kStringMaxLength"),
Integer::New(env->isolate(), String::kMaxLength)).FromJust();

env->SetMethodNoSideEffect(target, "asciiSlice", StringSlice<ASCII>);
env->SetMethodNoSideEffect(target, "base64Slice", StringSlice<BASE64>);
env->SetMethodNoSideEffect(target, "latin1Slice", StringSlice<LATIN1>);
env->SetMethodNoSideEffect(target, "hexSlice", StringSlice<HEX>);
env->SetMethodNoSideEffect(target, "ucs2Slice", StringSlice<UCS2>);
env->SetMethodNoSideEffect(target, "utf8Slice", StringSlice<UTF8>);

env->SetMethod(target, "asciiWrite", StringWrite<ASCII>);
env->SetMethod(target, "base64Write", StringWrite<BASE64>);
env->SetMethod(target, "latin1Write", StringWrite<LATIN1>);
env->SetMethod(target, "hexWrite", StringWrite<HEX>);
env->SetMethod(target, "ucs2Write", StringWrite<UCS2>);
env->SetMethod(target, "utf8Write", StringWrite<UTF8>);

// It can be a nullptr when running inside an isolate where we
// do not own the ArrayBuffer allocator.
if (uint32_t* zero_fill_field = env->isolate_data()->zero_fill_field()) {
Local<ArrayBuffer> array_buffer = ArrayBuffer::New(
env->isolate(), zero_fill_field, sizeof(*zero_fill_field));
CHECK(target
->Set(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "zeroFill"),
Uint32Array::New(array_buffer, 0, 1))
.FromJust());
}
}

} // anonymous namespace
Expand Down
1 change: 1 addition & 0 deletions src/node_internals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ v8::MaybeLocal<v8::Uint8Array> New(Environment* env,
size_t byte_offset,
size_t length) {
v8::Local<v8::Uint8Array> ui = v8::Uint8Array::New(ab, byte_offset, length);
CHECK(!env->buffer_prototype_object().IsEmpty());
v8::Maybe<bool> mb =
ui->SetPrototype(env->context(), env->buffer_prototype_object());
if (mb.IsNothing())
Expand Down