Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

buffer: slow buffer copy compatibility fix #4639

Closed
trevnorris wants to merge 3 commits into from
Closed

buffer: slow buffer copy compatibility fix #4639

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
a0ed4ca
buffer: slow buffer copy compatibility fix
trevnorris Jan 22, 2013
bb24d84
buffer: cleanup copy() asserts and tests
trevnorris Jan 22, 2013
2175b9e
buffer: error and misc cleanup
trevnorris Jan 22, 2013
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
3 changes: 3 additions & 0 deletions doc/api/buffer.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ Does copy between buffers. The source and target regions can be overlapped.
`targetStart` and `sourceStart` default to `0`.
`sourceEnd` defaults to `buffer.length`.

All values passed that are `undefined`/`NaN` or are out of bounds are set equal
to their respective defaults.

Example: build two Buffers, then copy `buf1` from byte 16 through byte 19
into `buf2`, starting at the 8th byte in `buf2`.

Expand Down
98 changes: 46 additions & 52 deletions lib/buffer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ SlowBuffer.prototype.toString = function(encoding, start, end) {
return this.ucs2Slice(start, end);

default:
throw new Error('Unknown encoding: ' + encoding);
throw new TypeError('Unknown encoding: ' + encoding);
}
};

Expand All @@ -104,14 +104,14 @@ SlowBuffer.prototype.hexWrite = function(string, offset, length) {
// must be an even number of digits
var strLen = string.length;
if (strLen % 2) {
throw new Error('Invalid hex string');
throw new TypeError('Invalid hex string');
}
if (length > strLen / 2) {
length = strLen / 2;
}
for (var i = 0; i < length; i++) {
var byte = parseInt(string.substr(i * 2, 2), 16);
if (isNaN(byte)) throw new Error('Invalid hex string');
if (isNaN(byte)) throw new TypeError('Invalid hex string');
this[offset + i] = byte;
}
SlowBuffer._charsWritten = i * 2;
Expand Down Expand Up @@ -170,7 +170,7 @@ SlowBuffer.prototype.write = function(string, offset, length, encoding) {
return this.ucs2Write(string, offset, length);

default:
throw new Error('Unknown encoding: ' + encoding);
throw new TypeError('Unknown encoding: ' + encoding);
}
};

