Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New LibC.SysInfo FieldOrder overrides don't work #1027

Closed
dbwiddis opened this issue Oct 15, 2018 · 1 comment
Closed

New LibC.SysInfo FieldOrder overrides don't work #1027

dbwiddis opened this issue Oct 15, 2018 · 1 comment

Comments

@dbwiddis
Copy link
Contributor

dbwiddis commented Oct 15, 2018

For reference, see discussion on #997.

I just updated my project to JNA 5.0.0 and went through the process of removing classes I implemented locally that are now in JNA. The LibC.SysInfo class that I submitted seems to not behave as we intended. I had all the code implemented in my local class except for the @FieldOrder annotation, so inclusion of the annotation (in the JNA class) is causing test failures.

Multiple Travis CI tests fail with the same underlying error:

java.lang.Error: Structure.getFieldOrder() on class com.sun.jna.platform.linux.LibC$Sysinfo does not provide enough names [13] ([_f, bufferram, freehigh, freeram, freeswap, loads, mem_unit, procs, sharedram, totalhigh, totalram, totalswap, uptime]) to match declared fields [12] ([bufferram, freehigh, freeram, freeswap, loads, mem_unit, procs, sharedram, totalhigh, totalram, totalswap, uptime])

Ignoring the strange wording (not enough when 13 > 12 should be "too many") it does look like the intent to omit the _f from a 64-bit build isn't working correctly. I suspect the error is probably on line 92-94:

            if (PADDING_SIZE > 0) {
                fieldOrder.add("_f");
            }

Given the existing annocation the _f is already there, so I think this conditional should really check if it's equal to zero and remove the _f (as is done in the getFieldList() method preceding it.)

Happy to submit a PR to fix this, but wanted to get confirmation that's the right way to go, as this FieldOrder stuff still seems like a little bit of black magic to me.

Separately, I'm surprised this isn't being caught in a test case... or perhaps the test case is being run on 32-bit and is rather happy with the padding.

@matthiasblaesing
Copy link
Member

Your diagnosis is correct and the solution is sensible. Please also patch build.xml of platform to include the linux package in unittests. This also answers your last question. The problem was not seen, because the unittest was not run, because Linux was handled as "plain" unix.

# This patch file was generated by NetBeans IDE
# It uses platform neutral UTF-8 encoding and \n newlines.
--- a/contrib/platform/build.xml
+++ b/contrib/platform/build.xml
@@ -169,16 +169,22 @@
       <property name="results.junit" location="${build}/junit-results/${os.prefix}"/>
       <mkdir dir="${results.junit}"/>
       <echo>Saving test results in ${results.junit}</echo>
-      <condition property="tests.platform" value="**/mac/**/*Test.java">
+      <condition property="tests.platform.mac" value="**/mac/**/*Test.java">
         <os family="mac"/>
       </condition>
-      <condition property="tests.platform" value="**/win32/**/*Test.java">
+      <condition property="tests.platform.windows" value="**/win32/**/*Test.java">
         <os family="windows"/>
       </condition>
-      <condition property="tests.platform" value="**/unix/**/*Test.java">
+      <condition property="tests.platform.linux" value="**/linux/**/*Test.java">
+        <os name="Linux"/>
+      </condition>
+      <condition property="tests.platform.unix" value="**/unix/**/*Test.java">
         <os family="unix"/>
       </condition>
-      <property name="tests.platform" value=""/>
+      <property name="tests.platform.mac" value=""/>
+      <property name="tests.platform.windows" value=""/>
+      <property name="tests.platform.linux" value=""/>
+      <property name="tests.platform.unix" value=""/>
       <property name="tests.exclude" value=""/>
       <property name="tests.exclude-patterns" value=""/>
       <condition property="java.awt.headless" value="true">
@@ -194,7 +200,10 @@
       <propertyset id="headless">
         <propertyref prefix="java.awt.headless"/>
       </propertyset>
-      <echo>tests.platform=${tests.platform}</echo>
+      <echo>tests.platform.mac=${tests.platform.mac}</echo>
+      <echo>tests.platform.windows=${tests.platform.windows}</echo>
+      <echo>tests.platform.linux=${tests.platform.linux}</echo>
+      <echo>tests.platform.unix=${tests.platform.unix}</echo>
       <junit fork="${test.fork}" failureproperty="testfailure" tempdir="${build.dir}">
         <!-- optionally run headless -->
         <syspropertyset refid="headless"/>
@@ -211,7 +220,10 @@
             <!-- Until StructureFieldOrderTest gets fixed up a little -->
             <exclude name="**/StructureFieldOrderTest.java"/>
             <exclude name="com/sun/jna/platform/AbstractWin32TestSupport.java"/>
-            <include name="${tests.platform}"/>
+            <include name="${tests.platform.mac}"/>
+            <include name="${tests.platform.windows}"/>
+            <include name="${tests.platform.linux}"/>
+            <include name="${tests.platform.unix}"/>
             <exclude name="${tests.exclude}"/>
           </fileset>
         </batchtest>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants