From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=58014 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OfxFn-0002YR-Hp for qemu-devel@nongnu.org; Mon, 02 Aug 2010 11:48:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OfxFi-0004iK-9L for qemu-devel@nongnu.org; Mon, 02 Aug 2010 11:48:39 -0400 Received: from mail-qy0-f180.google.com ([209.85.216.180]:37779) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OfxFi-0004i3-2J for qemu-devel@nongnu.org; Mon, 02 Aug 2010 11:48:34 -0400 Received: by qyk31 with SMTP id 31so77122qyk.4 for ; Mon, 02 Aug 2010 08:48:33 -0700 (PDT) Message-ID: <4C56E8CB.1090104@codemonkey.ws> Date: Mon, 02 Aug 2010 10:48:27 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] Drop braces around single statement rule References: <1280593414-2232-1-git-send-email-av1474@comtv.ru> <20100731234956.GJ31640@hall.aurel32.net> <4C56E253.7010403@codemonkey.ws> <4C56E732.7060102@redhat.com> In-Reply-To: <4C56E732.7060102@redhat.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Blue Swirl , qemu-devel@nongnu.org, Aurelien Jarno On 08/02/2010 10:41 AM, Kevin Wolf wrote: > >> But something like braces around an if doesn't seem like it creates a >> big problem. Most C programmers are used to seeing braces in some >> statements and not other. Therefore, it's hard to argue that the code >> gets really unreadable if this isn't strictly enforced. >> > I won't argue that missing braces impact readability of the code, they > probably don't. However, as was pointed out in earlier discussion there > still remain two important points: > > 1. While it doesn't make a difference for the code itself, readability > of patches suffers when braces have to be added/removed when a second > line is inserted or disappears. > I understand the argument but it's not something that I strongly agree with. > 2. I've messed up more than once with adding some debug code (even worse > when it happens with real code): > > if (foo) > fprintf(stderr, "debug msg\n"); > bar(); /* oops, not conditional any more */ > This is hard to do with editors that auto indent unless you're copying and pasting debugging. And yeah, I've made that mistake too doing the later :-) > This is why I tend to disagree with removing the rule, and suggest to > rather implement some automatic checks like Aurelien suggested (if we > need to change anything at all). I usually don't ask for a respin just > for braces if the patch is good otherwise, but if you think we should > just reject such patches without exceptions, I can change that. > Yeah, I try to remember to enforce it but often forget or just don't notice. My eyes don't tend to catch missing braces like they would catch bad type naming because the former is really not uncommon. I'm happy with the status quo but I won't object to a git commit hook that enforces style. Regards, Anthony Liguori