buffer: add Buffer.prototype.compare by offset

Adds additional `targetStart`, `targetEnd`, `sourceStart,
and `sourceEnd` arguments to `Buffer.prototype.compare`
to allow comparison of sub-ranges of two Buffers without
requiring Buffer.prototype.slice()

Fixes: https://github.com/nodejs/node/issues/521
PR-URL: https://github.com/nodejs/node/pull/5880
Reviewed-By: Trevor Norris <trev.norris@gmail.com>

benchmark/buffers/buffer-compare-offset.js

@@ -0,0 +1,58 @@
+'use strict';
+const common = require('../common.js');
+const v8 = require('v8');
+
+const bench = common.createBenchmark(main, {
+  method: ['offset', 'slice'],
+  size: [16, 512, 1024, 4096, 16386],
+  millions: [1]
+});
+
+function compareUsingSlice(b0, b1, len, iter) {
+
+  // Force optimization before starting the benchmark
+  Buffer.compare(b0.slice(1, len), b1.slice(1, len));
+  v8.setFlagsFromString('--allow_natives_syntax');
+  eval('%OptimizeFunctionOnNextCall(Buffer.compare)');
+  eval('%OptimizeFunctionOnNextCall(b0.slice)');
+  eval('%OptimizeFunctionOnNextCall(b1.slice)');
+  Buffer.compare(b0.slice(1, len), b1.slice(1, len));
+  doCompareUsingSlice(b0, b1, len, iter);
+}
+
+function doCompareUsingSlice(b0, b1, len, iter) {
+  var i;
+  bench.start();
+  for (i = 0; i < iter; i++)
+    Buffer.compare(b0.slice(1, len), b1.slice(1, len));
+  bench.end(iter / 1e6);
+}
+
+function compareUsingOffset(b0, b1, len, iter) {
+  len = len + 1;
+  // Force optimization before starting the benchmark
+  b0.compare(b1, 1, len, 1, len);
+  v8.setFlagsFromString('--allow_natives_syntax');
+  eval('%OptimizeFunctionOnNextCall(b0.compare)');
+  b0.compare(b1, 1, len, 1, len);
+  doCompareUsingOffset(b0, b1, len, iter);
+}
+
+function doCompareUsingOffset(b0, b1, len, iter) {
+  var i;
+  bench.start();
+  for (i = 0; i < iter; i++)
+    b0.compare(b1, 1, len, 1, len);
+  bench.end(iter / 1e6);
+}
+
+function main(conf) {
+  const iter = (conf.millions >>> 0) * 1e6;
+  const size = (conf.size >>> 0);
+  const method = conf.method === 'slice' ?
+      compareUsingSlice : compareUsingOffset;
+  method(Buffer.alloc(size, 'a'),
+         Buffer.alloc(size, 'b'),
+         size >> 1,
+         iter);
+}

doc/api/buffer.markdown

@@ -675,18 +675,26 @@ console.log(buf.toString('ascii'));
   // Prints: Node.js
 ```
 
-### buf.compare(otherBuffer)
+### buf.compare(target[, targetStart[, targetEnd[, sourceStart[, sourceEnd]]]])
 
-* `otherBuffer` {Buffer}
+* `target` {Buffer}
+* `targetStart` {Integer} The offset within `target` at which to begin
+  comparison. default = `0`.
+* `targetEnd` {Integer} The offset with `target` at which to end comparison.
+  Ignored when `targetStart` is `undefined`. default = `target.byteLength`.
+* `sourceStart` {Integer} The offset within `buf` at which to begin comparison.
+  Ignored when `targetStart` is `undefined`. default = `0`
+* `sourceEnd` {Integer} The offset within `buf` at which to end comparison.
+  Ignored when `targetStart` is `undefined`. default = `buf.byteLength`.
 * Return: {Number}
 
 Compares two Buffer instances and returns a number indicating whether `buf`
-comes before, after, or is the same as the `otherBuffer` in sort order.
+comes before, after, or is the same as the `target` in sort order.
 Comparison is based on the actual sequence of bytes in each Buffer.
 
-* `0` is returned if `otherBuffer` is the same as `buf`
-* `1` is returned if `otherBuffer` should come *before* `buf` when sorted.
-* `-1` is returned if `otherBuffer` should come *after* `buf` when sorted.
+* `0` is returned if `target` is the same as `buf`
+* `1` is returned if `target` should come *before* `buf` when sorted.
+* `-1` is returned if `target` should come *after* `buf` when sorted.
 
 ```js
 const buf1 = Buffer.from('ABC');
@@ -708,6 +716,25 @@ console.log(buf2.compare(buf3));
   // produces sort order [buf1, buf3, buf2]
 ```
 
