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.

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

buffer: add indexOf method #160

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions doc/api/buffer.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,13 @@ buffer.
var b = new Buffer(50);
b.fill("h");

### buf.indexOf(value[, fromIndex])
Copy link
Contributor

Choose a reason for hiding this comment

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

Possibly something like byteOffset would be better than fromIndex. The latter makes it sound like the character placement, but that's incorrect for multi-byte strings.


* `value` Buffer or String or Number
Copy link
Contributor

Choose a reason for hiding this comment

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

Following Buffer instantiation standards, should also accept an array.

* `fromIndex` Number, Optional, Default: 0

Finds the index within the buffer of the first occurrence of the specified value, starting the search at fromIndex. Returns -1 if the value is not found.

## buffer.INSPECT_MAX_BYTES

* Number, Default: 50
Expand Down
10 changes: 10 additions & 0 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,16 @@ Buffer.prototype.fill = function fill(val, start, end) {
return this;
};

Buffer.prototype.indexOf = function indexOf(needle, pos) {
if (typeof needle === 'number') {
needle = new Buffer([needle]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Allocating a buffer for a single byte seems too much. Also it's unclear that only one byte numbers are accepted. What if I want to search for 16 byte number? How to specify endianness?

Copy link
Author

Choose a reason for hiding this comment

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

This intentionally follows the semantics of Array.prototype.indexOf.

It can thus only search for an element of the array. If you wanted to search for a sequence of elements (bytes), you can supply a Buffer as the needle.

Copy link
Contributor

Choose a reason for hiding this comment

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

@vkurchatkin As I understand it, the backing slow buffer will have already been allocated, so this shouldn't be too bad of an allocation.

We could amortize the cost of repeated calls by having a statically available single byte buffer into which we swap the value, which might be a nice optimization.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking about special functions on internal for each case

Copy link
Author

Choose a reason for hiding this comment

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

If we go with a hybrid interface, I also think different specialised functions would be a cleaner approach.

} else if (typeof needle === 'string') {
needle = new Buffer(needle);
} else if (!(needle instanceof Buffer)) {
throw new TypeError('Argument must be a Buffer, number or string');
}
return internal.indexOf(this, needle, pos);
};

// XXX remove in v0.13
Buffer.prototype.get = util.deprecate(function get(offset) {
Expand Down
50 changes: 50 additions & 0 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <limits.h>

#define MIN(a, b) ((a) < (b) ? (a) : (b))
#define MAX(a, b) ((a) > (b) ? (a) : (b))

#define CHECK_NOT_OOB(r) \
do { \
Expand Down Expand Up @@ -585,6 +586,54 @@ void Compare(const FunctionCallbackInfo<Value> &args) {
args.GetReturnValue().Set(val);
}

void IndexOf(const FunctionCallbackInfo<Value> &args) {
Local<Object> obj = args[0]->ToObject();
char* obj_data =
static_cast<char*>(obj->GetIndexedPropertiesExternalArrayData());
int32_t obj_length = obj->GetIndexedPropertiesExternalArrayDataLength();

Local<Object> search = args[1]->ToObject();
char* search_data =
static_cast<char*>(search->GetIndexedPropertiesExternalArrayData());
int32_t search_length = search->GetIndexedPropertiesExternalArrayDataLength();

int32_t pos = args[2]->Int32Value();
int32_t start = MIN(MAX(pos, 0), obj_length);

if (search_length == 0) {
return args.GetReturnValue().Set(start);
}

while (search_length <= obj_length - start) {
// Search for the first byte of the needle.
char *chr = reinterpret_cast<char *>(
Copy link
Contributor

Choose a reason for hiding this comment

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

style: char* chr = reinterpret_cast<char*>(

memchr(&obj_data[start], search_data[0], obj_length - start));
int32_t chrpos = (intptr_t)chr - (intptr_t)obj_data;
if (chr == NULL) {
// First byte not found, short circuit.
return args.GetReturnValue().Set(-1);
}
if (search_length == 1) {
// Nothing more to compare, we found it.
return args.GetReturnValue().Set(chrpos);
}
if (search_length > obj_length - chrpos) {
// Needle is longer than the rest of the haystack,
// no way it is contained in there.
return args.GetReturnValue().Set(-1);
}
int cmp = memcmp(&chr[1], &search_data[1], search_length - 1);
if (cmp == 0) {
// All bytes are equal, we found it.
return args.GetReturnValue().Set(chrpos);
}
// Advance start position for next iteration.
start = chrpos + 1;
}

return args.GetReturnValue().Set(-1);
}


// pass Buffer object to load prototype methods
void SetupBufferJS(const FunctionCallbackInfo<Value>& args) {
Expand Down Expand Up @@ -629,6 +678,7 @@ void SetupBufferJS(const FunctionCallbackInfo<Value>& args) {
env->SetMethod(internal, "byteLength", ByteLength);
env->SetMethod(internal, "compare", Compare);
env->SetMethod(internal, "fill", Fill);
env->SetMethod(internal, "indexOf", IndexOf);

env->SetMethod(internal, "readDoubleBE", ReadDoubleBE);
env->SetMethod(internal, "readDoubleLE", ReadDoubleLE);
Expand Down
10 changes: 10 additions & 0 deletions test/simple/test-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1184,3 +1184,13 @@ assert.throws(function() {
var b = new Buffer(1);
b.equals('abc');
});

// IndexOf Tests
assert.equal(Buffer('abc').indexOf(''), 0)
assert.equal(Buffer('abc').indexOf('bd'), -1)
assert.equal(Buffer('abc').indexOf('bc'), 1)
assert.equal(Buffer('abc').indexOf(0x62), 1)
assert.equal(Buffer('abc').indexOf(Buffer('bc')), 1)
assert.equal(Buffer('abc').indexOf(Buffer([0x62,0x63])), 1)
assert.equal(Buffer('abc').indexOf('bc', 1), 1)
assert.equal(Buffer('abc').indexOf('bc', 2), -1)