Rework device discovery API libusb_find_devices and libusb_get_devices are no more libusb_get_device_list obtains a list of libusb_device structures for all known devices in the system. Each libusb_device now has a reference count, defaulting to 1 on instantiation. The reference count of 1 refers to the fact that it is present in the list in this scenario. Opening a device adds a pointer to the libusb_device structure in the handle, so that also adds a reference. Closing the device removes that reference. The function to free the device list can optionally unref all the devices inside. In future we will make the libusb_device instances all "global" so that if the app calls get_device_list twice it actually gets the same libusb_device structure references back. This way we can start to track disconnects, and we can investigate adding a unique "session ID" to each libusb_device, an identifier guaranteed to be unique to that device until reboot.
diff --git a/TODO b/TODO index 68adc12..c4ff9b1 100644 --- a/TODO +++ b/TODO
@@ -8,6 +8,10 @@ isochronous endpoint I/O thread safety abstraction for cross-platform-ness +fallback on /proc/bus/usb on linux +error codes +assign session ID to each libusb_device +share libusb_device structures in memory between all get_devices callers for 1.1 or future ================== @@ -21,6 +25,6 @@ struct libusb_(bulk|control)_transfer or parameters? devh in general urbh in general (should this be a transfer handle?) -find/get devices API config struct/function naming typedef _cb or _cb_fn or _cb_t? +typedef as-is or pointers? libusb_dev_t rather than libusb_dev *?
diff --git a/examples/dpfp.c b/examples/dpfp.c index d99b3af..87a7da1 100644 --- a/examples/dpfp.c +++ b/examples/dpfp.c
@@ -21,6 +21,7 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ +#include <errno.h> #include <signal.h> #include <string.h> #include <stdio.h> @@ -78,19 +79,38 @@ .length = sizeof(irqbuf), }; -static struct libusb_dev *find_dpfp_device(void) +static int find_dpfp_device(void) { - struct libusb_dev *dev; + struct libusb_device *dev; + struct libusb_device *found = NULL; + struct libusb_device **devs; + size_t i = 0; + int r; - libusb_find_devices(); + r = libusb_get_device_list(&devs); + if (r < 0) + return r; - for (dev = libusb_get_devices(); dev; dev = libusb_dev_next(dev)) { - struct libusb_dev_descriptor *desc = libusb_dev_get_descriptor(dev); - if (desc->idVendor == 0x05ba && desc->idProduct == 0x000a) - return dev; + while ((dev = devs[i++]) != NULL) { + struct libusb_dev_descriptor *desc = libusb_device_get_descriptor(dev); + if (desc->idVendor == 0x05ba && desc->idProduct == 0x000a) { + found = dev; + break; + } } - return NULL; + if (!found) { + r = -ENODEV; + goto out; + } + + devh = libusb_open(dev); + if (!devh) + r = -EIO; + +out: + libusb_free_device_list(devs, 1); + return r; } static int print_f0_data(void) @@ -469,7 +489,6 @@ int main(void) { - struct libusb_dev *dev; struct sigaction sigact; int r = 1; @@ -479,19 +498,11 @@ exit(1); } - dev = find_dpfp_device(); - if (!dev) { - fprintf(stderr, "No device found\n"); + r = find_dpfp_device(); + if (r < 0) { + fprintf(stderr, "Could not find/open device\n"); goto out; } - printf("found device\n"); - - devh = libusb_open(dev); - if (!devh) { - fprintf(stderr, "Could not open device\n"); - goto out; - } - printf("opened device\n"); r = libusb_claim_interface(devh, 0); if (r < 0) {
diff --git a/examples/lsusb.c b/examples/lsusb.c index c511b29..d592fee 100644 --- a/examples/lsusb.c +++ b/examples/lsusb.c
@@ -21,29 +21,32 @@ #include <libusb/libusb.h> -void print_devs(libusb_dev *devs) +void print_devs(libusb_device **devs) { - libusb_dev *dev; + libusb_device *dev; + int i; - for (dev = devs; dev; dev = libusb_dev_next(dev)) { - struct libusb_dev_descriptor *desc = libusb_dev_get_descriptor(dev); + while ((dev = devs[i++]) != NULL) { + struct libusb_dev_descriptor *desc = libusb_device_get_descriptor(dev); printf("%04x:%04x\n", desc->idVendor, desc->idProduct); } } int main(void) { - libusb_dev *devs; + libusb_device **devs; int r; r = libusb_init(); if (r < 0) return r; - libusb_find_devices(); - devs = libusb_get_devices(); + r = libusb_get_device_list(&devs); + if (r < 0) + return r; print_devs(devs); + libusb_free_device_list(devs, 1); libusb_exit(); return 0;
diff --git a/libusb/core.c b/libusb/core.c index 1560567..cd01515 100644 --- a/libusb/core.c +++ b/libusb/core.c
@@ -37,14 +37,82 @@ #include "libusb.h" #include "libusbi.h" -static struct list_head usb_devs; struct list_head open_devs; -static int scan_device(char *busdir, const char *devnum) +/* we traverse usbfs without knowing how many devices we are going to find. + * so we create this discovered_devs model which is similar to a linked-list + * which grows when required. it can be freed once discovery has completed, + * eliminating the need for a list node in the libusb_device structure + * itself. */ +#define DISCOVERED_DEVICES_SIZE_STEP 8 +struct discovered_devs { + size_t len; + size_t capacity; + struct libusb_device *devices[0]; +}; + +static struct discovered_devs *discovered_devs_alloc(void) +{ + struct discovered_devs *ret = + malloc(sizeof(*ret) + (sizeof(void *) * DISCOVERED_DEVICES_SIZE_STEP)); + + if (ret) { + ret->len = 0; + ret->capacity = DISCOVERED_DEVICES_SIZE_STEP; + } + return ret; +} + +/* append a device to the discovered devices collection. may realloc itself, + * returning new discdevs. returns NULL on realloc failure. */ +static struct discovered_devs *discovered_devs_append( + struct discovered_devs *discdevs, struct libusb_device *dev) +{ + size_t len = discdevs->len; + size_t capacity; + + /* if there is space, just append the device */ + if (len < discdevs->capacity) { + discdevs->devices[len] = dev; + discdevs->len++; + return discdevs; + } + + /* exceeded capacity, need to grow */ + usbi_dbg("need to increase capacity"); + capacity = discdevs->capacity + DISCOVERED_DEVICES_SIZE_STEP; + discdevs = realloc(discdevs, + sizeof(*discdevs) + (sizeof(void *) * capacity)); + if (discdevs) { + discdevs->capacity = capacity; + discdevs->devices[len] = dev; + discdevs->len++; + } + + return discdevs; +} + +static void discovered_devs_free(struct discovered_devs *discdevs) +{ + size_t i; + + for (i = 0; i < discdevs->len; i++) + libusb_device_unref(discdevs->devices[i]); + + free(discdevs); +} + +/* open a device file, set up the libusb_device structure for it, and add it to + * discdevs. on failure (non-zero return) the pre-existing discdevs should + * be destroyed (and devices freed). on success, the new discdevs pointer + * should be used it may have been moved. */ +static int scan_device(struct discovered_devs **_discdevs, + char *busdir, const char *devnum) { char path[PATH_MAX + 1]; unsigned char raw_desc[DEVICE_DESC_LENGTH]; - struct libusb_dev *dev = malloc(sizeof(*dev)); + struct libusb_device *dev = malloc(sizeof(*dev)); + struct discovered_devs *discdevs; int fd = 0; int i; int r; @@ -53,12 +121,18 @@ if (!dev) return -1; + dev->refcnt = 1; + dev->nodepath = NULL; + dev->config = NULL; + snprintf(path, PATH_MAX, "%s/%s", busdir, devnum); usbi_dbg("%s", path); fd = open(path, O_RDWR); if (!fd) { usbi_dbg("open '%s' failed, ret=%d errno=%d", path, fd, errno); r = -1; + /* FIXME this might not be an error if the file has gone away due + * to unplugging */ goto err; } @@ -87,12 +161,11 @@ tmp = dev->desc.bNumConfigurations * sizeof(struct libusb_config_descriptor); dev->config = malloc(tmp); if (!dev->config) { - r = -1; + r = -ENOMEM; goto err; } memset(dev->config, 0, tmp); - for (i = 0; i < dev->desc.bNumConfigurations; i++) { unsigned char buffer[8], *bigbuffer; struct libusb_config_descriptor config; @@ -132,47 +205,74 @@ goto err; usbi_dbg("found device %04x:%04x", dev->desc.idVendor, dev->desc.idProduct); - list_add(&dev->list, &usb_devs); - r = 0; + discdevs = discovered_devs_append(*_discdevs, dev); + if (discdevs) { + *_discdevs = discdevs; + return 0; + } err: if (fd) close(fd); - if (r < 0 && dev) + if (dev->config) + free(dev->config); + if (dev->nodepath) + free(dev->nodepath); + if (dev) free(dev); return r; } -static int scan_busdir(const char *busnum) +/* open a bus directory and adds all discovered devices to discdevs. on + * failure (non-zero return) the pre-existing discdevs should be destroyed + * (and devices freed). on success, the new discdevs pointer should be used + * as it may have been moved. */ +static int scan_busdir(struct discovered_devs **_discdevs, const char *busnum) { DIR *dir; char dirpath[PATH_MAX + 1]; struct dirent *entry; + struct discovered_devs *discdevs = *_discdevs; + int r = 0; snprintf(dirpath, PATH_MAX, "%s/%s", USBFS_PATH, busnum); usbi_dbg("%s", dirpath); dir = opendir(dirpath); if (!dir) { usbi_err("opendir '%s' failed, errno=%d", dirpath, errno); + /* FIXME: should handle valid race conditions like hub unplugged + * during directory iteration - this is not an error */ return -1; } while ((entry = readdir(dir))) { if (entry->d_name[0] == '.') continue; - /* deliberately ignoring errors due to valid unplug race conditions */ - scan_device(dirpath, entry->d_name); + r = scan_device(&discdevs, dirpath, entry->d_name); + if (r < 0) + goto out; } - return 0; + *_discdevs = discdevs; +out: + closedir(dir); + return r; } -API_EXPORTED int libusb_find_devices(void) +API_EXPORTED int libusb_get_device_list(struct libusb_device ***list) { DIR *buses; + struct discovered_devs *discdevs = discovered_devs_alloc(); struct dirent *entry; + struct libusb_device **ret; + int r = 0; + size_t i; + size_t len; usbi_dbg(""); + if (!discdevs) + return -ENOMEM; + buses = opendir(USBFS_PATH); if (!buses) { usbi_err("opendir buses failed errno=%d", errno); @@ -180,44 +280,87 @@ } while ((entry = readdir(buses))) { + struct discovered_devs *discdevs_new = discdevs; + if (entry->d_name[0] == '.') continue; /* deliberately ignoring errors, valid race conditions exist - * e.g. unplugging of hubs in the middle of this loop*/ - scan_busdir(entry->d_name); + * e.g. unplugging of hubs in the middle of this loop */ + r = scan_busdir(&discdevs_new, entry->d_name); + if (r < 0) + goto out; + discdevs = discdevs_new; } - return 0; + /* convert discovered_devs into a list */ + len = discdevs->len; + ret = malloc(sizeof(void *) * (len + 1)); + if (!ret) + goto out; + + ret[len] = NULL; + for (i = 0; i < len; i++) { + struct libusb_device *dev = discdevs->devices[i]; + libusb_device_ref(dev); + ret[i] = dev; + } + *list = ret; + +out: + discovered_devs_free(discdevs); + closedir(buses); + return r; } -API_EXPORTED struct libusb_dev *libusb_get_devices(void) +API_EXPORTED void libusb_free_device_list(struct libusb_device **list, + int unref_devices) { - if (list_empty(&usb_devs)) - return NULL; - return list_entry(usb_devs.next, struct libusb_dev, list); + if (!list) + return; + + if (unref_devices) { + int i = 0; + struct libusb_device *dev; + + while ((dev = list[i++]) != NULL) + libusb_device_unref(dev); + } + free(list); } -API_EXPORTED struct libusb_dev *libusb_dev_next(struct libusb_dev *dev) +API_EXPORTED struct libusb_device *libusb_device_ref(struct libusb_device *dev) { - struct list_head *head = &dev->list; - if (!head || head->next == &usb_devs) - return NULL; - return list_entry(head->next, struct libusb_dev, list); + dev->refcnt++; + return dev; } -API_EXPORTED struct libusb_dev_descriptor *libusb_dev_get_descriptor( - struct libusb_dev *dev) +API_EXPORTED void libusb_device_unref(struct libusb_device *dev) +{ + if (!dev) + return; + + if (--dev->refcnt == 0) { + usbi_dbg("destroy device %04x:%04x", dev->desc.idVendor, + dev->desc.idProduct); + free(dev->config); + free(dev->nodepath); + free(dev); + } +} + +API_EXPORTED struct libusb_dev_descriptor *libusb_device_get_descriptor( + struct libusb_device *dev) { return &dev->desc; } -API_EXPORTED struct libusb_config_descriptor *libusb_dev_get_config( - struct libusb_dev *dev) +API_EXPORTED struct libusb_config_descriptor *libusb_device_get_config( + struct libusb_device *dev) { return dev->config; } -API_EXPORTED struct libusb_dev_handle *libusb_open(struct libusb_dev *dev) +API_EXPORTED struct libusb_dev_handle *libusb_open(struct libusb_device *dev) { struct libusb_dev_handle *devh; int fd; @@ -236,7 +379,7 @@ } devh->fd = fd; - devh->dev = dev; + devh->dev = libusb_device_ref(dev); list_add(&devh->list, &open_devs); usbi_add_pollfd(fd, POLLOUT); return devh; @@ -246,6 +389,7 @@ { usbi_remove_pollfd(devh->fd); close(devh->fd); + libusb_device_unref(devh->dev); } API_EXPORTED void libusb_close(struct libusb_dev_handle *devh) @@ -259,7 +403,8 @@ free(devh); } -API_EXPORTED struct libusb_dev *libusb_devh_get_dev(struct libusb_dev_handle *devh) +API_EXPORTED struct libusb_device *libusb_devh_get_dev( + struct libusb_dev_handle *devh) { return devh->dev; } @@ -292,7 +437,6 @@ { /* FIXME: find correct usb node path */ usbi_dbg(""); - list_init(&usb_devs); list_init(&open_devs); usbi_io_init(); return 0;
diff --git a/libusb/libusb.h b/libusb/libusb.h index 6cfc780..cddc232 100644 --- a/libusb/libusb.h +++ b/libusb/libusb.h
@@ -180,8 +180,8 @@ /* libusb */ -struct libusb_dev; -typedef struct libusb_dev libusb_dev; +struct libusb_device; +typedef struct libusb_device libusb_device; struct libusb_dev_handle; typedef struct libusb_dev_handle libusb_dev_handle; @@ -222,15 +222,16 @@ int libusb_init(void); void libusb_exit(void); -int libusb_find_devices(void); -libusb_dev *libusb_get_devices(void); -struct libusb_dev_descriptor *libusb_dev_get_descriptor(libusb_dev *dev); -struct libusb_config_descriptor *libusb_dev_get_config(libusb_dev *dev); -libusb_dev *libusb_dev_next(libusb_dev *dev); +int libusb_get_device_list(libusb_device ***list); +void libusb_free_device_list(libusb_device **list, int unref_devices); +struct libusb_dev_descriptor *libusb_device_get_descriptor(libusb_device *dev); +struct libusb_config_descriptor *libusb_device_get_config(libusb_device *dev); +libusb_device *libusb_device_ref(libusb_device *dev); +void libusb_device_unref(libusb_device *dev); -libusb_dev_handle *libusb_open(libusb_dev *dev); +libusb_dev_handle *libusb_open(libusb_device *dev); void libusb_close(libusb_dev_handle *devh); -struct libusb_dev *libusb_devh_get_dev(libusb_dev_handle *devh); +struct libusb_device *libusb_devh_get_device(libusb_dev_handle *devh); int libusb_claim_interface(libusb_dev_handle *dev, int iface); int libusb_release_interface(libusb_dev_handle *dev, int iface);
diff --git a/libusb/libusbi.h b/libusb/libusbi.h index b639ed6..c6b7f3e 100644 --- a/libusb/libusbi.h +++ b/libusb/libusbi.h
@@ -143,8 +143,8 @@ #define usbi_warn(fmt...) _usbi_log(LOG_LEVEL_WARNING, fmt) #define usbi_err(fmt...) _usbi_log(LOG_LEVEL_ERROR, fmt) -struct libusb_dev { - struct list_head list; +struct libusb_device { + int refcnt; char *nodepath; struct libusb_dev_descriptor desc; struct libusb_config_descriptor *config; @@ -152,7 +152,7 @@ struct libusb_dev_handle { struct list_head list; - struct libusb_dev *dev; + struct libusb_device *dev; int fd; };