+The optional `targetStart`, `targetEnd`, `sourceStart`, and `sourceEnd` 
+arguments can be used to limit the comparison to specific ranges within the two 
+`Buffer` objects.
+
+```js
+const buf1 = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9]);
+const buf2 = Buffer.from([5, 6, 7, 8, 9, 1, 2, 3, 4]);
+
+console.log(buf1.compare(buf2, 5, 9, 0, 4));
+  // Prints: 0
+console.log(buf1.compare(buf2, 0, 6, 4));
+  // Prints: -1
+console.log(buf1.compare(buf2, 5, 6, 5));
+  // Prints: 1
+```
+
+A `RangeError` will be thrown if: `targetStart < 0`, `sourceStart < 0`,
+`targetEnd > target.byteLength` or `sourceEnd > source.byteLength`.
+
 ### buf.copy(targetBuffer[, targetStart[, sourceStart[, sourceEnd]]])
 
 * `targetBuffer` {Buffer} Buffer to copy into

lib/buffer.js

@@ -537,15 +537,44 @@ Buffer.prototype.inspect = function inspect() {
   return '<' + this.constructor.name + ' ' + str + '>';
 };
 
+Buffer.prototype.compare = function compare(target,
+                                            start,
+                                            end,
+                                            thisStart,
+                                            thisEnd) {
 
-Buffer.prototype.compare = function compare(b) {
-  if (!(b instanceof Buffer))
+  if (!(target instanceof Buffer))
     throw new TypeError('Argument must be a Buffer');
 
-  if (this === b)
-    return 0;
+  if (start === undefined)
+    start = 0;
+  if (end === undefined)
+    end = target ? target.length : 0;
+  if (thisStart === undefined)
+    thisStart = 0;
+  if (thisEnd === undefined)
+    thisEnd = this.length;
 
-  return binding.compare(this, b);
+  if (start < 0 ||
+      end > target.length ||
+      thisStart < 0 ||
+      thisEnd > this.length) {
+    throw new RangeError('out of range index');
+  }
+
+  if (thisStart >= thisEnd && start >= end)
+    return 0;
+  if (thisStart >= thisEnd)
+    return -1;
+  if (start >= end)
+    return 1;
+
+  start >>>= 0;
+  end >>>= 0;
+  thisStart >>>= 0;
+  thisEnd >>>= 0;
+
+  return binding.compareOffset(this, target, start, thisStart, end, thisEnd);
 };
 
 function slowIndexOf(buffer, val, byteOffset, encoding) {

src/node_buffer.cc

@@ -870,6 +870,62 @@ void ByteLengthUtf8(const FunctionCallbackInfo<Value> &args) {
   args.GetReturnValue().Set(args[0].As<String>()->Utf8Length());
 }
 
+// Normalize val to be an integer in the range of [1, -1] since
+// implementations of memcmp() can vary by platform.
+static int normalizeCompareVal(int val, size_t a_length, size_t b_length) {
+  if (val == 0) {
+    if (a_length > b_length)
+      return 1;
+    else if (a_length < b_length)
+      return -1;
+  } else {
+    if (val > 0)
+      return 1;
+    else
+      return -1;
+  }
+  return val;
+}
+
+void CompareOffset(const FunctionCallbackInfo<Value> &args) {
+  Environment* env = Environment::GetCurrent(args);
+
+  THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
+  THROW_AND_RETURN_UNLESS_BUFFER(env, args[1]);
+  SPREAD_ARG(args[0], ts_obj);
+  SPREAD_ARG(args[1], target);
+
+  size_t target_start;
+  size_t source_start;
+  size_t source_end;
+  size_t target_end;
+
+  CHECK_NOT_OOB(ParseArrayIndex(args[2], 0, &target_start));
+  CHECK_NOT_OOB(ParseArrayIndex(args[3], 0, &source_start));
+  CHECK_NOT_OOB(ParseArrayIndex(args[4], target_length, &target_end));
+  CHECK_NOT_OOB(ParseArrayIndex(args[5], ts_obj_length, &source_end));
+
+  if (source_start > ts_obj_length)
+    return env->ThrowRangeError("out of range index");
+  if (target_start > target_length)
+    return env->ThrowRangeError("out of range index");
+
+  CHECK_LE(source_start, source_end);
+  CHECK_LE(target_start, target_end);
+
+  size_t to_cmp = MIN(MIN(source_end - source_start,
+                      target_end - target_start),
+                      ts_obj_length - source_start);
+
+  int val = normalizeCompareVal(to_cmp > 0 ?
+                                  memcmp(ts_obj_data + source_start,
+                                         target_data + target_start,
+                                         to_cmp) : 0,
+                                source_end - source_start,
+                                target_end - target_start);
+
+  args.GetReturnValue().Set(val);
+}
 
 void Compare(const FunctionCallbackInfo<Value> &args) {
   Environment* env = Environment::GetCurrent(args);
@@ -881,22 +937,9 @@ void Compare(const FunctionCallbackInfo<Value> &args) {
 
   size_t cmp_length = MIN(obj_a_length, obj_b_length);
 
-  int val = cmp_length > 0 ? memcmp(obj_a_data, obj_b_data, cmp_length) : 0;
-
-  // Normalize val to be an integer in the range of [1, -1] since
-  // implementations of memcmp() can vary by platform.
-  if (val == 0) {
-    if (obj_a_length > obj_b_length)
-      val = 1;
-    else if (obj_a_length < obj_b_length)
-      val = -1;
-  } else {
-    if (val > 0)
-      val = 1;
-    else
-      val = -1;
-  }
-
+  int val = normalizeCompareVal(cmp_length > 0 ?
+                                memcmp(obj_a_data, obj_b_data, cmp_length) : 0,
+                                obj_a_length, obj_b_length);
   args.GetReturnValue().Set(val);
 }
 
@@ -1172,6 +1215,7 @@ void Initialize(Local<Object> target,
 
   env->SetMethod(target, "byteLengthUtf8", ByteLengthUtf8);
   env->SetMethod(target, "compare", Compare);
+  env->SetMethod(target, "compareOffset", CompareOffset);
   env->SetMethod(target, "fill", Fill);
   env->SetMethod(target, "indexOfBuffer", IndexOfBuffer);
   env->SetMethod(target, "indexOfNumber", IndexOfNumber);

test/parallel/test-buffer-compare-offset.js

@@ -0,0 +1,63 @@
+'use strict';
+
+require('../common');
+const assert = require('assert');
+
+const a = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 0]);
+const b = Buffer.from([5, 6, 7, 8, 9, 0, 1, 2, 3, 4]);
+
+assert.equal(-1, a.compare(b));
+
+// Equivalent to a.compare(b).
+assert.equal(-1, a.compare(b, 0));
+assert.equal(-1, a.compare(b, '0'));
+
+// Equivalent to a.compare(b).
+assert.equal(-1, a.compare(b, 0, undefined, 0));
+
+// Zero-length targer, return 1
+assert.equal(1, a.compare(b, 0, 0, 0));
+assert.equal(1, a.compare(b, '0', '0', '0'));
+
+// Equivalent to Buffer.compare(a, b.slice(6, 10))
+assert.equal(1, a.compare(b, 6, 10));
+
+// Zero-length source, return -1
+assert.equal(-1, a.compare(b, 6, 10, 0, 0));
+
+// Equivalent to Buffer.compare(a.slice(4), b.slice(0, 5))
+assert.equal(1, a.compare(b, 0, 5, 4));
+
+// Equivalent to Buffer.compare(a.slice(1), b.slice(5))
+assert.equal(1, a.compare(b, 5, undefined, 1));
+
+// Equivalent to Buffer.compare(a.slice(2), b.slice(2, 4))
+assert.equal(-1, a.compare(b, 2, 4, 2));
+
+// Equivalent to Buffer.compare(a.slice(4), b.slice(0, 7))
+assert.equal(-1, a.compare(b, 0, 7, 4));
+
+// Equivalent to Buffer.compare(a.slice(4, 6), b.slice(0, 7));
+assert.equal(-1, a.compare(b, 0, 7, 4, 6));
+
+// zero length target
+assert.equal(1, a.compare(b, 0, null));
+
+// coerces to targetEnd == 5
+assert.equal(-1, a.compare(b, 0, {valueOf: () => 5}));
+
+// zero length target
+assert.equal(1, a.compare(b, Infinity, -Infinity));
+
+// zero length target because default for targetEnd <= targetSource
+assert.equal(1, a.compare(b, '0xff'));
+
+const oor = /out of range index/;
+
+assert.throws(() => a.compare(b, 0, 100, 0), oor);
+assert.throws(() => a.compare(b, 0, 1, 0, 100), oor);
+assert.throws(() => a.compare(b, -1), oor);
+assert.throws(() => a.compare(b, 0, '0xff'), oor);
+assert.throws(() => a.compare(b, 0, Infinity), oor);
+assert.throws(() => a.compare(b, -Infinity, Infinity), oor);
+assert.throws(() => a.compare(), /Argument must be a Buffer/);