From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KjEJb-00070S-Ds for qemu-devel@nongnu.org; Fri, 26 Sep 2008 10:29:03 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KjEJW-0006uo-Ij for qemu-devel@nongnu.org; Fri, 26 Sep 2008 10:29:02 -0400 Received: from [199.232.76.173] (port=39103 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KjEJW-0006uO-5R for qemu-devel@nongnu.org; Fri, 26 Sep 2008 10:28:58 -0400 Received: from yx-out-1718.google.com ([74.125.44.153]:21656) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KjEJV-0008Dq-73 for qemu-devel@nongnu.org; Fri, 26 Sep 2008 10:28:57 -0400 Received: by yx-out-1718.google.com with SMTP id 3so159105yxi.82 for ; Fri, 26 Sep 2008 07:28:54 -0700 (PDT) Message-ID: <48DCF168.5080502@codemonkey.ws> Date: Fri, 26 Sep 2008 09:27:52 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH] Add USB sys file-system support (v6) References: <1220571341.2638.6.camel@hephaestion> <1220580385.2638.15.camel@hephaestion> <48C1346F.3000405@windriver.com> <1220640699.5470.15.camel@hephaestion> <48C1862C.3050307@windriver.com> <1220649226.9611.13.camel@hephaestion> <48C53B04.9030006@windriver.com> <1221679892.17792.6.camel@hephaestion> <48D16904.30104@codemonkey.ws> <1221691647.17792.55.camel@hephaestion> <48D81E26.9080802@codemonkey.ws> <1222133639.18297.13.camel@hephaestion> <48DBCE35.4030607@codemonkey.ws> <48DCEA3E.7010006@windriver.com> In-Reply-To: <48DCEA3E.7010006@windriver.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wessel Cc: TJ , qemu-devel@nongnu.org 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. >