Re: [Qemu-devel] Re: [PATCH] Add USB sys file-system support (v6)

[Jason Wessel]

Anthony Liguori wrote:
> Jason Wessel wrote:
>> Anthony if that is all that you consider to be blocking a commit,
>> attached is a fixed version with white space and comments fixups.
>>  
>
> Hi Jason,
>
> Thanks for trying to help, but it's *still* whitespace damaged.  This
> may seem like a minor nit but not taking the time to try and make code
> match up with the rest of the source tree doesn't suggest that much time
> has been taking to make the code fit in architecturally or to think
> about any sort of wider interactions.  I also like to give an author the
> chance to fix something because it helps encourage people to do the
> right thing the first time around.
>

While I agree in part, it is also frustrating when you want to put
changes on top someone else's work that is changing.  It is in the
spirit of trying to stabilize this patch that I offered a bit of
assistance.

The rest of your concerns to the white space fixes are addressed in
the next version here.

I also fixed the instances of:
    if (f) ...
to be:
    if (f)
        ...

As well a closing else {} problem:
    if () {
        ...
    } else
        ...
to be:
    if () {
        ...
    } else {
        ...
    }

Perhaps further technical review can occur with no more white space issues?

My comment on technical content would be that the several places that
atoi() and strtoul() the values are not checked for any kind of
correctness and subsequently pass on.  Perhaps no validity check is
needed because it is dealt with in the later call to the passed in
function...  It just was not immediately obvious.

Jason.

From: TJ <address@hidden>
Subject: [PATCH] Add USB sys file-system support

This patch adds support for host USB devices discovered via:

/sys/bus/usb/devices/* and opened from /dev/bus/usb/*/*
/dev/bus/usb/devices and opened from /dev/bus/usb/*/*

in addition to the existing discovery via:

/proc/bus/usb/devices and opened from /proc/bus/usb/*/*

Signed-off-by: TJ <address@hidden>

---
 usb-linux.c |  229 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 219 insertions(+), 10 deletions(-)

--- a/usb-linux.c
+++ b/usb-linux.c
@@ -7,6 +7,10 @@
  *      Support for host device auto connect & disconnect
  *      Major rewrite to support fully async operation
  *
+ * Copyright 2008 TJ <address@hidden>
+ *      Added flexible support for /dev/bus/usb /sys/bus/usb/devices in 
addition
+ *      to the legacy /proc/bus/usb USB device discovery and handling
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to 
deal
  * in the Software without restriction, including without limitation the rights
