Keep only the largest field as the public field

Without this, ffi-napi will iterate through the `fields` property
and **sum up** the sizes of the member fields [1]. This will cause
unions to appear to be larger than they are. This can cause memory
corruption on 64-bit x86 Windows when the return type should be 8
bytes (and thus be returned in a register) but ffi-napi makes it into
an indirect buffer instead.

Fixes node-ffi-napi#15

[1]: https://github.com/node-ffi-napi/node-ffi-napi/blob/1e7bbb170462f5f0880350cc4a518a2755b9337f/lib/type.js#L56

lib/union.js

@@ -64,7 +64,7 @@ function Union () {
 
   UnionType.defineProperty = defineProperty
   UnionType.toString = toString
-  UnionType.fields = {}
+  UnionType.allFields = {}
 
   // comply with ref's "type" interface
   UnionType.size = 0
@@ -73,7 +73,7 @@ function Union () {
   UnionType.get = get
   UnionType.set = set
 
-  // Read the fields list
+  // Read the allFields list
   var arg = arguments[0]
   if (typeof arg === 'object') {
     Object.keys(arg).forEach(function (name) {
@@ -102,7 +102,7 @@ function set (buffer, offset, value) {
   var isUnion = value instanceof this
   if (isUnion) {
     // TODO: optimize - use Buffer#copy()
-    Object.keys(this.fields).forEach(function (name) {
+    Object.keys(this.allFields).forEach(function (name) {
       // hopefully hit the setters
       union[name] = value[name]
     })
@@ -151,7 +151,7 @@ function defineProperty (name, type) {
   var field = {
     type: type
   }
-  this.fields[name] = field
+  this.allFields[name] = field
 
   // calculate the new size and alignment
   recalc(this);
@@ -168,33 +168,34 @@ function defineProperty (name, type) {
 }
 
 function recalc (union) {
-  // reset size and alignment
-  union.size = 0
-  union.alignment = 0
+  var biggest
+  var fieldNames = Object.keys(union.allFields)
 
-  var fieldNames = Object.keys(union.fields)
-
-  // loop through to set the size of the union of the largest member field
-  // and the alignment to the requirements of the largest member
+  // find the largest member field by size / alignment
   fieldNames.forEach(function (name) {
-    var field = union.fields[name]
+    var field = union.allFields[name]
     var type = field.type
 
     var size = type.indirection === 1 ? type.size : ref.sizeof.pointer
     var alignment = type.alignment || ref.alignof.pointer
     if (type.indirection > 1) {
       alignment = ref.alignof.pointer
     }
-    union.alignment = Math.max(union.alignment, alignment)
-    union.size = Math.max(union.size, size)
+    var fields = {}
+    fields[name] = { type: type }
+
+    var current =  { fields: fields, size: size, alignment: alignment }
+
+    if (!biggest || size > biggest.size || alignment > biggest.alignment) {
+      biggest = current
+    }
   })
 
-  // any padding
-  var left = union.size % union.alignment
-  if (left > 0) {
-    debug('additional padding to the end of union:', union.alignment - left)
-    union.size += union.alignment - left
-  }
+  union.alignment = biggest.alignment;
+  union.size = biggest.size;
+  // Pretend like we only have this one field so that
+  // ffi-napi doesn't overcount the size
+  union.fields = biggest.fields;
 }
 
 

test/union.js

@@ -51,9 +51,9 @@ describe('Union', function () {
         , 'sval': 'string'
       })
 
-      assert.strictEqual(ref.types.int, U.fields.ival.type)
-      assert.strictEqual(ref.types.long, U.fields.lval.type)
-      assert.strictEqual(ref.coerceType('string'), U.fields.sval.type)
+      assert.strictEqual(ref.types.int, U.allFields.ival.type)
+      assert.strictEqual(ref.types.long, U.allFields.lval.type)
+      assert.strictEqual(ref.coerceType('string'), U.allFields.sval.type)
     })
 
   })
@@ -86,7 +86,7 @@ describe('Union', function () {
           assert.equal(expectedAlignment, unionType.alignment, 'test' + testNumber +
             ': __alignof__(): expected ' + unionType.alignment + ' to equal ' + expectedAlignment)
         })
-        Object.keys(unionType.fields).forEach(function (name) {
+        Object.keys(unionType.allFields).forEach(function (name) {
           if ('skip' == name) return;
           // these tests just verify the assumption that the
           // offset of every field is always 0