From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1M7TP8-0007Wy-Ao for qemu-devel@nongnu.org; Fri, 22 May 2009 07:59:14 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1M7TP3-0007Wb-LF for qemu-devel@nongnu.org; Fri, 22 May 2009 07:59:13 -0400 Received: from [199.232.76.173] (port=43006 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1M7TP3-0007WU-Ha for qemu-devel@nongnu.org; Fri, 22 May 2009 07:59:09 -0400 Received: from mx20.gnu.org ([199.232.41.8]:8601) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1M7TP3-0001kf-6k for qemu-devel@nongnu.org; Fri, 22 May 2009 07:59:09 -0400 Received: from mail.codesourcery.com ([65.74.133.4]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1M7TP2-0005G3-2c for qemu-devel@nongnu.org; Fri, 22 May 2009 07:59:08 -0400 From: Paul Brook Subject: Re: [Qemu-devel] [PATCH] Add new function qemu_register_machines Date: Fri, 22 May 2009 12:59:04 +0100 References: <4A159CBE.9020606@mail.berlios.de> <200905212333.21133.paul@codesourcery.com> <4A167996.6030200@mail.berlios.de> In-Reply-To: <4A167996.6030200@mail.berlios.de> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200905221259.05433.paul@codesourcery.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Anthony Liguori > well, it's a matter of personal taste whether you prefer > to have one or two interfaces for machine registration. True. This is not a performance critical bit of code and has a well bounded number of occurrences so I'm inclined to go for simplicity/clarity over efficiency. If we use arrays I recommend doing so consistently, even in the single machine case. Given your goal is to avoid repeated code I'd also use something along the lines of: #define qemu_register_machines(x) qemu_do_register_machines(x, ARRAY_SIZE(x)) > The new function is an improvement if you want to reduce > lines of code and binary code size. > > Removing unused entries like register_machines from > header files is also an improvement. This part of > my patch is fixed now in QEMU HEAD, thanks Anthony! > > And adding "static" to a machine declaration which is > only used locally is also an improvement (hw/spitz.c). > > I assume that you agree that those last two points are > an improvement. Yes. > So the patch improves two details and adds something > where people disagree about its usefulness. > > What now? Will the patch be rejected because you > don't see an improvement? Will you fix the missing > "static" yourself? Do you want a new patch without > qemu_register_machines? Or a new patch which uses > qemu_register_machines for all files with more than > one machine? I'm never going to approve a patch until I'm happy with all of it. In general this is why it's a bad idea to mix cleanups and new features. In this case your new feature obsoletes some of your cleanups, so the case for separate patches is less clear. However unless I'm feeling particularly bored or enthusiastic I'm unlikely to split up or rework the patch for you. Paul