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 --sysdev/-y option for direct device node access #600

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions uhubctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,9 @@ static int opt_force = 0; /* force operation even on unsupported hubs */
static int opt_nodesc = 0; /* skip querying device description */
#if defined(__gnu_linux__) || defined(__linux__)
static int opt_nosysfs = 0; /* don't use the Linux sysfs port disable interface, even if available */
#if defined(LIBUSB_API_VERSION) && (LIBUSB_API_VERSION >= 0x01000107)
static const char *opt_sysdev;
#endif
#endif

/* For Raspberry Pi detection and workarounds: */
Expand All @@ -232,8 +235,12 @@ static int is_rpi_5 = 0;
static const char short_options[] =
"l:L:n:a:p:d:r:w:s:hvefRN"
#if defined(__gnu_linux__) || defined(__linux__)
#if defined(LIBUSB_API_VERSION) && (LIBUSB_API_VERSION >= 0x01000107)
tormodvolden marked this conversation as resolved.
Show resolved Hide resolved
"Su:"
#else
"S"
#endif
#endif
;

static const struct option long_options[] = {
Expand All @@ -251,6 +258,9 @@ static const struct option long_options[] = {
{ "nodesc", no_argument, NULL, 'N' },
#if defined(__gnu_linux__) || defined(__linux__)
{ "nosysfs", no_argument, NULL, 'S' },
#if defined(LIBUSB_API_VERSION) && (LIBUSB_API_VERSION >= 0x01000107)
{ "sysdev", required_argument, NULL, 'u' },
tormodvolden marked this conversation as resolved.
Show resolved Hide resolved
#endif
#endif
{ "reset", no_argument, NULL, 'R' },
{ "version", no_argument, NULL, 'v' },
Expand Down Expand Up @@ -280,6 +290,9 @@ static int print_usage(void)
"--nodesc, -N - do not query device description (helpful for unresponsive devices).\n"
#if defined(__gnu_linux__) || defined(__linux__)
"--nosysfs, -S - do not use the Linux sysfs port disable interface.\n"
#if defined(LIBUSB_API_VERSION) && (LIBUSB_API_VERSION >= 0x01000107)
"--sysdev, -u - open system device node instead of scanning.\n"
#endif
#endif
"--reset, -R - reset hub after each power-on action, causing all devices to reassociate.\n"
"--wait, -w - wait before repeat power off [%d ms].\n"
Expand Down Expand Up @@ -1099,6 +1112,12 @@ int main(int argc, char *argv[])
int rc;
int c = 0;
int option_index = 0;
#if defined(LIBUSB_API_VERSION) && (LIBUSB_API_VERSION >= 0x01000107) && \
(defined(__gnu_linux__) || defined(__linux__))
int sys_fd;
libusb_device_handle *sys_devh = NULL;
libusb_device *dev_list[2];
Copy link
Owner

Choose a reason for hiding this comment

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

Please move this variable definition into if (opt_sysdev) block - the only place it is used

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

Let me know what you think about the calloc variant I pushed now.

#endif

for (;;) {
c = getopt_long(argc, argv, short_options, long_options, &option_index);
Expand Down Expand Up @@ -1167,6 +1186,11 @@ int main(int argc, char *argv[])
case 'S':
opt_nosysfs = 1;
break;
#if defined(LIBUSB_API_VERSION) && (LIBUSB_API_VERSION >= 0x01000107)
case 'u':
opt_sysdev = optarg;
break;
#endif
#endif
case 'e':
opt_exact = 1;
Expand Down Expand Up @@ -1209,7 +1233,30 @@ int main(int argc, char *argv[])
exit(1);
}

#if defined(LIBUSB_API_VERSION) && (LIBUSB_API_VERSION >= 0x01000107) && \
(defined(__gnu_linux__) || defined(__linux__))
if (opt_sysdev) {
sys_fd = open(opt_sysdev, O_RDWR);
if (sys_fd < 0) {
fprintf(stderr, "Cannot open system node!\n");
rc = 1;
goto cleanup;
}
rc = libusb_wrap_sys_device(NULL, sys_fd, &sys_devh);
if (rc != 0) {
fprintf(stderr, "Cannot wrap system node!\n");
tormodvolden marked this conversation as resolved.
Show resolved Hide resolved
rc = 1;
goto cleanup;
}
dev_list[0] = libusb_get_device(sys_devh);
dev_list[1] = NULL;
usb_devs = dev_list;
} else {
rc = libusb_get_device_list(NULL, &usb_devs);
}
#else
rc = libusb_get_device_list(NULL, &usb_devs);
#endif
if (rc < 0) {
fprintf(stderr,
"Cannot enumerate USB devices!\n"
Expand Down Expand Up @@ -1315,8 +1362,18 @@ int main(int argc, char *argv[])
}
rc = 0;
cleanup:
#if defined(LIBUSB_API_VERSION) && (LIBUSB_API_VERSION >= 0x01000107) && \
(defined(__gnu_linux__) || defined(__linux__))
if (opt_sysdev && sys_fd >= 0) {
if (sys_devh)
libusb_close(sys_devh);
close(sys_fd);
} else if (usb_devs)
libusb_free_device_list(usb_devs, 1);
#else
if (usb_devs)
libusb_free_device_list(usb_devs, 1);
#endif
usb_devs = NULL;
libusb_exit(NULL);
return rc;
Expand Down