@@ -72,9 +76,20 @@ static int usb_host_find_device(int *pbu
 #define dprintf(...)
 #endif
 
-#define USBDEVFS_PATH "/proc/bus/usb"
+#define USBPROCBUS_PATH "/proc/bus/usb"
 #define PRODUCT_NAME_SZ 32
 #define MAX_ENDPOINTS 16
+#define USBDEVBUS_PATH "/dev/bus/usb"
+#define USBSYSBUS_PATH "/sys/bus/usb"
+
+static char *usb_host_device_path;
+
+#define USB_FS_NONE 0
+#define USB_FS_PROC 1
+#define USB_FS_DEV 2
+#define USB_FS_SYS 3
+
+static int usb_fs_type = 0;
 
 /* endpoint association data */
 struct endp_data {
@@ -890,13 +905,18 @@ static USBDevice *usb_host_device_open_a
 
     printf("husb: open device %d.%d\n", bus_num, addr);
 
-    snprintf(buf, sizeof(buf), USBDEVFS_PATH "/%03d/%03d",
+    if (!usb_host_device_path) {
+        perror("husb: USB Host Device Path not set");
+        goto fail;
+    }
+    snprintf(buf, sizeof(buf), "%s/%03d/%03d", usb_host_device_path,
              bus_num, addr);
     fd = open(buf, O_RDWR | O_NONBLOCK);
     if (fd < 0) {
         perror(buf);
         goto fail;
     }
+    dprintf("husb: opened %s\n", buf);
 
     /* read the device description */
     dev->descr_len = read(fd, dev->descr, sizeof(dev->descr));
@@ -1038,23 +1058,29 @@ static int get_tag_value(char *buf, int 
     return q - buf;
 }
 
-static int usb_host_scan(void *opaque, USBScanFunc *func)
+/*
+ * Use /proc/bus/usb/devices or /dev/bus/usb/devices file to determine
+ * host's USB devices. This is legacy support since many distributions
+ * are moving to /sys/bus/usb
+ */
+static int usb_host_scan_dev(void *opaque, USBScanFunc *func)
 {
-    FILE *f;
+    FILE *f = 0;
     char line[1024];
     char buf[1024];
     int bus_num, addr, speed, device_count, class_id, product_id, vendor_id;
-    int ret;
     char product_name[512];
+    int ret = 0;
 
-    f = fopen(USBDEVFS_PATH "/devices", "r");
+    snprintf(line, sizeof(line), "%s/devices", usb_host_device_path);
+    f = fopen(line, "r");
     if (!f) {
-        term_printf("husb: could not open %s\n", USBDEVFS_PATH "/devices");
-        return 0;
+        perror("husb: cannot open devices file");
+        goto the_end;
     }
+
     device_count = 0;
     bus_num = addr = speed = class_id = product_id = vendor_id = 0;
-    ret = 0;
     for(;;) {
         if (fgets(line, sizeof(line), f) == NULL)
             break;
@@ -1111,7 +1137,190 @@ static int usb_host_scan(void *opaque, U
                    product_id, product_name, speed);
     }
  the_end:
-    fclose(f);
+    if (f)
+        fclose(f);
+    return ret;
+}
+
+/*
+ * Use /sys/bus/usb/devices/ directory to determine host's USB
+ * devices.
+ *
+ * This code is taken from Robert Schiele's original patches posted to
+ * the Novell bug-tracker https://bugzilla.novell.com/show_bug.cgi?id=241950
+ */
+static int usb_host_scan_sys(void *opaque, USBScanFunc *func)
+{
+    FILE *f;
+    DIR *dir = 0;
+    char line[1024];
+    int bus_num, addr, speed, class_id, product_id, vendor_id;
+    int ret = 0;
+    char product_name[512];
+    struct dirent* de;
+
+    dir = opendir(USBSYSBUS_PATH "/devices");
+    if (!dir) {
+        perror("husb: cannot open devices directory");
+        goto the_end;
+    }
+
+    while ((de = readdir(dir))) {
+        if (de->d_name[0] != '.' && ! strchr(de->d_name, ':')) {
+            char filename[PATH_MAX];
+            char* tmpstr = de->d_name;
+            if (!strncmp(de->d_name, "usb", 3))
+                tmpstr += 3;
+
+            bus_num = atoi(tmpstr);
+            snprintf(filename, PATH_MAX, USBSYSBUS_PATH "/devices/%s/devnum", 
de->d_name);
+            f = fopen(filename, "r");
+            if (!f) {
+                term_printf("Could not open %s\n", filename);
+                goto the_end;
+            }
+            fgets(line, sizeof(line), f);
+            fclose(f);
+            addr = atoi(line);
+            snprintf(filename, PATH_MAX, USBSYSBUS_PATH 
"/devices/%s/bDeviceClass", de->d_name);
+            f = fopen(filename, "r");
+            if (!f) {
+                term_printf("Could not open %s\n", filename);
+                goto the_end;
+            }
+            fgets(line, sizeof(line), f);
+            fclose(f);
+            class_id = strtoul(line, NULL, 16);
+            snprintf(filename, PATH_MAX, USBSYSBUS_PATH 
"/devices/%s/idVendor", de->d_name);
+            f = fopen(filename, "r");
+            if (!f) {
+                term_printf("Could not open %s\n", filename);
+                goto the_end;
+            }
+            fgets(line, sizeof(line), f);
+            fclose(f);
+            vendor_id = strtoul(line, NULL, 16);
+            snprintf(filename, PATH_MAX, USBSYSBUS_PATH 
"/devices/%s/idProduct", de->d_name);
+            f = fopen(filename, "r");
+            if (!f) {
+                term_printf("Could not open %s\n", filename);
+                goto the_end;
+            }
+            fgets(line, sizeof(line), f);
+            fclose(f);
+            product_id = strtoul(line, NULL, 16);
+            snprintf(filename, PATH_MAX, USBSYSBUS_PATH "/devices/%s/product", 
de->d_name);
+            f = fopen(filename, "r");
+            if (f) {
+                fgets(line, sizeof(line), f);
+                fclose(f);
+                if (strlen(line) > 0)
+                    line[strlen(line) - 1] = '\0';
+
+                pstrcpy(product_name, sizeof(product_name), line);
+            } else {
+                *product_name = 0;
+            }
+
+            snprintf(filename, PATH_MAX, USBSYSBUS_PATH "/devices/%s/speed", 
de->d_name);
+            f = fopen(filename, "r");
+            if (!f) {
+                term_printf("Could not open %s\n", filename);
+                goto the_end;
+            }
+            fgets(line, sizeof(line), f);
+            fclose(f);
+            if (!strcmp(line, "480\n"))
+                speed = USB_SPEED_HIGH;
+            else if (!strcmp(line, "1.5\n"))
+                speed = USB_SPEED_LOW;
+            else
+                speed = USB_SPEED_FULL;
+
+            ret = func(opaque, bus_num, addr, class_id, vendor_id,
+                       product_id, product_name, speed);
+            if (ret)
+                goto the_end;
+        }
+    }
+ the_end:
+    if (dir) closedir(dir);
+    return ret;
+}
+
+/*
+ * Determine how to access the host's USB devices and call the
+ * specific support function.
+ */
+static int usb_host_scan(void *opaque, USBScanFunc *func)
+{
+    FILE *f = 0;
+    DIR *dir = 0;
+    int ret = 0;
+    const char *devices = "/devices";
+    const char *trying = "husb: trying to open %s%s\n";
+    const char *failed = "husb: could not open %s%s\n";
+    char devpath[PATH_MAX];
+
+    /* only check the host once */
+    if (!usb_fs_type) {
+        /* test for dev file-system access in /proc/ */
+        dprintf(trying, USBPROCBUS_PATH, devices);
+        f = fopen(USBPROCBUS_PATH "/devices", "r");
+        if (!f) {
+            dprintf(failed, USBPROCBUS_PATH, devices);
+            /* maybe it has been moved to the /dev/ base */
+            dprintf(trying, USBDEVBUS_PATH, devices);
+            f = fopen(USBDEVBUS_PATH "/devices", "r");
+            if (!f) {
+                dprintf(failed, USBDEVBUS_PATH, devices);
+                /* test for newer sys file-system access */
+                dprintf(trying, USBSYSBUS_PATH, devices);
+                dir = opendir(USBSYSBUS_PATH "/devices");
+                if (!dir) {
+                    dprintf(failed, USBSYSBUS_PATH, devices);
+                    goto the_end;
+                } else {
+                    /* devices found in /dev/bus/usb/ (yes - not a mistake!) */
+                    strcpy(devpath, USBDEVBUS_PATH);
+                    usb_fs_type = USB_FS_SYS;
+                }
+                if (dir) closedir(dir);
+            } else {
+                /* devices found in /dev/bus/usb/ */
+                strcpy(devpath, USBDEVBUS_PATH);
+                usb_fs_type = USB_FS_DEV;
+            }
+        } else {
+            /* devices found in /proc/bus/usb/ */
+            strcpy(devpath, USBPROCBUS_PATH);
+            usb_fs_type = USB_FS_PROC;
+        }
+        if (f)
+            fclose(f);
+
+        /* the module setting (used later for opening devices) */
+        usb_host_device_path = qemu_mallocz(strlen(devpath)+1);
+        if (usb_host_device_path) {
+            strcpy(usb_host_device_path, devpath);
+            term_printf("husb: using %s\n", usb_host_device_path);
+        } else {
+            /* out of memory? */
+            perror("husb: unable to allocate memory for device path");
+            goto the_end;
+        }
+    }
+
+    switch (usb_fs_type) {
+    case USB_FS_PROC:
+    case USB_FS_DEV:
+        ret = usb_host_scan_dev(opaque, func);
+        break;
+    case USB_FS_SYS:
+        ret = usb_host_scan_sys(opaque, func);
+        break;
+    }
+ the_end:
     return ret;
 }