Skip to content

Add mappings for Udev Library.#1200

Closed
dbwiddis wants to merge 4 commits intojava-native-access:masterfrom
dbwiddis:udev
Closed

Add mappings for Udev Library.#1200
dbwiddis wants to merge 4 commits intojava-native-access:masterfrom
dbwiddis:udev

Conversation

@dbwiddis
Copy link
Copy Markdown
Contributor

See references here.

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

@matthiasblaesing matthiasblaesing May 23, 2020

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

2 participants