qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Anthony Liguori
Subject: Re: [Qemu-devel] Re: [PATCH] Add USB sys file-system support (v6)
Date: Fri, 26 Sep 2008 09:27:52 -0500
User-agent: Thunderbird 2.0.0.16 (X11/20080723)

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.

It would be good to get this applied because it actually makes the usb
pass through usable on a significant number of hosts.

Honestly, I still haven't done a thorough review because as soon as I've seen the whitespace damage I've stopped looking. Early versions of the patch were impossible to review too because they reformatted huge chunks of code.

         ret = func(opaque, bus_num, addr, class_id, vendor_id,
-                   product_id, product_name, speed);
+                    product_id, product_name, speed);

This line not only shouldn't be here, but it's breaking the previously correct indentation.

+    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;

This is not how switch statements should be indented.

Regards,

Anthony Liguori

Thanks,
Jason.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]