Add --sysdev/-y option for direct device node access #600
Conversation
Only the device specified by the given device path will be accessed, instead of scanning the USB bus. Most useful if you use udev rules to create stable device node aliases for hubs, independent of bus topology. For instance, if your matching udev rules include SYMLINK+="MYSMARTHUB1" you can call uhubctl with --sysdev /dev/MYSMARTHUB1 instead of using -l and a non-stable bus location to specify it. Signed-off-by: Tormod Volden <[email protected]>
There was a problem hiding this comment.
Thank you for this work! It looks good, but there are few things to be fixed before merging:
- libusb_wrap_sys_device is not Linux only. We should be able to support any OS that libusb suppors, e.g. FreeBSD, Darwin, Solaris. It means applicable code should not be under Linux ifdef.
- I would like to reserve -u/--usb for future functionality. Can you use -y or -Y instead?
uhubctl.c
Outdated
| (defined(__gnu_linux__) || defined(__linux__)) | ||
| int sys_fd; | ||
| libusb_device_handle *sys_devh = NULL; | ||
| libusb_device *dev_list[2]; |
There was a problem hiding this comment.
Please move this variable definition into if (opt_sysdev) block - the only place it is used
There was a problem hiding this comment.
The allocated memory is pointed to by the usb_dev pointer later, so it cannot be free'd before libusb_free_device_list(usb_devs, 1) at the end.
There was a problem hiding this comment.
So it cannot be allocated on the stack inside the if (opt_sysdev) block. I agree this isn't pretty. The alternative is malloc'ing it inside that block instead, but I found it a lot of overhead for 2 measly pointers.
There was a problem hiding this comment.
Let me know what you think about the calloc variant I pushed now.
Thanks for the review! The libusb_wrap_sys_device() is (unfortunately) Linux-only and I don't foresee that changing any time soon.
Sure! I have no preference on -y vs -Y, is there a system to it, or any preference from your side? |
|
I pushed a fixup commit (of -y option renaming for now) for review, but I will of course squash it once ready. |
tormodvolden
changed the title
Only the device specified by the given device path will be accessed, instead of scanning the USB bus.
Most useful if you use udev rules to create stable device node aliases for hubs, independent of bus topology.
For instance, if your matching udev rules include
SYMLINK+="MYSMARTHUB1"you can call uhubctl with--sysdev /dev/MYSMARTHUB1instead of using-land a non-stable bus location to specify it.