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

change armSoftFloat condition in ELFAnalyser #863

Merged
merged 1 commit into from
Oct 28, 2017

Conversation

kunkun26
Copy link
Contributor

@kunkun26 kunkun26 commented Oct 4, 2017

Hi, i will send PR related ELFAnalyser. Please refer to the followings.

ELF for the ARM Architecture says

If both
EF_ARM_ABI_FLOAT_XXXX bits are clear, conformance to the base
procedure-call standard is implied.

The base standard is software-floating. But now, the condition of armSoftFloat is whether EF_ARM_ABI_FLOAT_SOFT is exists or not. So, if jna is called from a program based on JDK without both EF_ARM_ABI_FLOAT_XXXX bits (e.g Oracle Java Embedded, a homemade JDK), jna select lib for hardware-floating and do not work correctly.

So, I changed the condition of armSoftFloat.

Copy link
Member

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Your argument is sound and the reference to the documentation appreciated!

Before this can be merged, I'd like to ask you:

  • add a comment to CHANGES.md and add this as a bugfix entry. Please see the other entries as reference for the structure.
  • squash the commits together into one (rewrite the history and do a force push on this branch)

@@ -104,5 +116,16 @@ private static void extractFileFromZip(File zipTarget, String zipEntryName, File
zip.close();
}
}

private static void makeLinuxArmelNoflagLib(File sourceFile, File outputFile) throws IOException {
final int POS_ABI_FLOAT_BIT = (byte) 0x25;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment would be helpful here to understand why this works. Something about the lines:
The e_flags for elf arm binaries begin at an offset of 0x24 bytes. The procedure call standard is coded on the second byte.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, thank you for your advice.

@kunkun26
Copy link
Contributor Author

kunkun26 commented Oct 8, 2017

thank you for your comments and just a moment please.
i will fix your comment asap.

@matthiasblaesing
Copy link
Member

@kunkun26 are you working on this? Can I help you?

@kunkun26
Copy link
Contributor Author

sorry for to be late.
Fixed your commets and please check it.

@kunkun26
Copy link
Contributor Author

@matthiasblaesing
I want to apply this bug to 4.5.0.
So, I plan to publish jna 4.5.x which is applied this bug, from own repository to Nexus.

I could get grid for sonatype.
Then i will chage both of pom-jna and pom-jna-platform.

Are there any changes i should ?
Please give me your advices.

@kunkun26
Copy link
Contributor Author

[junit] expected:<0> but was:<1>
[junit] junit.framework.AssertionFailedError: expected:<0> but was:<1>
[junit] at com.sun.jna.MemoryTest.testRemoveAllocatedMemoryMap(MemoryTest.java:200)

i did not change related the above test and tests successfully on OSX in my local.
What should i do ?

@matthiasblaesing
Copy link
Member

@kunkun26 looks good. I'll pull this into master (for 5.0.0) and see, that I also create a 4.5.1 bugfix branch.
It'll take a few days though.

@matthiasblaesing matthiasblaesing merged commit 52eb1fd into java-native-access:master Oct 28, 2017
@matthiasblaesing
Copy link
Member

I integrated the changes into master and created 4.5.1-dev branch. I'll give it a few days to settle and will look into a release from that branch.

@matthiasblaesing
Copy link
Member

@kunkun26 I saw that you pushed a custom build to maven central. I'd have preferred if you'd have waited for a release. Now there will be two different 4.5.1 JNA builds in the wild and this might cause confusion. This project is maintained and if you needed an immediate release asking on the mailing list would have been the preferred way.

@kunkun26
Copy link
Contributor Author

kunkun26 commented Dec 7, 2017

@matthiasblaesing
Thank you for your comment. And very sorry to push JNA to Maven Cantral.
As you mentioned I already pushed homemade JNA 4.5.1 in my own GroupId. And i use it in my project.
If official JNA 4.5.1 is published. i plan to use your JNA builds.
But mybe i cannot delete my homemade JNA 4.5.1 from maven central.

Should i ask Sonatype to delete my JNA ?

@bhamail
Copy link
Contributor

bhamail commented Dec 7, 2017

Probably best to ask to remove it. I think dead forks like that one will only cause confusion.

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 this pull request may close these issues.

3 participants