Skip to content

Commit

Permalink
lib: only slice buffer in set() in the non-instance case
Browse files Browse the repository at this point in the history
Otherwise before we slice, and then also pass in the `offset`
to `Buffer.copy()`, which is incorrect.

Opt for passing the correct offset to copy(), rather then
always slicing, which will hopefully be more performant.
  • Loading branch information
TooTallNate committed Nov 3, 2014
1 parent 27a2005 commit 9b20a3b
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 3 deletions.
6 changes: 3 additions & 3 deletions lib/struct.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,15 @@ function get (buffer, offset) {

function set (buffer, offset, value) {
debug('Struct "type" setter for buffer at offset', buffer, offset, value)
if (offset > 0) {
buffer = buffer.slice(offset)
}
var isStruct = value instanceof this
if (isStruct) {
// optimization: copy the buffer contents directly rather
// than going through the ref-struct constructor
value['ref.buffer'].copy(buffer, offset, 0, this.size);
} else {
if (offset > 0) {
buffer = buffer.slice(offset)
}
new this(buffer, value)
}
}
Expand Down
59 changes: 59 additions & 0 deletions test/struct.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,65 @@ describe('Struct', function () {

})

describe('set()', function () {

it('should work to set() an Object', function () {
var Foo = Struct({
test1: ref.types.int,
test2: ref.types.int
});

var b = new Buffer(Foo.size * 2);

Foo.set(b, Foo.size * 0, {
test1: 7123,
test2: -555
});

var f = new Foo(b);
assert.equal(f.test1, 7123);
assert.equal(f.test2, -555);

Foo.set(b, Foo.size * 1, {
test1: 1234,
test2: -1234
});

f = new Foo(b.slice(Foo.size * 1));
assert.equal(f.test1, 1234);
assert.equal(f.test2, -1234);
});

it('should work to set() a Struct instance', function () {
// see: https://github.com/TooTallNate/ref-struct/issues/11
var Foo = Struct({
test1: ref.types.int,
test2: ref.types.int
});

var b = new Buffer(Foo.size * 2);

Foo.set(b, Foo.size * 0, new Foo({
test1: 7123,
test2: -555
}));

var f = new Foo(b);
assert.equal(f.test1, 7123);
assert.equal(f.test2, -555);

Foo.set(b, Foo.size * 1, new Foo({
test1: 1234,
test2: -1234
}));

f = new Foo(b.slice(Foo.size * 1));
assert.equal(f.test1, 1234);
assert.equal(f.test2, -1234);
});

});

describe('ref(), deref()', function () {

it('should work to ref() and then deref() 1 level deep', function () {
Expand Down

0 comments on commit 9b20a3b

Please sign in to comment.