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 #561

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
13 changes: 13 additions & 0 deletions doc/api/buffer.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,19 @@ byte from the original Buffer.
// abc
// !bc


### buf.indexOf(value[, byteOffset])

* `value` String, Buffer or Number
* `byteOffset` Number, Optional, Default: 0
* Return: Number

Operates similar to
[Array#indexOf()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/indexOf).
Copy link
Member

Choose a reason for hiding this comment

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

should probably have been

[`Array#indexOf()`](...)

shouldn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately that removes the code formatting to let the user know that the text is a link. Technically the correct output would have been

`[Array#indexOf()](...)`

to place the <code></code> around the <a></a>, instead of the other way around, but markdown wouldn't be interpreted that way.

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately that removes the code formatting to let the user know that the text is a link.

My point is that there is no code formatting on this text and it shoudl be added, or are you pointing to a deficiency in our styles that should be fixed?

grep '\[' doc/api/*` says that we're using this pattern a lot already and I don't really see a problem.

Copy link
Member

Choose a reason for hiding this comment

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

grep '\[`' doc/api/*

that was supposed to be

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes there is an issue with the styling. If written as

[`Array#indexOf()`](...)

then there is no visual queue that the text is a link. So I intentionally didn't write it that way.

Copy link
Member

Choose a reason for hiding this comment

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

/cc @chrisdickinson this is a problem in our styling then

Accepts a String, Buffer or Number. Strings are interpreted as UTF8. Buffers
will use the entire buffer. So in order to compare a partial Buffer use
`Buffer#slice()`. Numbers can range from 0 to 255.

### buf.readUInt8(offset[, noAssert])

* `offset` Number
Expand Down
18 changes: 18 additions & 0 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,24 @@ Buffer.prototype.compare = function compare(b) {
};


Buffer.prototype.indexOf = function indexOf(val, byteOffset) {
if (byteOffset > 0x7fffffff)
byteOffset = 0x7fffffff;
else if (byteOffset < -0x80000000)
byteOffset = -0x80000000;
byteOffset >>= 0;

if (typeof val === 'string')
return binding.indexOfString(this, val, byteOffset);
if (val instanceof Buffer)
return binding.indexOfBuffer(this, val, byteOffset);
if (typeof val === 'number')
Copy link
Contributor

Choose a reason for hiding this comment

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

we should probably throw an exception if val > 255 since it is unlikely to do what folks expect.

Copy link
Contributor

Choose a reason for hiding this comment

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

or just truncate in some way on val > 255, which is what is done for byte values elsewhere such as in buffer.fill() and when passing an array of bytes to the Buffer constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it truncates automatically.

return binding.indexOfNumber(this, val, byteOffset);

throw new TypeError('val must be string, number or Buffer');
};


Buffer.prototype.fill = function fill(val, start, end) {
start = start >> 0;
end = (end === undefined) ? this.length : end >> 0;
Expand Down
116 changes: 116 additions & 0 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,119 @@ void Compare(const FunctionCallbackInfo<Value> &args) {
}


int32_t IndexOf(const char* haystack,
size_t h_length,
const char* needle,
size_t n_length) {
CHECK_GE(h_length, n_length);
// TODO(trevnorris): Implement Boyer-Moore string search algorithm.
for (size_t i = 0; i < h_length - n_length + 1; i++) {
if (haystack[i] == needle[0]) {
if (memcmp(haystack + i, needle, n_length) == 0)
return i;
}
}
return -1;
}


void IndexOfString(const FunctionCallbackInfo<Value>& args) {
ASSERT(args[0]->IsObject());
ASSERT(args[1]->IsString());
ASSERT(args[2]->IsNumber());

ARGS_THIS(args[0].As<Object>());
node::Utf8Value str(args.GetIsolate(), args[1]);
int32_t offset_i32 = args[2]->Int32Value();
uint32_t offset;

if (offset_i32 < 0) {
if (offset_i32 + static_cast<int32_t>(obj_length) < 0)
offset = 0;
else
offset = static_cast<uint32_t>(obj_length + offset_i32);
} else {
offset = static_cast<uint32_t>(offset_i32);
}

if (str.length() == 0 ||
obj_length == 0 ||
(offset != 0 && str.length() + offset <= str.length()) ||
str.length() + offset > obj_length)
return args.GetReturnValue().Set(-1);

int32_t r =
IndexOf(obj_data + offset, obj_length - offset, *str, str.length());
args.GetReturnValue().Set(r == -1 ? -1 : static_cast<int32_t>(r + offset));
}


void IndexOfBuffer(const FunctionCallbackInfo<Value>& args) {
ASSERT(args[0]->IsObject());
ASSERT(args[1]->IsObject());
ASSERT(args[2]->IsNumber());

ARGS_THIS(args[0].As<Object>());
Local<Object> buf = args[1].As<Object>();
int32_t offset_i32 = args[2]->Int32Value();
size_t buf_length = buf->GetIndexedPropertiesExternalArrayDataLength();
char* buf_data =
static_cast<char*>(buf->GetIndexedPropertiesExternalArrayData());
uint32_t offset;

if (buf_length > 0)
CHECK_NE(buf_data, nullptr);

if (offset_i32 < 0) {
if (offset_i32 + static_cast<int32_t>(obj_length) < 0)
offset = 0;
else
offset = static_cast<uint32_t>(obj_length + offset_i32);
} else {
offset = static_cast<uint32_t>(offset_i32);
}

if (buf_length == 0 ||
obj_length == 0 ||
(offset != 0 && buf_length + offset <= buf_length) ||
buf_length + offset > obj_length)
return args.GetReturnValue().Set(-1);

int32_t r =
IndexOf(obj_data + offset, obj_length - offset, buf_data, buf_length);
args.GetReturnValue().Set(r == -1 ? -1 : static_cast<int32_t>(r + offset));
}


void IndexOfNumber(const FunctionCallbackInfo<Value>& args) {
ASSERT(args[0]->IsObject());
ASSERT(args[1]->IsNumber());
ASSERT(args[2]->IsNumber());

ARGS_THIS(args[0].As<Object>());
uint32_t needle = args[1]->Uint32Value();
int32_t offset_i32 = args[2]->Int32Value();
uint32_t offset;

if (offset_i32 < 0) {
if (offset_i32 + static_cast<int32_t>(obj_length) < 0)
offset = 0;
else
offset = static_cast<uint32_t>(obj_length + offset_i32);
} else {
offset = static_cast<uint32_t>(offset_i32);
}

if (obj_length == 0 || offset + 1 > obj_length)
return args.GetReturnValue().Set(-1);

void* ptr = memchr(obj_data + offset, needle, obj_length - offset);
char* ptr_char = static_cast<char*>(ptr);
args.GetReturnValue().Set(
ptr ? static_cast<int32_t>(ptr_char - obj_data) : -1);
}


// pass Buffer object to load prototype methods
void SetupBufferJS(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Expand Down Expand Up @@ -650,6 +763,9 @@ void Initialize(Handle<Object> target,
env->SetMethod(target, "byteLength", ByteLength);
env->SetMethod(target, "compare", Compare);
env->SetMethod(target, "fill", Fill);
env->SetMethod(target, "indexOfBuffer", IndexOfBuffer);
env->SetMethod(target, "indexOfNumber", IndexOfNumber);
env->SetMethod(target, "indexOfString", IndexOfString);

env->SetMethod(target, "readDoubleBE", ReadDoubleBE);
env->SetMethod(target, "readDoubleLE", ReadDoubleLE);
Expand Down
75 changes: 75 additions & 0 deletions test/parallel/test-buffer-indexof.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
var common = require('../common');
var assert = require('assert');

var Buffer = require('buffer').Buffer;

var b = new Buffer('abcdef');
var buf_a = new Buffer('a');
var buf_bc = new Buffer('bc');
var buf_f = new Buffer('f');
var buf_z = new Buffer('z');
var buf_empty = new Buffer('');

assert.equal(b.indexOf('a'), 0);
assert.equal(b.indexOf('a', 1), -1);
assert.equal(b.indexOf('a', -1), -1);
assert.equal(b.indexOf('a', -4), -1);
assert.equal(b.indexOf('a', -b.length), 0);
assert.equal(b.indexOf('a', NaN), 0);
assert.equal(b.indexOf('a', -Infinity), 0);
assert.equal(b.indexOf('a', Infinity), -1);
assert.equal(b.indexOf('bc'), 1);
assert.equal(b.indexOf('bc', 2), -1);
assert.equal(b.indexOf('bc', -1), -1);
assert.equal(b.indexOf('bc', -3), -1);
assert.equal(b.indexOf('bc', -5), 1);
assert.equal(b.indexOf('bc', NaN), 1);
assert.equal(b.indexOf('bc', -Infinity), 1);
assert.equal(b.indexOf('bc', Infinity), -1);
assert.equal(b.indexOf('f'), b.length - 1);
assert.equal(b.indexOf('z'), -1);
assert.equal(b.indexOf(''), -1);
assert.equal(b.indexOf('', 1), -1);
assert.equal(b.indexOf('', b.length + 1), -1);
assert.equal(b.indexOf('', Infinity), -1);
assert.equal(b.indexOf(buf_a), 0);
assert.equal(b.indexOf(buf_a, 1), -1);
assert.equal(b.indexOf(buf_a, -1), -1);
assert.equal(b.indexOf(buf_a, -4), -1);
assert.equal(b.indexOf(buf_a, -b.length), 0);
assert.equal(b.indexOf(buf_a, NaN), 0);
assert.equal(b.indexOf(buf_a, -Infinity), 0);
assert.equal(b.indexOf(buf_a, Infinity), -1);
assert.equal(b.indexOf(buf_bc), 1);
assert.equal(b.indexOf(buf_bc, 2), -1);
assert.equal(b.indexOf(buf_bc, -1), -1);
assert.equal(b.indexOf(buf_bc, -3), -1);
assert.equal(b.indexOf(buf_bc, -5), 1);
assert.equal(b.indexOf(buf_bc, NaN), 1);
assert.equal(b.indexOf(buf_bc, -Infinity), 1);
assert.equal(b.indexOf(buf_bc, Infinity), -1);
assert.equal(b.indexOf(buf_f), b.length - 1);
assert.equal(b.indexOf(buf_z), -1);
assert.equal(b.indexOf(buf_empty), -1);
assert.equal(b.indexOf(buf_empty, 1), -1);
assert.equal(b.indexOf(buf_empty, b.length + 1), -1);
assert.equal(b.indexOf(buf_empty, Infinity), -1);
assert.equal(b.indexOf(0x61), 0);
assert.equal(b.indexOf(0x61, 1), -1);
assert.equal(b.indexOf(0x61, -1), -1);
assert.equal(b.indexOf(0x61, -4), -1);
assert.equal(b.indexOf(0x61, -b.length), 0);
assert.equal(b.indexOf(0x61, NaN), 0);
assert.equal(b.indexOf(0x61, -Infinity), 0);
assert.equal(b.indexOf(0x61, Infinity), -1);
assert.equal(b.indexOf(0x0), -1);

assert.throws(function() {
b.indexOf(function() { });
});
assert.throws(function() {
b.indexOf({});
});
assert.throws(function() {
b.indexOf([]);
});