Skip to content

Commit

Permalink
Don't extend Buffer prototype by default.
Browse files Browse the repository at this point in the history
Make extension of Buffer.prototype opt-in.  The rationale for that is
that some of the buffertools methods conflict with the Buffer methods
of the same name.  Users now have to call `buffertools.extend()` if
they want the old behavior.

Bump the version number to 2.0.0 because this is a breaking change to
the API.
  • Loading branch information
bnoordhuis committed Dec 19, 2013
1 parent 89144c3 commit 20b1921
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 60 deletions.
52 changes: 40 additions & 12 deletions README.md
Expand Up @@ -15,19 +15,41 @@ From source:

Now you can include the module in your project.

require('buffertools');
new Buffer(42).clear();
require('buffertools').extend(); // extend Buffer.prototype
var buf = new Buffer(42); // create a 42 byte buffer
buf.clear(); // clear it!

If you don't want to extend the Buffer class's prototype (recommended):

var buffertools = require('buffertools');
var buf = new Buffer(42);
buffertools.clear(buf);

## Methods

Note that most methods that take a buffer as an argument, will also accept a string.

### Buffer.clear()
### buffertools.extend([object], [object...])

Extend the arguments with the buffertools methods. If called without arguments,
defaults to `[Buffer.prototype, SlowBuffer.prototype]`. Extending prototypes
only makes sense for classes that derive from `Buffer`.

