Skip to content

Commit

Permalink
Implement pack option
Browse files Browse the repository at this point in the history
Fixes TooTallNate#2.
Closes TooTallNate#21.

Squashed commit of the following:

commit 093212b
Author: Lee, SungUk <[email protected]>
Date:   Wed Aug 3 10:33:09 2016 +0900

    Change Struct's second argument type to object

    Applied review comments;
    - Constructor arugments type change boolean to object
    - Assign of `StructType.isPacked` use ternary operator
    - Remove `usePack` method
    - `sizeof` and `alignof` calculation move to C++ code

commit a1e8f66
Author: Lee, SungUk <[email protected]>
Date:   Tue Jan 5 15:11:11 2016 +0900

    Fix broken unittests of appveyor

commit d5415f3
Author: Lee, SungUk <[email protected]>
Date:   Tue Jan 5 13:15:27 2016 +0900

    Resolve TooTallNate#2 Implement `usePack`

    - Apply [Alexander Seliverstov's method][1]
    - Now, `Struct` use 2nd arguments
    - `usePack()` switch padding mode

    [1]: https://groups.google.com/d/msg/nodejs/2wKCYmGPT5E/DLi4Pjt7j_4J
  • Loading branch information
d3m3vilurr authored and TooTallNate committed Aug 3, 2016
1 parent c595fb9 commit eb4550f
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 3 deletions.
14 changes: 11 additions & 3 deletions lib/struct.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,12 @@ function Struct () {
StructType.toString = toString
StructType.fields = {}

var opt = (arguments.length > 0 && arguments[1]) ? arguments[1] : {};
// Setup the ref "type" interface. The constructor doubles as the "type" object
StructType.size = 0
StructType.alignment = 0
StructType.indirection = 1
StructType.isPacked = opt.packed ? Boolean(opt.packed) : false
StructType.get = get
StructType.set = set

Expand Down Expand Up @@ -245,7 +247,11 @@ function recalc (struct) {
if (type.indirection > 1) {
alignment = ref.alignof.pointer
}
struct.alignment = Math.max(struct.alignment, alignment)
if (struct.isPacked) {
struct.alignment = Math.min(struct.alignment || alignment, alignment)
} else {
struct.alignment = Math.max(struct.alignment, alignment)
}
})

// second loop through sets the `offset` property on each "field"
Expand All @@ -270,12 +276,14 @@ function recalc (struct) {
function addType (type) {
var offset = struct.size
var align = type.indirection === 1 ? type.alignment : ref.alignof.pointer
var padding = (align - (offset % align)) % align
var padding = struct.isPacked ? 0 : (align - (offset % align)) % align
var size = type.indirection === 1 ? type.size : ref.sizeof.pointer

offset += padding

assert.equal(offset % align, 0, "offset should align")
if (!struct.isPacked) {
assert.equal(offset % align, 0, "offset should align")
}

// adjust the "size" of the struct type
struct.size = offset + size
Expand Down
14 changes: 14 additions & 0 deletions test/struct.js
Original file line number Diff line number Diff line change
Expand Up @@ -379,4 +379,18 @@ describe('Struct', function () {

})

describe('packed struct', function () {

it('with-padding/no-padding struct', function () {
var np = Struct({ a: 'char', p: ref.refType('void') }, {packed: true})
assert.equal(bindings['test20 sizeof'], np.size)
assert.equal(bindings['test20 alignof'], np.alignment)

var wp = Struct({ a: 'char', p: ref.refType('void') })
assert.equal(bindings['test21 sizeof'], wp.size)
assert.equal(bindings['test21 alignof'], wp.alignment)
})

})

})
17 changes: 17 additions & 0 deletions test/struct_tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,18 @@ typedef struct _test19 {
in an album or playlist struct */
} test19;

#pragma pack(1)
typedef struct _test20 {
char a;
void *p;
} test20;

#pragma pack()
typedef struct _test21 {
char a;
void *p;
} test21;

void Initialize(v8::Handle<v8::Object> target) {
Nan::HandleScope scope;

Expand Down Expand Up @@ -275,6 +287,11 @@ void Initialize(v8::Handle<v8::Object> target) {
target->Set(Nan::New<v8::String>("test19 offsetof popularity").ToLocalChecked(), Nan::New<v8::Number>(offsetof(test19, popularity)));
target->Set(Nan::New<v8::String>("test19 offsetof next").ToLocalChecked(), Nan::New<v8::Number>(offsetof(test19, next)));

target->Set(Nan::New<v8::String>("test20 sizeof").ToLocalChecked(), Nan::New<v8::Number>(sizeof(test20)));
target->Set(Nan::New<v8::String>("test20 alignof").ToLocalChecked(), Nan::New<v8::Number>(__alignof__(test20)));

target->Set(Nan::New<v8::String>("test21 sizeof").ToLocalChecked(), Nan::New<v8::Number>(sizeof(test21)));
target->Set(Nan::New<v8::String>("test21 alignof").ToLocalChecked(), Nan::New<v8::Number>(__alignof__(test21)));
}

} // anonymous namespace
Expand Down

0 comments on commit eb4550f

Please sign in to comment.