Skip to content

change armSoftFloat condition in ELFAnalyser#863

Merged
matthiasblaesing merged 1 commit intojava-native-access:masterfrom
kunkun26:master
Oct 28, 2017
Merged

change armSoftFloat condition in ELFAnalyser#863
matthiasblaesing merged 1 commit intojava-native-access:masterfrom
kunkun26:master

Conversation

@kunkun26
Copy link
Copy Markdown
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.

matthiasblaesing
matthiasblaesing requested changes Oct 5, 2017
View reviewed changes
Copy link
Copy Markdown
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)

Comment thread test/com/sun/jna/ELFAnalyserTest.java
}

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

@matthiasblaesing matthiasblaesing Oct 5, 2017

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
Copy Markdown
Contributor Author

@kunkun26 kunkun26 Oct 8, 2017

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
Copy Markdown
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
Copy Markdown
Member

matthiasblaesing commented Oct 23, 2017

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

@kunkun26
Copy link
Copy Markdown
Contributor Author

kunkun26 commented Oct 26, 2017

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

@kunkun26
Copy link
Copy Markdown
Contributor Author

kunkun26 commented Oct 26, 2017

@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
Copy Markdown
Contributor Author

kunkun26 commented Oct 26, 2017

[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
Copy Markdown
Member

matthiasblaesing commented Oct 26, 2017

@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
Copy Markdown
Member

matthiasblaesing commented Oct 28, 2017

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
Copy Markdown
Member

matthiasblaesing commented Dec 3, 2017

@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
Copy Markdown
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
Copy Markdown
Contributor

bhamail commented Dec 7, 2017

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

@matthiasblaesing matthiasblaesing mentioned this pull request Feb 5, 2018
Closed
@matthiasblaesing matthiasblaesing mentioned this pull request May 16, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matthiasblaesing matthiasblaesing matthiasblaesing requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kunkun26 @matthiasblaesing @bhamail