Expand All @@ -180,10 +180,10 @@ SlowBuffer.prototype.slice = function(start, end) {
if (end === undefined) end = this.length;

if (end > this.length) {
throw new Error('oob');
throw new RangeError('end > this.length');
}
if (start > end) {
throw new Error('oob');
throw new RangeError('start > end');
}

return new Buffer(this, end - start, +start);
Expand Down Expand Up @@ -212,7 +212,7 @@ function Buffer(subject, encoding, offset) {
// Are we slicing?
if (typeof offset === 'number') {
if (!Buffer.isBuffer(subject)) {
throw new Error('First argument must be a Buffer when slicing');
throw new TypeError('First argument must be a Buffer when slicing');
}

this.length = coerce(encoding);
Expand All @@ -234,8 +234,8 @@ function Buffer(subject, encoding, offset) {
break;

default:
throw new Error('First argument needs to be a number, ' +
'array or string.');
throw new TypeError('First argument needs to be a number, ' +
'array or string.');
}

if (this.length > Buffer.poolSize) {
Expand Down Expand Up @@ -336,15 +336,17 @@ Buffer.prototype.inspect = function inspect() {
};


Buffer.prototype.get = function get(i) {
if (i < 0 || i >= this.length) throw new Error('oob');
return this.parent[this.offset + i];
Buffer.prototype.get = function get(offset) {
if (offset < 0 || offset >= this.length)
throw new RangeError('offset is out of bounds');
return this.parent[this.offset + offset];
};


Buffer.prototype.set = function set(i, v) {
if (i < 0 || i >= this.length) throw new Error('oob');
return this.parent[this.offset + i] = v;
Buffer.prototype.set = function set(offset, v) {
if (offset < 0 || offset >= this.length)
throw new RangeError('offset is out of bounds');
return this.parent[this.offset + offset] = v;
};


Expand Down Expand Up @@ -408,7 +410,7 @@ Buffer.prototype.write = function(string, offset, length, encoding) {
break;

default:
throw new Error('Unknown encoding: ' + encoding);
throw new TypeError('Unknown encoding: ' + encoding);
}

Buffer._charsWritten = SlowBuffer._charsWritten;
Expand Down Expand Up @@ -465,7 +467,7 @@ Buffer.prototype.toString = function(encoding, start, end) {
return this.parent.ucs2Slice(start, end);

default:
throw new Error('Unknown encoding: ' + encoding);
throw new TypeError('Unknown encoding: ' + encoding);
}
};

Expand All @@ -484,21 +486,21 @@ Buffer.prototype.fill = function fill(value, start, end) {
value = value.charCodeAt(0);
}
if (!(typeof value === 'number') || isNaN(value)) {
throw new Error('value is not a number');
throw new TypeError('value is not a number');
}

if (end < start) throw new Error('end < start');
if (end < start) throw new RangeError('end < start');

// Fill 0 bytes; we're done
if (end === start) return 0;
if (this.length == 0) return 0;

if (start < 0 || start >= this.length) {
throw new Error('start out of bounds');
throw new RangeError('start out of bounds');
}

if (end < 0 || end > this.length) {
throw new Error('end out of bounds');
throw new RangeError('end out of bounds');
}

return this.parent.fill(value,
Expand All @@ -509,7 +511,7 @@ Buffer.prototype.fill = function fill(value, start, end) {

Buffer.concat = function(list, length) {
if (!Array.isArray(list)) {
throw new Error('Usage: Buffer.concat(list, [length])');
throw new TypeError('Usage: Buffer.concat(list, [length])');
}

if (list.length === 0) {
Expand Down Expand Up @@ -541,40 +543,29 @@ Buffer.concat = function(list, length) {

// copy(targetBuffer, targetStart=0, sourceStart=0, sourceEnd=buffer.length)
Buffer.prototype.copy = function(target, target_start, start, end) {
var source = this;
start || (start = 0);
end || (end = this.length);
target_start || (target_start = 0);

if (end < start) throw new Error('sourceEnd < sourceStart');
// set undefined/NaN or out of bounds values equal to their default
if (!(target_start >= 0)) target_start = 0;
if (!(start >= 0)) start = 0;
if (!(end < this.length)) end = this.length;

// Copy 0 bytes; we're done
if (end === start) return 0;
if (target.length == 0 || source.length == 0) return 0;
if (end === start ||
target.length === 0 ||
this.length === 0 ||
start > this.length)
return 0;

if (target_start < 0 || target_start >= target.length) {
throw new Error('targetStart out of bounds');
}
if (end < start)
throw new RangeError('sourceEnd < sourceStart');

if (start < 0 || start >= source.length) {
throw new Error('sourceStart out of bounds');
}
if (target_start >= target.length)
throw new RangeError('targetStart out of bounds');

if (end < 0 || end > source.length) {
throw new Error('sourceEnd out of bounds');
}

// Are we oob?
if (end > this.length) {
end = this.length;
}

if (target.length - target_start < end - start) {
if (target.length - target_start < end - start)
end = target.length - target_start + start;
}

return this.parent.copy(target.parent,
target_start + target.offset,
return this.parent.copy(target.parent || target,
target_start + (target.offset || 0),
start + this.offset,
end + this.offset);
};
Expand All @@ -583,9 +574,12 @@ Buffer.prototype.copy = function(target, target_start, start, end) {
// slice(start, end)
Buffer.prototype.slice = function(start, end) {
if (end === undefined) end = this.length;
if (end > this.length) throw new Error('oob');
if (start > end) throw new Error('oob');
if (start < 0) throw new Error('start out of bounds');
if (end > this.length)
throw new RangeError('end > this.length');
if (start > end)
throw new RangeError('start > end');
if (start < 0)
throw new RangeError('start < 0');
return new Buffer(this.parent, end - start, +start + this.offset);
};

Expand Down
59 changes: 22 additions & 37 deletions src/node_buffer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,21 +395,19 @@ Handle<Value> Buffer::Copy(const Arguments &args) {
Buffer *source = ObjectWrap::Unwrap<Buffer>(args.This());

if (!Buffer::HasInstance(args[0])) {
return ThrowException(Exception::TypeError(String::New(
"First arg should be a Buffer")));
return ThrowTypeError("First arg should be a Buffer");
}

Local<Object> target = args[0]->ToObject();
char* target_data = Buffer::Data(target);
size_t target_length = Buffer::Length(target);
size_t target_start = args[1]->Uint32Value();
size_t source_start = args[2]->Uint32Value();
size_t source_end = args[3]->IsUint32() ? args[3]->Uint32Value()
: source->length_;
size_t target_start = args[1]->IsUndefined() ? 0 : args[1]->Uint32Value();
Copy link
Member

@bnoordhuis bnoordhuis Jan 25, 2013

Choose a reason for hiding this comment

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

Small note: I wonder if calling IsUndefined() first is a minor deoptimization. Uint32Value() is only expensive when the value is not an SMI but that should be relatively rare.

size_t source_start = args[2]->IsUndefined() ? 0 : args[2]->Uint32Value();
size_t source_end = args[3]->IsUndefined() ? source->length_
: args[3]->Uint32Value();

if (source_end < source_start) {
return ThrowException(Exception::Error(String::New(
"sourceEnd < sourceStart")));
return ThrowRangeError("sourceEnd < sourceStart");
}

// Copy 0 bytes; we're done
Expand All @@ -418,18 +416,15 @@ Handle<Value> Buffer::Copy(const Arguments &args) {
}

if (target_start >= target_length) {
return ThrowException(Exception::Error(String::New(
"targetStart out of bounds")));
return ThrowRangeError("targetStart out of bounds");
}

if (source_start >= source->length_) {
return ThrowException(Exception::Error(String::New(
"sourceStart out of bounds")));
return ThrowRangeError("sourceStart out of bounds");
}

if (source_end > source->length_) {
return ThrowException(Exception::Error(String::New(
"sourceEnd out of bounds")));
return ThrowRangeError("sourceEnd out of bounds");
}

size_t to_copy = MIN(MIN(source_end - source_start,
Expand Down Expand Up @@ -468,8 +463,7 @@ Handle<Value> Buffer::Utf8Write(const Arguments &args) {
}

if (length > 0 && offset >= buffer->length_) {
return ThrowException(Exception::TypeError(String::New(
"Offset is out of bounds")));
return ThrowTypeError("Offset is out of bounds");
}

size_t max_length = args[2]->IsUndefined() ? buffer->length_ - offset
Expand Down Expand Up @@ -500,8 +494,7 @@ Handle<Value> Buffer::Ucs2Write(const Arguments &args) {
Buffer *buffer = ObjectWrap::Unwrap<Buffer>(args.This());

if (!args[0]->IsString()) {
return ThrowException(Exception::TypeError(String::New(
"Argument must be a string")));
return ThrowTypeError("Argument must be a string");
}

Local<String> s = args[0]->ToString();
Expand Down Expand Up @@ -539,17 +532,15 @@ Handle<Value> Buffer::AsciiWrite(const Arguments &args) {
Buffer *buffer = ObjectWrap::Unwrap<Buffer>(args.This());

if (!args[0]->IsString()) {
return ThrowException(Exception::TypeError(String::New(
"Argument must be a string")));
return ThrowTypeError("Argument must be a string");
}

Local<String> s = args[0]->ToString();
size_t length = s->Length();
size_t offset = args[1]->Int32Value();

if (length > 0 && offset >= buffer->length_) {
return ThrowException(Exception::TypeError(String::New(
"Offset is out of bounds")));
return ThrowTypeError("Offset is out of bounds");
}

size_t max_length = args[2]->IsUndefined() ? buffer->length_ - offset
Expand Down Expand Up @@ -578,8 +569,7 @@ Handle<Value> Buffer::Base64Write(const Arguments &args) {
Buffer *buffer = ObjectWrap::Unwrap<Buffer>(args.This());

if (!args[0]->IsString()) {
return ThrowException(Exception::TypeError(String::New(
"Argument must be a string")));
return ThrowTypeError("Argument must be a string");
}

String::AsciiValue s(args[0]);
Expand All @@ -590,8 +580,7 @@ Handle<Value> Buffer::Base64Write(const Arguments &args) {
max_length = MIN(length, MIN(buffer->length_ - offset, max_length));

if (max_length && offset >= buffer->length_) {
return ThrowException(Exception::TypeError(String::New(
"Offset is out of bounds")));
return ThrowTypeError("Offset is out of bounds");
}

char a, b, c, d;
Expand Down Expand Up @@ -643,17 +632,15 @@ Handle<Value> Buffer::BinaryWrite(const Arguments &args) {
Buffer *buffer = ObjectWrap::Unwrap<Buffer>(args.This());

if (!args[0]->IsString()) {
return ThrowException(Exception::TypeError(String::New(
"Argument must be a string")));
return ThrowTypeError("Argument must be a string");
}

Local<String> s = args[0]->ToString();
size_t length = s->Length();
size_t offset = args[1]->Int32Value();

if (s->Length() > 0 && offset >= buffer->length_) {
return ThrowException(Exception::TypeError(String::New(
"Offset is out of bounds")));
return ThrowTypeError("Offset is out of bounds");
}

char *p = (char*)buffer->data_ + offset;
Expand Down Expand Up @@ -793,8 +780,7 @@ Handle<Value> Buffer::ByteLength(const Arguments &args) {
HandleScope scope;

if (!args[0]->IsString()) {
return ThrowException(Exception::TypeError(String::New(
"Argument must be a string")));
return ThrowTypeError("Argument must be a string");
}

Local<String> s = args[0]->ToString();
Expand All @@ -808,8 +794,7 @@ Handle<Value> Buffer::MakeFastBuffer(const Arguments &args) {
HandleScope scope;

if (!Buffer::HasInstance(args[0])) {
return ThrowException(Exception::TypeError(String::New(
"First argument must be a Buffer")));
return ThrowTypeError("First argument must be a Buffer");
}

Buffer *buffer = ObjectWrap::Unwrap<Buffer>(args[0]->ToObject());
Expand Down Expand Up @@ -838,9 +823,9 @@ Handle<Value> Buffer::MakeFastBuffer(const Arguments &args) {
}


bool Buffer::HasInstance(v8::Handle<v8::Value> val) {
bool Buffer::HasInstance(Handle<Value> val) {
if (!val->IsObject()) return false;
v8::Local<v8::Object> obj = val->ToObject();
Local<Object> obj = val->ToObject();

if (obj->GetIndexedPropertiesExternalArrayDataType() == kExternalUnsignedByteArray)
return true;
Expand All @@ -853,7 +838,7 @@ bool Buffer::HasInstance(v8::Handle<v8::Value> val) {
}


class RetainedBufferInfo: public v8::RetainedObjectInfo {
class RetainedBufferInfo: public RetainedObjectInfo {
public:
RetainedBufferInfo(Buffer* buffer);
virtual void Dispose();
Expand Down
Loading