Merge pull request #80 from headius/error_library_options

Improvements and fixes for errno saving to fix #78.

src/main/java/jnr/ffi/LibraryLoader.java

@@ -77,6 +77,41 @@ protected LibraryLoader(Class<T> interfaceClass) {
         this.interfaceClass = interfaceClass;
     }
 
+    /**
+     * When either the {@link jnr.ffi.annotations.SaveError} or
+     * {@link jnr.ffi.annotations.IgnoreError} annotations are used, the
+     * following matrix applies:
+     *
+     * (SL = save at library level, IM = ignore at method level, etc)
+     *
+     * <pre>
+     *         | none |  SL  |  IL  | SL+IL|
+     * -------------------------------------
+     * none    | save | save | ignr | save |
+     * SM      | save | save | save | save |
+     * IM      | ignr | ignr | ignr | ignr |
+     * SM + IM | save | save | save | save |
+     * </pre>
+     */
+    public static boolean saveError(Map<LibraryOption, ?> options, boolean methodHasSave, boolean methodHasIgnore) {
+
+        // default to save
+        boolean saveError = options.containsKey(LibraryOption.SaveError) || !options.containsKey(LibraryOption.IgnoreError);
+
+        // toggle only according to above matrix
+        if (saveError) {
+            if (methodHasIgnore && !methodHasSave) {
+                saveError = false;
+            }
+        } else {
+            if (methodHasSave) {
+                saveError = true;
+            }
+        }
+
+        return saveError;
+    }
+
     /**
      * Adds a library to be loaded.  Multiple libraries can be specified using additional calls
      * to this method, and all libraries will be searched to resolve symbols (e.g. functions, variables).

src/main/java/jnr/ffi/LibraryOption.java

@@ -18,6 +18,8 @@
 
 package jnr.ffi;
 
+import java.util.Map;
+
 /**
  * Options that apply to a library
  */
