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

Add mappings for Udev Library. #1200

Closed
wants to merge 4 commits into from

Conversation

dbwiddis
Copy link
Contributor

See references here.

@matthiasblaesing
Copy link
Member

The bindings looks sane to me. Looking at the API I'm thinking, that this is basicly a OO model. This might be an optional enhancement (not finished, but should give the idea):

matthiasblaesing@ba02d6b

@dbwiddis
Copy link
Contributor Author

Great idea. I'll update. One thing to note, the docs specify return types for both the *_ref() and *_unref() functions: ref returns the unmodified object and unref "always returns null". I do agree with changing the unref return to void (since testing shows it doesn't actually return null as advertised) but I think the ref version should return this;.

@dbwiddis
Copy link
Contributor Author

Hmm, I expect the "double-free" is me trying to release parent reference. This doc is confusing but I suppose it means "don't unref it"...

On success, udev_device_get_parent() and udev_device_get_parent_with_subsystem_devtype() return a pointer to the parent device. No additional reference to this device is acquired, but the child device owns a reference to such a parent device.

@dbwiddis dbwiddis force-pushed the udev branch 5 times, most recently from 719ecb4 to 67030b9 Compare May 22, 2020 03:06
@dbwiddis
Copy link
Contributor Author

Sorry for the 8 or 9 force pushes. The good news is my own appveyor setup is pretty strict on javadocs. The bad news is that it was killing the build too quickly for me to see the errors in the logs and I kept finding and fixing a few at a time. It should be good to go now.

* @param udev
* A udev context object.
*/
void udev_unref(UdevContext udev);
Copy link
Member

Choose a reason for hiding this comment

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

This is not correct - the definition needs to match the C version. I dropped the return in my suggestion for the OO binding - but only on the java level, not on the Java<->C interface level. Depending on how the return value is returned, memory might need to be reserved for this. If it does not happen, strange issues could result, so please return the UdevContext here.

The same is true for all callsites, where Structure was turned to void.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, I missed that subtle distinction.

@matthiasblaesing
Copy link
Member

Merged - thank you

@dbwiddis dbwiddis deleted the udev branch January 11, 2022 22:13
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

2 participants