Daniel Thompson has uploaded this change for review.

View Change

usbdev: Refactor device discovery code

Currently there is a lot of code shared between
usb_dev_get_by_vid_pid_serial() and usb_dev_get_by_vid_pid_number().
Fix this by pulling out the conditional filtering at the heart of each loop
and calling it via a function pointer.

I haven't got (two) dediprog programmers to test with but I have tested
both by...serial() and by...number() calls using a pair of Developerboxen
and a hacked driver.

Change-Id: I31ed572501e4314b9455e1b70a5e934ec96408b1
Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
M usbdev.c
1 file changed, 57 insertions(+), 58 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/27444/1
diff --git a/usbdev.c b/usbdev.c
index fb26cbb..3282b93 100644
--- a/usbdev.c
+++ b/usbdev.c
@@ -17,12 +17,15 @@

#include "platform.h"

+#include <stdbool.h>
#include <string.h>
#include <libusb.h>
#include "programmer.h"

-struct libusb_device_handle *usb_dev_get_by_vid_pid_serial(
- struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, const char *serialno)
+typedef bool (*filter_t)(struct libusb_device_descriptor *desc, struct libusb_device_handle *handle, void*ctx);
+
+static struct libusb_device_handle *get_by_vid_pid_filter(struct libusb_context *usb_ctx,
+ uint16_t vid, uint16_t pid, filter_t filter_fn, void *filter_ctx)
{
struct libusb_device **list;
ssize_t count = libusb_get_device_list(usb_ctx, &list);
@@ -50,28 +53,22 @@
desc.idVendor, desc.idProduct,
libusb_get_bus_number(dev), libusb_get_device_address(dev));

+ /* allow filters to trigger before the device is opened */
+ if (filter_fn(&desc, NULL, filter_ctx))
+ continue;
+
res = libusb_open(dev, &handle);
if (res != 0) {
- msg_perr("Opening the USB device failed (%s)!\n", libusb_error_name(res));
- continue;
+ msg_perr("Opening the USB device at address %d-%d failed (%s)!\n",
+ libusb_get_bus_number(dev), libusb_get_device_address(dev),
+ libusb_error_name(res));
+ break;
}

- if (serialno) {
- unsigned char myserial[64];
- res = libusb_get_string_descriptor_ascii(handle, desc.iSerialNumber, myserial,
- sizeof(myserial));
- if (res < 0) {
- msg_perr("Reading the USB serialno failed (%s)!\n", libusb_error_name(res));
- libusb_close(handle);
- continue;
- }
- msg_pdbg("Serial number is %s\n", myserial);
-
- /* Reject any serial number that does not commence with serialno */
- if (0 != strncmp(serialno, (char *)myserial, strlen(serialno))) {
- libusb_close(handle);
- continue;
- }
+ /* filter can also trigger after a device is opened */
+ if (filter_fn(&desc, handle, filter_ctx)) {
+ libusb_close(handle);
+ continue;
}

libusb_free_device_list(list, 1);
@@ -80,6 +77,45 @@

libusb_free_device_list(list, 1);
return NULL;
+
+}
+
+static bool filter_by_serial(struct libusb_device_descriptor *desc, struct libusb_device_handle *handle,
+ void *serialno)
+{
+ /* never filter if device is not yet open or when user did not provide a serial number */
+ if (!handle || !serialno)
+ return false;
+
+ unsigned char myserial[64];
+ int res = libusb_get_string_descriptor_ascii(handle, desc->iSerialNumber, myserial, sizeof(myserial));
+ if (res < 0) {
+ msg_perr("Reading the USB serialno failed (%s)!\n", libusb_error_name(res));
+ return true;
+ }
+ msg_pdbg("Serial number is %s\n", myserial);
+
+ /* Filter out any serial number that does not commence with serialno */
+ return 0 != strncmp(serialno, (char *)myserial, strlen(serialno));
+}
+
+static bool filter_by_number(struct libusb_device_descriptor *desc, struct libusb_device_handle *handle,
+ void *ctx)
+{
+ /* no need to NULL check handle; the conditional decrement makes it unnecessary */
+ unsigned int *nump = ctx;
+ if (*nump) {
+ (*nump)--;
+ return true;
+ }
+
+ return false;
+}
+
+struct libusb_device_handle *usb_dev_get_by_vid_pid_serial(
+ struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, const char *serialno)
+{
+ return get_by_vid_pid_filter(usb_ctx, vid, pid, filter_by_serial, (void *) serialno);
}

/*
@@ -91,42 +127,5 @@
struct libusb_device_handle *usb_dev_get_by_vid_pid_number(
struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num)
{
- struct libusb_device **list;
- ssize_t count = libusb_get_device_list(usb_ctx, &list);
- if (count < 0) {
- msg_perr("Getting the USB device list failed (%s)!\n", libusb_error_name(count));
- return NULL;
- }
-
- struct libusb_device_handle *handle = NULL;
- ssize_t i = 0;
- for (i = 0; i < count; i++) {
- struct libusb_device *dev = list[i];
- struct libusb_device_descriptor desc;
- int err = libusb_get_device_descriptor(dev, &desc);
- if (err != 0) {
- msg_perr("Reading the USB device descriptor failed (%s)!\n", libusb_error_name(err));
- libusb_free_device_list(list, 1);
- return NULL;
- }
- if ((desc.idVendor == vid) && (desc.idProduct == pid)) {
- msg_pdbg("Found USB device %04"PRIx16":%04"PRIx16" at address %d-%d.\n",
- desc.idVendor, desc.idProduct,
- libusb_get_bus_number(dev), libusb_get_device_address(dev));
- if (num == 0) {
- err = libusb_open(dev, &handle);
- if (err != 0) {
- msg_perr("Opening the USB device failed (%s)!\n",
- libusb_error_name(err));
- libusb_free_device_list(list, 1);
- return NULL;
- }
- break;
- }
- num--;
- }
- }
- libusb_free_device_list(list, 1);
-
- return handle;
+ return get_by_vid_pid_filter(usb_ctx, vid, pid, filter_by_number, &num);
}

To view, visit change 27444. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I31ed572501e4314b9455e1b70a5e934ec96408b1
Gerrit-Change-Number: 27444
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Thompson <daniel.thompson@linaro.org>