@@ -26,12 +28,17 @@ public enum LibraryOption {
      * Function calls should save the errno/last error after the call.
      * This option can be overridden on individual methods by use of the 
      * {@link jnr.ffi.annotations.IgnoreError} annotation.
+     *
+     * @see LibraryLoader#saveError(Map, boolean, boolean)
      */
     SaveError,
+
     /**
      * Function calls should NOT save the errno/last error after the call.
      * This option can be overridden on individual methods by use of the 
      * {@link jnr.ffi.annotations.SaveError} annotation.
+     *
+     * @see LibraryLoader#saveError(Map, boolean, boolean)
      */
     IgnoreError,
 

src/main/java/jnr/ffi/annotations/IgnoreError.java

@@ -18,8 +18,12 @@
 
 package jnr.ffi.annotations;
 
+import jnr.ffi.LibraryLoader;
+import jnr.ffi.LibraryOption;
+
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
+import java.util.Map;
 
 /**
  * Indicates that the errno value for a native function need not be saved after
@@ -37,6 +41,7 @@
  * avoid unneccessary saving of the errno value.
  *
  * @see SaveError
+ * @see LibraryLoader#saveError(Map, boolean, boolean)
  */
 @Retention(RetentionPolicy.RUNTIME)
 public @interface IgnoreError {

src/main/java/jnr/ffi/annotations/SaveError.java

@@ -18,14 +18,19 @@
 
 package jnr.ffi.annotations;
 
+import jnr.ffi.LibraryLoader;
+import jnr.ffi.LibraryOption;
+
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
+import java.util.Map;
 
 /**
  * Tags a library method as requiring any error codes as returned
  * by errno on unix, or GetLastError on windows be saved.
  *
  * @see IgnoreError
+ * @see LibraryLoader#saveError(Map, boolean, boolean)
  */
 @Retention(RetentionPolicy.RUNTIME)
 public @interface SaveError {

src/main/java/jnr/ffi/provider/NativeFunction.java

@@ -32,20 +32,16 @@ public final class NativeFunction {
     private final Method method;
     private final Collection<Annotation> annotations;
     private final boolean saveError;
+    private final boolean ignoreError;
     private final CallingConvention callingConvention;
 
     public NativeFunction(Method method, CallingConvention callingConvention) {
         this.method = method;
         this.annotations = Collections.unmodifiableCollection(Arrays.asList(method.getAnnotations()));
-        boolean saveError = true;
-        for (Annotation a : annotations) {
-            if (a instanceof IgnoreError) {
-                saveError = false;
-            } else if (a instanceof SaveError) {
-                saveError = true;
-            }
-        }
-        this.saveError = saveError;
+
+        this.saveError = hasSaveError(method);
+        this.ignoreError = hasIgnoreError(method);
+
         this.callingConvention = callingConvention;
     }
 
@@ -62,10 +58,26 @@ public String name() {
     }
 
     public boolean isErrnoRequired() {
+        return !ignoreError || saveError;
+    }
+
+    public boolean hasSaveError() {
         return saveError;
     }
 
+    public boolean hasIgnoreError() {
+        return ignoreError;
+    }
+
     public Method getMethod() {
         return method;
     }
+
+    public static boolean hasSaveError(Method method) {
+        return method.getAnnotation(SaveError.class) != null;
+    }
+
+    public static boolean hasIgnoreError(Method method) {
+        return method.getAnnotation(IgnoreError.class) != null;
+    }
 }

src/main/java/jnr/ffi/provider/jffi/AsmLibraryLoader.java

@@ -128,12 +128,14 @@ private <T> T generateInterfaceImpl(final NativeLibrary library, Class<T> interf
 
                 ParameterType[] parameterTypes = getParameterTypes(runtime, typeMapper, function.getMethod());
 
+                boolean saveError = jnr.ffi.LibraryLoader.saveError(libraryOptions, function.hasSaveError(), function.hasIgnoreError());
+
                 Function jffiFunction = new Function(functionAddress, 
-                        getCallContext(resultType, parameterTypes,function.convention(), function.isErrnoRequired())); 
+                        getCallContext(resultType, parameterTypes,function.convention(), saveError));
 
                 for (MethodGenerator g : generators) {
                     if (g.isSupported(resultType, parameterTypes, function.convention())) {
-                        g.generate(builder, function.getMethod().getName(), jffiFunction, resultType, parameterTypes, !function.isErrnoRequired());
+                        g.generate(builder, function.getMethod().getName(), jffiFunction, resultType, parameterTypes, !saveError);
                         break;
                     }
                 }

src/main/java/jnr/ffi/provider/jffi/InvokerUtil.java

@@ -28,6 +28,7 @@
 import jnr.ffi.annotations.SaveError;
 import jnr.ffi.annotations.StdCall;
 import jnr.ffi.mapper.*;
+import jnr.ffi.provider.NativeFunction;
 import jnr.ffi.provider.ParameterType;
 import jnr.ffi.provider.ResultType;
 import jnr.ffi.provider.SigType;
@@ -42,18 +43,6 @@
 
 final class InvokerUtil {
 
-    public static boolean requiresErrno(Method method) {
-        boolean saveError = true;
-        for (Annotation a : method.getAnnotations()) {
-            if (a instanceof IgnoreError) {
-                saveError = false;
-            } else if (a instanceof SaveError) {
-                saveError = true;
-            }
-        }
-        return saveError;
-    }
-
     public static jnr.ffi.CallingConvention getCallingConvention(Map<LibraryOption, ?> libraryOptions) {
         Object convention = libraryOptions.get(LibraryOption.CallingConvention);
 

src/main/java/jnr/ffi/provider/jffi/LibraryLoader.java

@@ -19,9 +19,11 @@
 package jnr.ffi.provider.jffi;
 
 import jnr.ffi.LibraryOption;
+import jnr.ffi.provider.NativeFunction;
 
 import java.util.Map;
 
 public abstract class LibraryLoader {
+
     abstract <T> T loadLibrary(NativeLibrary library, Class<T> interfaceClass, Map<LibraryOption, ?> libraryOptions);
 }

src/main/java/jnr/ffi/provider/jffi/ReflectionLibraryLoader.java

@@ -180,8 +180,11 @@ private Invoker getFunctionInvoker(Method method) {
             CallingConvention callingConvention = method.isAnnotationPresent(StdCall.class)
                     ? CallingConvention.STDCALL : libraryCallingConvention;
 
+            // If ignore has been specified and save error has not, ignore error; otherwise, library default
+            boolean saveError = jnr.ffi.LibraryLoader.saveError(libraryOptions, NativeFunction.hasSaveError(method), NativeFunction.hasIgnoreError(method));
+
             Function function = new Function(functionAddress,
-                    getCallContext(resultType, parameterTypes, callingConvention, InvokerUtil.requiresErrno(method)));
+                    getCallContext(resultType, parameterTypes, callingConvention, saveError));
 
             Invoker invoker = invokerFactory.createInvoker(runtime, library, function, resultType, parameterTypes);
 

src/test/java/jnr/ffi/LastErrorTest.java

@@ -18,48 +18,87 @@
 
 package jnr.ffi;
 
+import jnr.ffi.annotations.IgnoreError;
 import jnr.ffi.annotations.SaveError;
-import org.junit.After;
-import org.junit.AfterClass;
-import org.junit.Before;
-import org.junit.BeforeClass;
 import org.junit.Test;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
 import static org.junit.Assert.*;
 
 /**
  *
  */
 public class LastErrorTest {
-    public static interface TestLib {
+    public interface ErrorSavingUnspecified {
+        int setLastError(int error);
+    }
+
+    public interface MethodSaveError extends ErrorSavingUnspecified {
         @SaveError
         int setLastError(int error);
     }
-
-    public LastErrorTest() {
+
+    public interface MethodIgnoreError extends ErrorSavingUnspecified {
+        @IgnoreError
+        int setLastError(int error);
     }
 
-    @BeforeClass
-    public static void setUpClass() throws Exception {
+    public interface MethodIgnoreAndSaveError extends ErrorSavingUnspecified {
+        @IgnoreError
+        @SaveError
+        int setLastError(int error);
     }
 
-    @AfterClass
-    public static void tearDownClass() throws Exception {
+    @Test
+    public void testLastError() {
+        assertSavesError(true, ErrorSavingUnspecified.class);
+        assertSavesError(true, MethodSaveError.class);
+        assertSavesError(false, MethodIgnoreError.class);
+        assertSavesError(true, MethodIgnoreAndSaveError.class);
+
+        assertSavesError(false, ErrorSavingUnspecified.class, LibraryOption.IgnoreError);
+        assertSavesError(true, MethodSaveError.class, LibraryOption.IgnoreError);
+        assertSavesError(false, MethodIgnoreError.class, LibraryOption.IgnoreError);
+        assertSavesError(true, MethodIgnoreAndSaveError.class, LibraryOption.IgnoreError);
+
+        assertSavesError(true, ErrorSavingUnspecified.class, LibraryOption.SaveError);
+        assertSavesError(true, MethodSaveError.class, LibraryOption.SaveError);
+        assertSavesError(false, MethodIgnoreError.class, LibraryOption.SaveError);
+        assertSavesError(true, MethodIgnoreAndSaveError.class, LibraryOption.SaveError);
+
+        assertSavesError(true, ErrorSavingUnspecified.class, LibraryOption.IgnoreError, LibraryOption.SaveError);
+        assertSavesError(true, MethodSaveError.class, LibraryOption.IgnoreError, LibraryOption.SaveError);
+        assertSavesError(false, MethodIgnoreError.class, LibraryOption.IgnoreError, LibraryOption.SaveError);
+        assertSavesError(true, MethodIgnoreAndSaveError.class, LibraryOption.IgnoreError, LibraryOption.SaveError);
     }
 
-    @Before
-    public void setUp() {
+    void assertSavesError(boolean expected, Class<? extends ErrorSavingUnspecified> cls) {
+        assertSavesError(expected, cls, Collections.<LibraryOption, Object>emptyMap());
     }
 
-    @After
-    public void tearDown() {
+    void assertSavesError(boolean expected, Class<? extends ErrorSavingUnspecified> cls, LibraryOption... options) {
+        Map<LibraryOption, Object> optionsMap = new HashMap<LibraryOption, Object>();
+        for (LibraryOption option : options) optionsMap.put(option, option);
+        assertSavesError(expected, cls, optionsMap);
     }
-
-    @Test public void testLastError() {
-        TestLib lib = TstUtil.loadTestLib(TestLib.class);
-        Runtime runtime = Runtime.getRuntime(lib);
+
+    void assertSavesError(boolean expected, Class<? extends ErrorSavingUnspecified> cls, Map<LibraryOption, ?> options) {
+        ErrorSavingUnspecified methodSaveError = TstUtil.loadTestLib(cls, options);
+        Runtime runtime = Runtime.getRuntime(methodSaveError);
+
+        // clear errno
+        runtime.setLastError(0);
 
         final int MAGIC = 0xdeadbeef;
-        lib.setLastError(MAGIC);
-        assertEquals("Wrong errno value", MAGIC, runtime.getLastError());
+        methodSaveError.setLastError(MAGIC);
+
+        if (expected) {
+            assertEquals("Errno value was not saved for " + cls, MAGIC, runtime.getLastError());
+        } else {
+            assertNotEquals("Errno value was saved for " + cls, MAGIC, runtime.getLastError());
+        }
     }
 }