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

Solaris Kstat library #784

Merged
merged 6 commits into from
Apr 9, 2017
Merged

Conversation

dbwiddis
Copy link
Contributor

@dbwiddis dbwiddis commented Apr 2, 2017

No description provided.

@dbwiddis
Copy link
Contributor Author

dbwiddis commented Apr 2, 2017

This is failing Travis CI tests because it can't find libkstat which is only present on Solaris (SunOS). Either jna-platform's build.xml line 154 should exclude the package location I put these classes in (but I'm not sure how to do that), or I should move the packages, but I'm not sure the best place to put them.

@twall
Copy link
Contributor

twall commented Apr 3, 2017 via email

@dbwiddis
Copy link
Contributor Author

dbwiddis commented Apr 8, 2017

The libkstat should always be there on Solaris but not other Unices. I added a check for Platform.isSolaris() on the tests and Travis seems happy now.

Should be ready to merge or, if desired:

  • I wrote several convenience methods to access the library that I have in my own application in a util class, that I included in the test class as private methods. I think this is probably sufficient for reference for anyone wanting to use this library. If desired I can split those methods out into a separate util class, however.

@matthiasblaesing
Copy link
Member

Thank you - I'm not an expert but I had a quick look at the documentation and the structures and they look sane.

Could you please adjust the license headers of the newly added files to align with the AL2.0 + LGPL2.1 and newer combo license. See here:

https://github.com/java-native-access/jna/blob/master/www/Contributing.md#copyright-headers-in-files

With regard to the utility methods: Most C interfaces feel awkward when called from java and utility methods wrapping them help IMHO. If you think it is worth it, please go ahead.

@dbwiddis
Copy link
Contributor Author

dbwiddis commented Apr 8, 2017

Headers updated. I left the utility methods in test since they would have log messages (or fail() messages in the test), and I don't see that JNA has any logging functionality.

@matthiasblaesing matthiasblaesing merged commit b0306c7 into java-native-access:master Apr 9, 2017
@matthiasblaesing
Copy link
Member

Unittests run cleanly on solaris 11.3, structures look sane, requested changes to new files were done.

Thank you, merged.

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.

None yet

3 participants