buffertools v1.x extended the `Buffer` prototype by default. In v2.x, it is
opt-in. The reason for that is that buffertools was originally developed for
node.js v0.3 (or maybe v0.2, I don't remember exactly when buffers were added)
where the `Buffer` class was devoid of any useful methods. Over the years, it
has grown a number of utility methods, some of which conflict with the
buffertools methods of the same name, like `Buffer#fill()`.

### Buffer#clear()
### buffertools.clear(buffer)

Clear the buffer. This is equivalent to `Buffer.fill(0)`.
Clear the buffer. This is equivalent to `Buffer#fill(0)`.
Returns the buffer object so you can chain method calls.

### Buffer.compare(buffer|string)
### Buffer#compare(buffer|string)
### buffertools.compare(buffer, buffer|string)

Lexicographically compare two buffers. Returns a number less than zero
if a < b, zero if a == b or greater than zero if a > b.
Expand All @@ -38,7 +60,7 @@ the same binary data.
Smaller buffers are considered to be less than larger ones. Some buffers
find this hurtful.

### Buffer.concat(a, b, c, ...)
### Buffer#concat(a, b, c, ...)
### buffertools.concat(a, b, c, ...)

Concatenate two or more buffers/strings and return the result. Example:
Expand All @@ -52,7 +74,8 @@ Concatenate two or more buffers/strings and return the result. Example:
// static variant
buffertools.concat('foo', new Buffer('bar'), 'baz');

### Buffer.equals(buffer|string)
### Buffer#equals(buffer|string)
### buffertools.equals(buffer, buffer|string)

Returns true if this buffer equals the argument, false otherwise.

Expand All @@ -62,31 +85,36 @@ the same binary data.
Caveat emptor: If your buffers contain strings with different character encodings,
they will most likely *not* be equal.

### Buffer.fill(integer|string|buffer)
### Buffer#fill(integer|string|buffer)
### buffertools.fill(buffer, integer|string|buffer)

Fill the buffer (repeatedly if necessary) with the argument.
Returns the buffer object so you can chain method calls.

### Buffer.fromHex()
### Buffer#fromHex()
### buffertools.fromHex(buffer)

Assumes this buffer contains hexadecimal data (packed, no whitespace)
and decodes it into binary data. Returns a new buffer with the decoded
content. Throws an exception if non-hexadecimal data is encountered.

### Buffer.indexOf(buffer|string, [start=0])
### Buffer#indexOf(buffer|string, [start=0])
### buffertools.indexOf(buffer, buffer|string, [start=0])

Search this buffer for the first occurrence of the argument, starting at
offset `start`. Returns the zero-based index or -1 if there is no match.

### Buffer.reverse()
### Buffer#reverse()
### buffertools.reverse(buffer)

Reverse the content of the buffer in place. Example:

b = new Buffer('live');
b.reverse();
console.log(b); // "evil"

### Buffer.toHex()
### Buffer#toHex()
### buffertools.toHex(buffer)

Returns the contents of this buffer encoded as a hexadecimal string.

Expand Down
102 changes: 67 additions & 35 deletions buffertools.cc
Expand Up @@ -31,41 +31,73 @@ namespace {

// this is an application of the Curiously Recurring Template Pattern
template <class Derived> struct UnaryAction {
Handle<Value> apply(Handle<Object>& buffer, const Arguments& args, HandleScope& scope);
Handle<Value> apply(
Handle<Object>& buffer,
const Arguments& args,
uint32_t args_start);

Handle<Value> operator()(const Arguments& args) {
HandleScope scope;

Local<Object> self = args.This();
if (!Buffer::HasInstance(self)) {
uint32_t args_start = 0;
Local<Object> target = args.This();
if (Buffer::HasInstance(target)) {
// Invoked as prototype method, no action required.
} else if (Buffer::HasInstance(args[0])) {
// First argument is the target buffer.
args_start = 1;
target = args[0]->ToObject();
} else {
return ThrowException(Exception::TypeError(String::New(
"Argument should be a buffer object.")));
}

return static_cast<Derived*>(this)->apply(self, args, scope);
return scope.Close(static_cast<Derived*>(this)->apply(target, args, args_start));
}
};

template <class Derived> struct BinaryAction {
Handle<Value> apply(Handle<Object>& buffer, const uint8_t* data, size_t size, const Arguments& args, HandleScope& scope);
Handle<Value> apply(
Handle<Object>& buffer,
const uint8_t* data,
size_t size,
const Arguments& args,
uint32_t args_start);

Handle<Value> operator()(const Arguments& args) {
HandleScope scope;

Local<Object> self = args.This();
if (!Buffer::HasInstance(self)) {
uint32_t args_start = 0;
Local<Object> target = args.This();
if (Buffer::HasInstance(target)) {
// Invoked as prototype method, no action required.
} else if (Buffer::HasInstance(args[0])) {
// First argument is the target buffer.
args_start = 1;
target = args[0]->ToObject();
} else {
return ThrowException(Exception::TypeError(String::New(
"Argument should be a buffer object.")));
}

if (args[0]->IsString()) {
String::Utf8Value s(args[0]->ToString());
return static_cast<Derived*>(this)->apply(self, (uint8_t*) *s, s.length(), args, scope);
if (args[args_start]->IsString()) {
String::Utf8Value s(args[args_start]);
return scope.Close(static_cast<Derived*>(this)->apply(
target,
(const uint8_t*) *s,
s.length(),
args,
args_start));
}
if (Buffer::HasInstance(args[0])) {
Local<Object> other = args[0]->ToObject();
return static_cast<Derived*>(this)->apply(
self, (const uint8_t*) Buffer::Data(other), Buffer::Length(other), args, scope);

if (Buffer::HasInstance(args[args_start])) {
Local<Object> other = args[args_start]->ToObject();
return scope.Close(static_cast<Derived*>(this)->apply(
target,
(const uint8_t*) Buffer::Data(other),
Buffer::Length(other),
args,
args_start));
}

Local<String> illegalArgumentException = String::New(
Expand Down Expand Up @@ -116,25 +148,25 @@ int compare(Handle<Object>& buffer, const uint8_t* data2, size_t length2) {
// actions
//
struct ClearAction: UnaryAction<ClearAction> {
Handle<Value> apply(Handle<Object>& buffer, const Arguments& args, HandleScope& scope) {
Handle<Value> apply(Handle<Object>& buffer, const Arguments& args, uint32_t args_start) {
return clear(buffer, 0);
}
};

struct FillAction: UnaryAction<FillAction> {
Handle<Value> apply(Handle<Object>& buffer, const Arguments& args, HandleScope& scope) {
if (args[0]->IsInt32()) {
int c = args[0]->ToInt32()->Int32Value();
Handle<Value> apply(Handle<Object>& buffer, const Arguments& args, uint32_t args_start) {
if (args[args_start]->IsInt32()) {
int c = args[args_start]->Int32Value();
return clear(buffer, c);
}

if (args[0]->IsString()) {
String::Utf8Value s(args[0]->ToString());
if (args[args_start]->IsString()) {
String::Utf8Value s(args[args_start]);
return fill(buffer, *s, s.length());
}

if (Buffer::HasInstance(args[0])) {
Handle<Object> other = args[0]->ToObject();
if (Buffer::HasInstance(args[args_start])) {
Handle<Object> other = args[args_start]->ToObject();
size_t length = Buffer::Length(other);
uint8_t* data = (uint8_t*) Buffer::Data(other);
return fill(buffer, data, length);
Expand All @@ -149,7 +181,7 @@ struct FillAction: UnaryAction<FillAction> {
struct ReverseAction: UnaryAction<ReverseAction> {
// O(n/2) for all cases which is okay, might be optimized some more with whole-word swaps
// XXX won't this trash the L1 cache something awful?
Handle<Value> apply(Handle<Object>& buffer, const Arguments& args, HandleScope& scope) {
Handle<Value> apply(Handle<Object>& buffer, const Arguments& args, uint32_t args_start) {
uint8_t* head = (uint8_t*) Buffer::Data(buffer);
uint8_t* tail = head + Buffer::Length(buffer) - 1;

Expand All @@ -166,23 +198,23 @@ struct ReverseAction: UnaryAction<ReverseAction> {
};

struct EqualsAction: BinaryAction<EqualsAction> {
Handle<Value> apply(Handle<Object>& buffer, const uint8_t* data, size_t size, const Arguments& args, HandleScope& scope) {
Handle<Value> apply(Handle<Object>& buffer, const uint8_t* data, size_t size, const Arguments& args, uint32_t args_start) {
return compare(buffer, data, size) == 0 ? True() : False();
}
};

struct CompareAction: BinaryAction<CompareAction> {
Handle<Value> apply(Handle<Object>& buffer, const uint8_t* data, size_t size, const Arguments& args, HandleScope& scope) {
return scope.Close(Integer::New(compare(buffer, data, size)));
Handle<Value> apply(Handle<Object>& buffer, const uint8_t* data, size_t size, const Arguments& args, uint32_t args_start) {
return Integer::New(compare(buffer, data, size));
}
};

struct IndexOfAction: BinaryAction<IndexOfAction> {
Handle<Value> apply(Handle<Object>& buffer, const uint8_t* data2, size_t size2, const Arguments& args, HandleScope& scope) {
Handle<Value> apply(Handle<Object>& buffer, const uint8_t* data2, size_t size2, const Arguments& args, uint32_t args_start) {
const uint8_t* data = (const uint8_t*) Buffer::Data(buffer);
const size_t size = Buffer::Length(buffer);

int32_t start = args[1]->Int32Value();
int32_t start = args[args_start + 1]->Int32Value();

if (start < 0)
start = size - std::min<size_t>(size, -start);
Expand All @@ -193,7 +225,7 @@ struct IndexOfAction: BinaryAction<IndexOfAction> {
data + start, size - start, data2, size2);

const ptrdiff_t offset = p ? (p - data) : -1;
return scope.Close(Integer::New(offset));
return Integer::New(offset);
}
};

Expand All @@ -211,7 +243,7 @@ static char fromHexTable[] = {
-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1
};

inline Handle<Value> decodeHex(const uint8_t* const data, const size_t size, const Arguments& args, HandleScope& scope) {
inline Handle<Value> decodeHex(const uint8_t* const data, const size_t size, const Arguments& args, uint32_t args_start) {
if (size & 1) {
return ThrowException(Exception::Error(String::New(
"Odd string length, this is not hexadecimal data.")));
Expand Down Expand Up @@ -242,15 +274,15 @@ inline Handle<Value> decodeHex(const uint8_t* const data, const size_t size, con
}

struct FromHexAction: UnaryAction<FromHexAction> {
Handle<Value> apply(Handle<Object>& buffer, const Arguments& args, HandleScope& scope) {
Handle<Value> apply(Handle<Object>& buffer, const Arguments& args, uint32_t args_start) {
const uint8_t* data = (const uint8_t*) Buffer::Data(buffer);
size_t length = Buffer::Length(buffer);
return decodeHex(data, length, args, scope);
return decodeHex(data, length, args, args_start);
}
};

struct ToHexAction: UnaryAction<ToHexAction> {
Handle<Value> apply(Handle<Object>& buffer, const Arguments& args, HandleScope& scope) {
Handle<Value> apply(Handle<Object>& buffer, const Arguments& args, uint32_t args_start) {
const size_t size = Buffer::Length(buffer);
const uint8_t* data = (const uint8_t*) Buffer::Data(buffer);

Expand All @@ -265,7 +297,7 @@ struct ToHexAction: UnaryAction<ToHexAction> {
s[i * 2 + 1] = toHexTable[c & 15];
}

return scope.Close(String::New(s.c_str(), s.size()));
return String::New(s.c_str(), s.size());
}
};

Expand Down Expand Up @@ -332,7 +364,7 @@ Handle<Value> Concat(const Arguments& args) {
for (int index = 0, length = args.Length(); index < length; ++index) {
Local<Value> arg = args[index];
if (arg->IsString()) {
String::Utf8Value v(arg->ToString());
String::Utf8Value v(arg);
memcpy(s, *v, v.length());
s += v.length();
}
Expand Down
34 changes: 22 additions & 12 deletions buffertools.js
Expand Up @@ -21,19 +21,29 @@ var Buffer = require('buffer').Buffer;
var events = require('events');
var util = require('util');

// extend object prototypes
for (var key in buffertools) {
var val = buffertools[key];
SlowBuffer.prototype[key] = val;
Buffer.prototype[key] = val;
exports[key] = val;
}

// bug fix, see https://github.com/bnoordhuis/node-buffertools/issues/#issue/6
Buffer.prototype.concat = SlowBuffer.prototype.concat = function() {
var args = [this].concat(Array.prototype.slice.call(arguments));
return buffertools.concat.apply(buffertools, args);
exports.extend = function() {
var receivers;
if (arguments.length > 0) {
receivers = Array.prototype.slice.call(arguments);
} else if (typeof SlowBuffer === 'function') {
receivers = [Buffer.prototype, SlowBuffer.prototype];
} else {
receivers = [Buffer.prototype];
}
for (var i = 0, n = receivers.length; i < n; i += 1) {
var receiver = receivers[i];
for (var key in buffertools) {
receiver[key] = buffertools[key];
}
if (receiver !== exports) {
receiver.concat = function() {
var args = [this].concat(Array.prototype.slice.call(arguments));
return buffertools.concat.apply(buffertools, args);
};
}
}
};
exports.extend(exports);

//
// WritableBufferStream
Expand Down
2 changes: 1 addition & 1 deletion package.json
@@ -1,7 +1,7 @@
{
"name": "buffertools",
"main": "buffertools",
"version": "1.1.1",
"version": "2.0.0",
"keywords": ["buffer", "buffers"],
"description": "Working with node.js buffers made easy.",
"homepage": "https://github.com/bnoordhuis/node-buffertools",
Expand Down
3 changes: 3 additions & 0 deletions test.js
Expand Up @@ -19,6 +19,9 @@ var assert = require('assert');

var WritableBufferStream = buffertools.WritableBufferStream;

// Extend Buffer.prototype and SlowBuffer.prototype.
buffertools.extend();

// these trigger the code paths for UnaryAction and BinaryAction
assert.throws(function() { buffertools.clear({}); });
assert.throws(function() { buffertools.equals({}, {}); });
Expand Down

0 comments on commit 20b1921

Please sign in to comment.