Skip to content

Commit

Permalink
fix #223, union sizing/layout issue
Browse files Browse the repository at this point in the history
  • Loading branch information
@twall
twall committed May 1, 2013
1 parent 5db3b42 commit 38b796a
Show file tree
Hide file tree
Showing 5 changed files with 160 additions and 146 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Bug Fixes
* [#215](https://github.com/twall/jna/issues/215): Force use of XSI `strerror_r` on linux - [LionelCons](https://github.com/LionelCons).
* [#214](https://github.com/twall/jna/issues/214): Don't map library names when an absolute path is provided - [@twall](https://github.com/twall).
* [#218](https://github.com/twall/jna/issues/218): Explicitly handle broken Android SecurityManager implementation - [@twall](https://github.com/twall).
* [#223](https://github.com/twall/jna/issues/223): Fix layout/size derivation for unions - [@twall](https://github.com/twall).

Release 3.5.2
=============
Expand Down
48 changes: 38 additions & 10 deletions src/com/sun/jna/Structure.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public interface ByReference { }
String arch = System.getProperty("os.arch").toLowerCase();
isPPC = "ppc".equals(arch) || "powerpc".equals(arch);
isSPARC = "sparc".equals(arch);
isARM = arch.startsWith("arm");
isARM = arch.startsWith("arm");
}

/** Use the platform default alignment. */
Expand Down Expand Up @@ -216,7 +216,7 @@ TypeMapper getTypeMapper() {
return typeMapper;
}

/** Initialize the type mapper for this structure.
/** Initialize the type mapper for this structure.
* If <code>null</code>, the default mapper for the
* defining class will be used.
*/
Expand Down Expand Up @@ -871,7 +871,7 @@ private List sort(Collection c) {
return list;
}

/** Returns all field names (sorted) provided so far by
/** Returns all field names (sorted) provided so far by
{@link #getFieldOrder}
@param force set if results are required immediately
@return null if not yet able to provide fields, and force is false.
Expand Down Expand Up @@ -899,7 +899,7 @@ protected List getFields(boolean force) {

Set orderedNames = new HashSet(fieldOrder);
if (!orderedNames.equals(names)) {
throw new Error("Structure.getFieldOrder() on " + getClass()
throw new Error("Structure.getFieldOrder() on " + getClass()
+ " returns names ("
+ sort(fieldOrder)
+ ") which do not match declared field names ("
Expand Down Expand Up @@ -991,6 +991,8 @@ private static class LayoutInfo {
private int alignType = ALIGN_DEFAULT;
private TypeMapper typeMapper;
private boolean variable;
// For unions only, field on which the union FFI type info is based
private StructField typeInfoField;
}

private void validateField(String name, Class type) {
Expand Down Expand Up @@ -1029,7 +1031,6 @@ private void validateFields() {
members.
*/
private LayoutInfo deriveLayout(boolean force, boolean avoidFFIType) {

int calculatedSize = 0;
List fields = getFields(force);
if (fields == null) {
Expand Down Expand Up @@ -1139,11 +1140,24 @@ else if (writeConverter != null || readConverter != null) {
if ((calculatedSize % fieldAlignment) != 0) {
calculatedSize += fieldAlignment - (calculatedSize % fieldAlignment);
}
structField.offset = calculatedSize;
calculatedSize += structField.size;
if (this instanceof Union) {
structField.offset = 0;
calculatedSize = Math.max(calculatedSize, structField.size);
}
else {
structField.offset = calculatedSize;
calculatedSize += structField.size;
}

// Save the field in our list
info.fields.put(structField.name, structField);

if (info.typeInfoField == null
|| info.typeInfoField.size < structField.size
|| (info.typeInfoField.size == structField.size
&& Structure.class.isAssignableFrom(structField.type))) {
info.typeInfoField = structField;
}
}

if (calculatedSize > 0) {
Expand Down Expand Up @@ -1504,7 +1518,7 @@ Pointer getTypeInfo() {
This is typically most effective when a native call populates a large
structure and you only need a few fields out of it. After the native
call you can call {@link #readField(String)} on only the fields of
interest.
interest.
*/
public void setAutoSynch(boolean auto) {
setAutoRead(auto);
Expand Down Expand Up @@ -1544,7 +1558,7 @@ static Pointer getTypeInfo(Object obj) {
return FFIType.get(obj);
}

/** Called from native code only; same as {@link
/** Called from native code only; same as {@link
* #newInstance(Class,Pointer)}, except that it additionally performs
* {@link #conditionalAutoRead()}.
*/
Expand Down Expand Up @@ -1619,6 +1633,20 @@ public static Structure newInstance(Class type) throws IllegalArgumentException
}
}

/** Keep track of the largest aggregate field of the union to use for
* FFI type information.
*/
StructField typeInfoField() {
LayoutInfo info;
synchronized(layoutInfo) {
info = (LayoutInfo)layoutInfo.get(getClass());
}
if (info != null) {
return info.typeInfoField;
}
return null;
}

static class StructField extends Object {
public String name;
public Class type;
Expand Down Expand Up @@ -1704,7 +1732,7 @@ private FFIType(Structure ref) {
ref.ensureAllocated(true);

if (ref instanceof Union) {
StructField sf = ((Union)ref).biggestField;
StructField sf = ((Union)ref).typeInfoField();
els = new Pointer[] {
get(ref.getFieldValue(sf.field), sf.type),
null,
Expand Down
75 changes: 15 additions & 60 deletions src/com/sun/jna/Union.java
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
/* Copyright (c) 2007-2012 Timothy Wall, All Rights Reserved
*
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 2.1 of the License, or (at your option) any later version.
*
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
* Lesser General Public License for more details.
*/
package com.sun.jna;

Expand All @@ -20,18 +20,17 @@
/** Represents a native union. When writing to native memory, the field
* corresponding to the type passed to {@link #setType} will be written
* to native memory. Upon reading from native memory, Structure, String,
* or WString fields will <em>not</em> be initialized unless they are
* or WString fields will <em>not</em> be initialized unless they are
* the current field as identified by a call to {@link #setType}. The current
* field is always unset by default to avoid accidentally attempting to read
* a field that is not valid. In the case of a String, for instance, an
* a field that is not valid. In the case of a String, for instance, an
* invalid pointer may result in a memory fault when attempting to initialize
* the String.
* the String.
*/
public abstract class Union extends Structure {
private StructField activeField;
StructField biggestField;

/** Create a Union whose size and alignment will be calculated

/** Create a Union whose size and alignment will be calculated
* automatically.
*/
protected Union() { }
Expand Down Expand Up @@ -65,10 +64,10 @@ protected List getFieldOrder() {
return list;
}

/** Indicates by type which field will be used to write to native memory.
/** Indicates by type which field will be used to write to native memory.
* If there are multiple fields of the same type, use {@link
* #setType(String)} instead with the field name.
* @throws IllegalArgumentException if the type does not correspond to
* @throws IllegalArgumentException if the type does not correspond to
* any declared union field.
*/
public void setType(Class type) {
Expand All @@ -82,7 +81,7 @@ public void setType(Class type) {
}
throw new IllegalArgumentException("No field of type " + type + " in " + this);
}

/**
* Indicates which field will be used to write to native memory.
* @throws IllegalArgumentException if the name does not correspond to
Expand Down Expand Up @@ -199,68 +198,24 @@ void writeField(StructField field) {
}

/** Avoid reading pointer-based fields and structures unless explicitly
* selected. Structures may contain pointer-based fields which can
* selected. Structures may contain pointer-based fields which can
* crash the VM if not properly initialized.
*/
Object readField(StructField field) {
if (field == activeField
if (field == activeField
|| (!Structure.class.isAssignableFrom(field.type)
&& !String.class.isAssignableFrom(field.type)
&& !WString.class.isAssignableFrom(field.type))) {
return super.readField(field);
}
// Field not accessible
// TODO: read structure, to the extent possible; need a "recursive"
// flag to "read"
// flag to "read" to indicate we want to avoid pointer-based fields
return null;
}

/** Adjust the size to be the size of the largest element, and ensure
* all fields begin at offset zero.
*/
int calculateSize(boolean force, boolean avoidFFIType) {
int size = super.calculateSize(force, avoidFFIType);
if (size != CALCULATE_SIZE) {
int fsize = 0;
for (Iterator i=fields().values().iterator();i.hasNext();) {
StructField f = (StructField)i.next();
f.offset = 0;
if (f.size > fsize
// Prefer aggregate types to simple types, since they
// will have more complex packing rules (some platforms
// have specific methods for packing small structs into
// registers, which may not match the packing of bytes
// for a primitive type).
|| (f.size == fsize
&& Structure.class.isAssignableFrom(f.type))) {
fsize = f.size;
biggestField = f;
}
}
size = calculateAlignedSize(fsize);
if (size > 0) {
// Update native FFI type information, if needed
if (this instanceof ByValue && !avoidFFIType) {
getTypeInfo();
}
}
}
return size;
}

/** All fields are considered the "first" element. */
protected int getNativeAlignment(Class type, Object value, boolean isFirstElement) {
return super.getNativeAlignment(type, value, true);
}

/** Avoid calculating type information until we know our biggest field.
* Return type information for the largest field to ensure all available
* bits are used.
*/
Pointer getTypeInfo() {
if (biggestField == null) {
// Not calculated yet
return null;
}
return super.getTypeInfo();
}
}
Loading

0 comments on commit 38b796a

Please sign in to comment.