Patchwork Fix recently introduced bugs in -usbdevice host

login
register
mail settings
Submitter Markus Armbruster
Date Nov. 27, 2009, 12:05 p.m.
Message ID <m33a40f8g7.fsf@crossbow.pond.sub.org>
Download mbox | patch
Permalink /patch/39628/
State New
Headers show

Comments

Markus Armbruster - Nov. 27, 2009, 12:05 p.m.
Commit 26a9e82a has the following flaws:

* It enabled DEBUG.

* It referenced two properties by the wrong name in
  usb_host_device_open(), which crashes with "qdev_prop_set: property
  "USB Host Device.bus" not found".

* It broke "-usbdevice host:auto:..." by calling parse_filter()
  incorrectly.

* It broke parsing of "-usbdevice host:BUS.ADDR" and "-usbdevice
  host:VID:PRID" with a trivial pasto.

* It broke wildcards in "-usbdevice host:auto:...".  Before, the four
  filter components were stored as int, and the wildcard was encoded
  as -1.  The faulty commit changed storage to uint32_t, and the
  wildcard encoding to 0.  But it failed to update parse_filter()
  accordingly.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
If you want me to split this up into one patch per bug, I can do that.

 usb-linux.c |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)
Gerd Hoffmann - Nov. 27, 2009, 1:40 p.m.
On 11/27/09 13:05, Markus Armbruster wrote:
> Commit 26a9e82a has the following flaws:
>

ACK, the fixes are correct.

/me used to test with -device usb-host,<properties> mostly and obviously 
didn't pay enougth attention to the -usbdevice backward compatibility 
code paths.

thanks for fixing it up,

   Gerd

Patch

diff --git a/usb-linux.c b/usb-linux.c
index 96f9a27..285ac22 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -65,7 +65,7 @@  typedef int USBScanFunc(void *opaque, int bus_num, int addr, int class_id,
                         int vendor_id, int product_id,
                         const char *product_name, int speed);
 
-#define DEBUG
+//#define DEBUG
 
 #ifdef DEBUG
 #define dprintf printf
@@ -1005,7 +1005,7 @@  device_init(usb_host_register_devices)
 
 USBDevice *usb_host_device_open(const char *devname)
 {
-    struct USBAutoFilter filter = { 0, 0, 0, 0 };
+    struct USBAutoFilter filter;
     USBDevice *dev;
     USBHostDevice *s;
     char *p;
@@ -1014,22 +1014,26 @@  USBDevice *usb_host_device_open(const char *devname)
     s = DO_UPCAST(USBHostDevice, dev, dev);
 
     if (strstr(devname, "auto:")) {
-        if (parse_filter(devname+5, &filter) < 0)
+        if (parse_filter(devname, &filter) < 0)
             goto fail;
     } else {
         if ((p = strchr(devname, '.'))) {
-            filter.bus_num = strtoul(devname, NULL, 0);
-            filter.addr    = strtoul(devname, NULL, 0);
+            filter.bus_num    = strtoul(devname, NULL, 0);
+            filter.addr       = strtoul(p + 1, NULL, 0);
+            filter.vendor_id  = 0;
+            filter.product_id = 0;
         } else if ((p = strchr(devname, ':'))) {
+            filter.bus_num    = 0;
+            filter.addr       = 0;
             filter.vendor_id  = strtoul(devname, NULL, 16);
-            filter.product_id = strtoul(devname, NULL, 16);
+            filter.product_id = strtoul(p + 1, NULL, 16);
         } else {
             goto fail;
         }
     }
 
-    qdev_prop_set_uint32(&dev->qdev, "bus",       filter.bus_num);
-    qdev_prop_set_uint32(&dev->qdev, "addr",      filter.addr);
+    qdev_prop_set_uint32(&dev->qdev, "hostbus",   filter.bus_num);
+    qdev_prop_set_uint32(&dev->qdev, "hostaddr",  filter.addr);
     qdev_prop_set_uint32(&dev->qdev, "vendorid",  filter.vendor_id);
     qdev_prop_set_uint32(&dev->qdev, "productid", filter.product_id);
     qdev_init(&dev->qdev);
@@ -1449,10 +1453,10 @@  static int parse_filter(const char *spec, struct USBAutoFilter *f)
     const char *p = spec;
     int i;
 
-    f->bus_num    = -1;
-    f->addr       = -1;
-    f->vendor_id  = -1;
-    f->product_id = -1;
+    f->bus_num    = 0;
+    f->addr       = 0;
+    f->vendor_id  = 0;
+    f->product_id = 0;
 
     for (i = BUS; i < DONE; i++) {
     	p = strpbrk(p, ":.");