Dynamically load libudev in linux-hidraw backend - #788
Conversation
|
The build failed under Ubuntu and Arch Linux. |
b86c642 to
de4b582
Compare
Related to more strict warnings, likely fixed now. |
Signed-off-by: falkTX <falktx@falktx.com>
Not yet... |
de4b582 to
8fc87a7
Compare
I built locally with the same pedantic flags and pushed another try. |
There was a problem hiding this comment.
I don't mind the idea of having libudev loaded dynamically in runtime, but I can't agree that the current aproach is best for that. A few points I'd ask to change if I where to accept it this way:
- licence shouldn't derive from libudev license - we don't copy-paste their code, and HIDAPI uses a bit different licencing sceme, and the new source that becomes a part of HIDAPI would potentially restrict its users;
- including the
.cfile for this purpose looks a bit ugly, and the namelibudev.csuggests that it actually is (i.e. implements) libudev, but in fact it is only a wrapper; what is best instead - have ahidapi_libudev_wrap.h, and all functions inside should be either static or inline; - the
C()macro... I checked - it is a valid C, but the fact that I had to go and double-check suggests that it looks like something that might break;HIDAPI_UDEV_RESOLVEwouldn't raise any naming suspicious RTLD_GLOBAL- that is a code smell to use it here, where passing the_lib(or ratherudev_lib_handletodlsym) explicitly is what I'd expect- no
dlclosewhich is technically a resource leak if HIDAPI itself is ever to be dynamically loaded
At the end of the day - I'd be much happier to see a more common way of dynamically loading a library during the initialisation, saving and using function pointers explicitly and unloading a library during the hid_exit, e.g. like we did it for windows backend.
Yes that would cause a lot more changes in linux/hid.c - but that's absolutely fine.
|
That is all very fair, I mostly did this change as a way to get my cross-compilation builds working and decided to push it upstream because why not. You have good points for something that would be integrated officially in hidapi, but sorry to say my needs are covered already as-is, the steps you mention make sense but sound like it would be quite a bit more work than I am willing to spend just for a small feature. If you think the feature is worthwhile, take all the code here to adapt freely under any license, I give written consent. |
|
Thanks for the contribution. I'll leave it be for now, in case anyone wants to catch up. |
Load libudev.so.1 through a scoped runtime handle, resolve the required entry points, and clean up partial and successful initialization state. Remove the build-time libudev development dependency and cover reload, failure recovery, and balanced unload behavior in Linux CI. Assisted-by: codex:gpt-5.6-sol
Resolve the Linux loader and CI workflow conflicts while preserving the upstream virtual-device tests and the PR's loader lifecycle coverage. Assisted-by: codex:gpt-5.6-sol
Resolve workflow and pkg-config conflicts, preserve private libdl linkage, and remove legacy author names from the new loader test files. Assisted-by: codex:gpt-5.6-sol
Refresh copyright notices in the Linux loader sources changed by this PR. Assisted-by: codex:gpt-5.6-sol
|
Anyone care to try this out? |
Just wondering how to properly test this PR. Thanks. |
|
This PR removes the direct build and link dependency on libudev from the Linux hidraw backend. The backend now loads
libudev.so.1at runtime, so applications that link hidapi do not inherit a direct libudev dependency. Systems using the hidraw backend still need the libudev runtime library.Implementation
RTLD_NOW | RTLD_LOCALhandle.hid_error().hid_exit().libudev1on Ubuntu andeudev-libson Alpine).dlopen/dlclosecleanup.Validation
Assisted-by: codex:gpt-5.6-sol