Merge pull request #468 from twall/issue-467-direct-enum-type-mapping

Fixes #467, enum type mapping for direct-mapped libraries. Should also improve type mapper lookups for direct-mapped libraries.
java-native-access · Jul 16, 2015 · 882d19f · 882d19f
2 parents ce80363 + fa2ddae
commit 882d19f
Show file tree

Hide file tree

Showing 6 changed files with 122 additions and 26 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,4 +1,4 @@
-NOTE: as of JNA 4.0, JNA is now dual-licensed under LGPL and ASL (see LICENSE).
+NOTE: as of JNA 4.0, JNA is now dual-licensed under LGPL and ASL (see LICENSE).
 
 NOTE: JNI native support is typically incompatible between minor versions, and almost always incompatible between major versions.
 
@@ -15,12 +15,12 @@ Features
 * Updated AIX natives and build - [@twall](https://github.com/twall).
 * [#290](https://github.com/twall/jna/pull/290): Improved the stacktrace for the exceptions thrown by `com.sun.jna.Structure` - [@ebourg](https://github.com/ebourg).
 * [#332](https://github.com/twall/jna/pull/332): Added Win32 Monitor Configuration API in `com.sun.jna.platform.win32.Dxva2` - [@msteiger](https://github.com/msteiger).
-* Added Winspool monitor sample and updated Kernel32, WinBase and Winspool - [@wolftobias](https://github.com/wolftobias).
-* [#333](https://github.com/twall/jna/pull/333): Added `com.sun.jna.platform.win32.Ole32.CoTaskMemAlloc`, `CoTaskMemRealloc` and `CoTaskMemFree` - [@msteiger](https://github.com/msteiger).
+* Added Winspool monitor sample and updated Kernel32, WinBase, Winspool - [@wolftobias](https://github.com/wolftobias).
+* [#333](https://github.com/twall/jna/pull/333): Added `CoTaskMemAlloc`, `CoTaskMemRealloc` and `CoTaskMemFree` to `com.sun.jna.platform.win32.Ole32` - [@msteiger](https://github.com/msteiger).
 * [#334](https://github.com/twall/jna/pull/334): Added `com.sun.jna.platform.win32.Shell32.SHGetKnownFolderPath` and `KnownFolders` GUID constants - [@msteiger](https://github.com/msteiger).
 * [#338](https://github.com/twall/jna/pull/338): Added `com.sun.jna.platform.mac.XAttr` and `com.sun.jna.platform.mac.XAttrUtil` JNA wrapper for `<sys/xattr.h>` for Mac OS X - [@rednoah](https://github.com/rednoah).
-* [#339](https://github.com/twall/jna/pull/339): Added `com.sun.jna.platform.win32.User32.GetWindowPlacement`, `SetWindowPlacement`, `AdjustWindowRect`, `AdjustWindowRectEx`, `ExitWindowsEx`, and `LockWorkstation` - [@Timeroot](https://github.com/Timeroot).
-* [#286](https://github.com/twall/jna/pull/286): Added `com.sun.jna.platform.win32.Kernel32.CreateRemoteThread`, `WritePocessMemory` and `ReadProcessMemory` - [@sstokic-tgm](https://github.com/sstokic-tgm).
+* [#339](https://github.com/twall/jna/pull/339): Added `GetWindowPlacement`, `SetWindowPlacement`, `AdjustWindowRect`, `AdjustWindowRectEx`, `ExitWindowsEx`, and `LockWorkstation` to `com.sun.jna.platform.win32.User32` - [@Timeroot](https://github.com/Timeroot).
+* [#286](https://github.com/twall/jna/pull/286): Added `CreateRemoteThread`, `WritePocessMemory` and `ReadProcessMemory` to `com.sun.jna.platform.win32.Kernel32` - [@sstokic-tgm](https://github.com/sstokic-tgm).
 * [#350](https://github.com/twall/jna/pull/350): Added `jnacontrib.x11.api.X.Window.getSubwindows` - [@rm5248](https://github.com/rm5248).
 * Improved `contrib/msoffice` sample - [@wolftobias](https://github.com/wolftobias).
 * [#352](https://github.com/twall/jna/pull/352): Performance improvements due to reduced locking in `com.sun.jna.Library$Handler` and fewer vararg checks in `com.sun.jna.Function` - [@Boereck](https://github.com/Boereck).
@@ -67,6 +67,7 @@ Bug Fixes
 * [#403](https://github.com/twall/jna/pull/403): Fix `com.sun.jna.platform.win32.COM.COMUtils.SUCCEEDED` and `FAILED` - [@lwahonen](https://github.com/lwahonen).
 * [#404](https://github.com/twall/jna/pull/404): Fix `VARIANT` constructors for `int`, `short`, and `long` - [@lwahonen](https://github.com/lwahonen).
 * [#420](https://github.com/twall/jna/pull/420): Fix structure leaving always one element in ThreadLocal set - [@sjappig](https://github.com/sjappig).
+* [#467](https://github.com/twall/jna/issues/467): Fix TypeMapper usage with direct-mapped libraries converting primitives to Java objects (specifically enums) - [@twall](https://github.com/twall).
 
 Release 4.1
 ===========

diff --git a/native/dispatch.c b/native/dispatch.c
@@ -1139,20 +1139,31 @@ toNativeTypeMapped(JNIEnv* env, jobject obj, void* valuep, size_t size, jobject
 }
 
 static void
-fromNativeTypeMapped(JNIEnv* env, jobject from_native, void* resp, ffi_type* type, jclass javaClass, void* result) {
-  int jtype = get_jtype_from_ffi_type(type);
-  jobject value = new_object(env, (char)jtype, resp, JNI_TRUE);
+fromNativeTypeMapped(JNIEnv* env, jobject from_native,
+                     void* native_return_value,
+                     ffi_type* native_return_type,
+                     jclass java_return_class,
+                     void* result_storage) {
+  int jtype = get_jtype_from_ffi_type(native_return_type);
+  jobject value = new_object(env, (char)jtype, native_return_value, JNI_TRUE);
   if (!(*env)->ExceptionCheck(env)) {
     jobject obj = (*env)->CallStaticObjectMethod(env, classNative,
                                                  MID_Native_fromNativeTypeMapped,
-                                                 from_native, value, javaClass);
+                                                 from_native, value, java_return_class);
     if (!(*env)->ExceptionCheck(env)) {
-      // Must extract primitive types
-      if (type->type != FFI_TYPE_POINTER) {
-        extract_value(env, obj, result, type->size, JNI_TRUE);
+      // Convert objects into primitive types if the return class demands it
+      if ((*env)->IsSameObject(env, java_return_class, classPrimitiveBoolean)
+          || (*env)->IsSameObject(env, java_return_class, classPrimitiveByte)
+          || (*env)->IsSameObject(env, java_return_class, classPrimitiveCharacter)
+          || (*env)->IsSameObject(env, java_return_class, classPrimitiveShort)
+          || (*env)->IsSameObject(env, java_return_class, classPrimitiveInteger)
+          || (*env)->IsSameObject(env, java_return_class, classPrimitiveLong)
+          || (*env)->IsSameObject(env, java_return_class, classPrimitiveFloat)
+          || (*env)->IsSameObject(env, java_return_class, classPrimitiveDouble)) {
+        extract_value(env, obj, result_storage, native_return_type->size, JNI_TRUE);
       }
       else {
-        *(jobject*)result = obj;
+        *(jobject*)result_storage = obj;
       }
     }
   }
@@ -1444,6 +1455,7 @@ JNA_init(JNIEnv* env) {
   return NULL;
 }
 
+/** Copy value from the given Java object into the given storage buffer. */
 void
 extract_value(JNIEnv* env, jobject value, void* resp, size_t size, jboolean promote) {
   if (value == NULL) {
@@ -1514,8 +1526,9 @@ extract_value(JNIEnv* env, jobject value, void* resp, size_t size, jboolean prom
     *(void **)resp = getNativeAddress(env, value);
   }
   else {
-    fprintf(stderr, "JNA: unrecognized return type, size %d\n", (int)size);
+    fprintf(stderr, "JNA: extract_value: unrecognized return type, size %d\n", (int)size);
     memset(resp, 0, size);
+    throwByName(env, EError, "Unrecognized return type");
   }
 }
 
@@ -1780,7 +1793,8 @@ method_handler(ffi_cif* cif, void* volatile resp, void** argp, void *cdata) {
     resp = alloca(sizeof(jobject));
   }
   else if (data->rflag == CVT_TYPE_MAPPER) {
-    // Ensure enough space for the inner call result
+    // Ensure enough space for the inner call result, which may differ
+    // from the closure result
     resp = alloca(data->cif.rtype->size);
   }
   else if (data->rflag == CVT_STRUCTURE_BYVAL) {

diff --git a/src/com/sun/jna/Native.java b/src/com/sun/jna/Native.java
@@ -1419,6 +1419,7 @@ public static void register(Class cls, NativeLibrary lib) {
         List mlist = new ArrayList();
         TypeMapper mapper = (TypeMapper)
             lib.getOptions().get(Library.OPTION_TYPE_MAPPER);
+        cacheOptions(cls, lib.getOptions(), null);
 
         for (int i=0;i < methods.length;i++) {
             if ((methods[i].getModifiers() & Modifier.NATIVE) != 0) {
@@ -1444,7 +1445,10 @@ public static void register(Class cls, NativeLibrary lib) {
                 throw new IllegalArgumentException(rclass + " is not a supported return type (in method " + method.getName() + " in " + cls + ")");
             case CVT_TYPE_MAPPER:
                 fromNative = mapper.getFromNativeConverter(rclass);
-                closure_rtype = FFIType.get(rclass).peer;
+                // FFIType.get() always looks up the native type for any given
+                // class, so if we actually have conversion into a Java
+                // object, make sure we use the proper type information
+                closure_rtype = FFIType.get(rclass.isPrimitive() ? rclass : Pointer.class).peer;
                 rtype = FFIType.get(fromNative.nativeType()).peer;
                 break;
             case CVT_NATIVE_MAPPED:
@@ -1490,10 +1494,7 @@ else if (cvt[t] == CVT_TYPE_MAPPER) {
                     closure_atypes[t] = FFIType.get(Pointer.class).peer;
                     break;
                 case CVT_TYPE_MAPPER:
-                    if (type.isPrimitive())
-                        closure_atypes[t] = FFIType.get(type).peer;
-                    else
-                        closure_atypes[t] = FFIType.get(Pointer.class).peer;
+                    closure_atypes[t] = FFIType.get(type.isPrimitive() ? type : Pointer.class).peer;
                     atypes[t] = FFIType.get(toNative[t].nativeType()).peer;
                     break;
                 case CVT_DEFAULT:
@@ -1535,7 +1536,6 @@ else if (cvt[t] == CVT_TYPE_MAPPER) {
             registeredClasses.put(cls, handles);
             registeredLibraries.put(cls, lib);
         }
-        cacheOptions(cls, lib.getOptions(), null);
     }
 
     /** Take note of options used for a given library mapping, to facilitate

diff --git a/src/com/sun/jna/Structure.java b/src/com/sun/jna/Structure.java
@@ -1798,6 +1798,7 @@ private void init(Pointer[] els) {
             write();
         }
 
+        /** Obtain a pointer to the native FFI type descriptor for the given object. */
         static Pointer get(Object obj) {
             if (obj == null)
                 return FFITypes.ffi_type_pointer;
@@ -1847,7 +1848,7 @@ private static Pointer get(Object obj, Class cls) {
                     typeInfoMap.put(obj, type);
                     return type.getPointer();
                 }
-                throw new IllegalArgumentException("Unsupported Structure field type " + cls);
+                throw new IllegalArgumentException("Unsupported type " + cls);
             }
         }
     }

diff --git a/test/com/sun/jna/DirectTypeMapperTest.java b/test/com/sun/jna/DirectTypeMapperTest.java
@@ -19,6 +19,7 @@
 
 public class DirectTypeMapperTest extends TestCase {
 
+    /** Converts boolean to int when going to native. */
     public static class DirectTestLibraryBoolean {
         final static int MAGIC = 0xABEDCF23;
         public native int returnInt32Argument(boolean b);
@@ -37,6 +38,7 @@ public Class nativeType() {
             Native.register(NativeLibrary.getInstance("testlib", options));
         }
     }
+    /** Converts String to int when going to native. */
     public static class DirectTestLibraryString {
         public native int returnInt32Argument(String s);
         static {
@@ -54,6 +56,7 @@ public Class nativeType() {
             Native.register(NativeLibrary.getInstance("testlib", options));
         }
     }
+    /** Converts CharSequence to int when going to native. */
     public static class DirectTestLibraryCharSequence {
         public native int returnInt32Argument(String n);
         static {
@@ -72,6 +75,7 @@ public Class nativeType() {
             Native.register(NativeLibrary.getInstance("testlib", options));
         }
     }
+    /** Converts Number to int when going to native. */
     public static class DirectTestLibraryNumber {
         public native int returnInt32Argument(Number n);
         static {
@@ -90,7 +94,6 @@ public Class nativeType() {
             Native.register(NativeLibrary.getInstance("testlib", options));
         }
     }
-
     public void testBooleanToIntArgumentConversion() {
         DirectTestLibraryBoolean lib = new DirectTestLibraryBoolean();
         assertEquals("Failed to convert Boolean argument to Int",
@@ -116,7 +119,8 @@ public void testNumberToIntArgumentConversion() {
         assertEquals("Failed to convert Double argument to Int", MAGIC,
                      lib.returnInt32Argument(new Double(MAGIC)));
     }
-    public static class DirectBooleanTestLibrary {
+    /** Uses a type mapper to convert boolean->int and int->boolean */
+    public static class DirectTestLibraryBidirectionalBoolean {
         public native boolean returnInt32Argument(boolean b);
         static {
             final int MAGIC = 0xABEDCF23;
@@ -144,7 +148,7 @@ public Class nativeType() {
         }
     }
     public void testIntegerToBooleanResultConversion() throws Exception {
-        DirectBooleanTestLibrary lib = new DirectBooleanTestLibrary();
+        DirectTestLibraryBidirectionalBoolean lib = new DirectTestLibraryBidirectionalBoolean();
         // argument "true" converts to zero; result zero converts to "true"
         assertTrue("Failed to convert integer return to boolean TRUE", 
                    lib.returnInt32Argument(true));
@@ -188,6 +192,45 @@ public void testTypeMapperResultTypeConversion() throws Exception {
         assertEquals("Failed to convert int* return to java.awt.Point", 1234, p.x);
         assertEquals("Failed to convert int* return to java.awt.Point", 5678, p.y);
     }
+    public static class DirectTypeMappedEnumerationTestLibrary {
+        public static enum Enumeration {
+            STATUS_0(0), STATUS_1(1), STATUS_ERROR(-1);
+            private final int code;
+            Enumeration(int code) { this.code = code; }
+            public int getCode() { return code; }
+            public static Enumeration fromCode(int code) {
+                switch(code) {
+                case 0: return STATUS_0;
+                case 1: return STATUS_1;
+                default: return STATUS_ERROR;
+                }
+            }
+        }
+        public native Enumeration returnInt32Argument(Enumeration e);
+        static {
+            DefaultTypeMapper mapper = new DefaultTypeMapper();
+            mapper.addTypeConverter(Enumeration.class, new TypeConverter() {
+                public Object toNative(Object arg, ToNativeContext ctx) {
+                    return new Integer(((Enumeration)arg).getCode());
+                }
+                public Object fromNative(Object value, FromNativeContext context) {
+                    return Enumeration.fromCode(((Integer)value).intValue());
+                }
+                public Class nativeType() {
+                    return Integer.class;
+                }
+            });
+            Map options = new HashMap();
+            options.put(Library.OPTION_TYPE_MAPPER, mapper);
+
+            Native.register(NativeLibrary.getInstance("testlib", options));
+        }
+    }
+    public void testEnumerationConversion() {
+        DirectTypeMappedEnumerationTestLibrary lib = new DirectTypeMappedEnumerationTestLibrary();
+        DirectTypeMappedEnumerationTestLibrary.Enumeration e = lib.returnInt32Argument(DirectTypeMappedEnumerationTestLibrary.Enumeration.STATUS_1);
+        assertEquals("Failed to convert enumeration", DirectTypeMappedEnumerationTestLibrary.Enumeration.STATUS_1, e);
+    }
 
     public static void main(String[] args) {
         junit.textui.TestRunner.run(DirectTypeMapperTest.class);

diff --git a/test/com/sun/jna/TypeMapperTest.java b/test/com/sun/jna/TypeMapperTest.java
@@ -173,7 +173,44 @@ public Class nativeType() {
         s.read();
         assertFalse("Wrong value read", s.data);
     }
-
+
+    public static enum Enumeration {
+        STATUS_0(0), STATUS_1(1), STATUS_ERROR(-1);
+        private final int code;
+        Enumeration(int code) { this.code = code; }
+        public int getCode() { return code; }
+        public static Enumeration fromCode(int code) {
+            switch(code) {
+            case 0: return STATUS_0;
+            case 1: return STATUS_1;
+            default: return STATUS_ERROR;
+            }
+        }
+    }
+    public static interface EnumerationTestLibrary extends Library {
+        Enumeration returnInt32Argument(Enumeration arg);
+    }
+    public void testEnumConversion() throws Exception {
+        DefaultTypeMapper mapper = new DefaultTypeMapper();
+        TypeConverter converter = new TypeConverter() {
+            public Object toNative(Object value, ToNativeContext ctx) {
+                return new Integer(((Enumeration)value).getCode());
+            }
+            public Object fromNative(Object value, FromNativeContext context) {
+                return Enumeration.fromCode(((Integer)value).intValue());
+            }
+            public Class nativeType() {
+                return Integer.class;
+            }
+        };
+        mapper.addTypeConverter(Enumeration.class, converter);
+        Map options = new HashMap();
+        options.put(Library.OPTION_TYPE_MAPPER, mapper);
+        EnumerationTestLibrary lib = (EnumerationTestLibrary)
+            Native.loadLibrary("testlib", EnumerationTestLibrary.class, options);
+        assertEquals("Enumeration improperly converted", Enumeration.STATUS_1, lib.returnInt32Argument(Enumeration.STATUS_1));
+    }
+
     public static void main(String[] args) {
         junit.textui.TestRunner.run(TypeMapperTest.class);
     }