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

[Anthony Liguori]

TJ wrote:
> Demoted to dprintf().
>
> --------------------------------------
> 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>
>   

This patch is still white spaced damaged.

> ---
> --- a/usb-linux.c   2008-09-17 22:39:38.000000000 +0100
> +++ b/usb-linux.c   2008-09-17 23:42:32.000000000 +0100
>   

This chunk:

> -        if (fgets(line, sizeof(line), f) == NULL)
> -            break;
> -        if (strlen(line) > 0)
> -            line[strlen(line) - 1] = '\0';
> -        if (line[0] == 'T' && line[1] == ':') {
> -            if (device_count && (vendor_id || product_id)) {
> -                /* New device.  Add the previously discovered device.  */
> -                ret = func(opaque, bus_num, addr, class_id, vendor_id,
> -                           product_id, product_name, speed);
> -                if (ret)
> -                    goto the_end;
> -            }
> -            if (get_tag_value(buf, sizeof(buf), line, "Bus=", " ") < 0)
> -                goto fail;
> -            bus_num = atoi(buf);
> -            if (get_tag_value(buf, sizeof(buf), line, "Dev#=", " ") < 0)
> -                goto fail;
> -            addr = atoi(buf);
> -            if (get_tag_value(buf, sizeof(buf), line, "Spd=", " ") < 0)
> -                goto fail;
> -            if (!strcmp(buf, "480"))
> -                speed = USB_SPEED_HIGH;
> -            else if (!strcmp(buf, "1.5"))
> -                speed = USB_SPEED_LOW;
> -            else
> -                speed = USB_SPEED_FULL;
> -            product_name[0] = '\0';
> -            class_id = 0xff;
> -            device_count++;
> -            product_id = 0;
> -            vendor_id = 0;
> -        } else if (line[0] == 'P' && line[1] == ':') {
> -            if (get_tag_value(buf, sizeof(buf), line, "Vendor=", " ") < 0)
> -                goto fail;
> -            vendor_id = strtoul(buf, NULL, 16);
> -            if (get_tag_value(buf, sizeof(buf), line, "ProdID=", " ") < 0)
> -                goto fail;
> -            product_id = strtoul(buf, NULL, 16);
> -        } else if (line[0] == 'S' && line[1] == ':') {
> -            if (get_tag_value(buf, sizeof(buf), line, "Product=", "") < 0)
> -                goto fail;
> -            pstrcpy(product_name, sizeof(product_name), buf);
> -        } else if (line[0] == 'D' && line[1] == ':') {
> -            if (get_tag_value(buf, sizeof(buf), line, "Cls=", " (") < 0)
> -                goto fail;
> -            class_id = strtoul(buf, NULL, 16);
> -        }
> -    fail: ;
>   

Is identical to this chunk with the exception of whitespace.  But 
curiously, not enough white space to warrant such a large diff.  I broke 
out ediff-buffers to verify this.  Did you reformat this whole region 
and then just query-replace the tabs with spaces?  I can't see how diff 
would generate this chunk otherwise.

There should be no changes here.  I don't mean to be pedantic but it's 
extremely difficult to review a patch like this.

> +        if (fgets(line, sizeof(line), f) == NULL)
> +            break;
> +        if (strlen(line) > 0)
> +            line[strlen(line) - 1] = '\0';
> +        if (line[0] == 'T' && line[1] == ':') {
> +            if (device_count && (vendor_id || product_id)) {
> +                /* New device.  Add the previously discovered device.  */
> +                ret = func(opaque, bus_num, addr, class_id, vendor_id,
> +                            product_id, product_name, speed);
> +                if (ret)
> +                    goto the_end;
> +            }
> +            if (get_tag_value(buf, sizeof(buf), line, "Bus=", " ") < 0)
> +                goto fail;
> +
> +            bus_num = atoi(buf);
> +            if (get_tag_value(buf, sizeof(buf), line, "Dev#=", " ") < 0)
> +                goto fail;
> +
> +            addr = atoi(buf);
> +            if (get_tag_value(buf, sizeof(buf), line, "Spd=", " ") < 0)
> +                goto fail;
> +
> +            if (!strcmp(buf, "480"))
> +                speed = USB_SPEED_HIGH;
> +            else if (!strcmp(buf, "1.5"))
> +                speed = USB_SPEED_LOW;
> +            else
> +                speed = USB_SPEED_FULL;
> +            product_name[0] = '\0';
> +            class_id = 0xff;
> +            device_count++;
> +            product_id = 0;
> +            vendor_id = 0;
> +        }
> +        else if (line[0] == 'P' && line[1] == ':') {
> +            if (get_tag_value(buf, sizeof(buf), line, "Vendor=", " ") < 0)
> +                goto fail;
> +
> +            vendor_id = strtoul(buf, NULL, 16);
> +            if (get_tag_value(buf, sizeof(buf), line, "ProdID=", " ") < 0)
> +                goto fail;
> +
> +            product_id = strtoul(buf, NULL, 16);
> +        }
> +        else if (line[0] == 'S' && line[1] == ':') {
> +            if (get_tag_value(buf, sizeof(buf), line, "Product=", "") < 0)
> +                goto fail;
> +
> +            pstrcpy(product_name, sizeof(product_name), buf);
> +        }
> +        else if (line[0] == 'D' && line[1] == ':') {
> +            if (get_tag_value(buf, sizeof(buf), line, "Cls=", " (") < 0)
> +                goto fail;
> +
> +            class_id = strtoul(buf, NULL, 16);
> +        }
> + fail: ;
>      }
>      if (device_count && (vendor_id || product_id)) {
> -        /* Add the last device.  */
> -        ret = func(opaque, bus_num, addr, class_id, vendor_id,
> -                   product_id, product_name, speed);
>   

Regards,

Anthony Liguori