qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH] Drop braces around single statement rule


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH] Drop braces around single statement rule
Date: Mon, 02 Aug 2010 11:18:26 -0500
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.11) Gecko/20100713 Lightning/1.0b1 Thunderbird/3.0.6

On 08/02/2010 11:06 AM, malc wrote:
On Mon, 2 Aug 2010, Anthony Liguori wrote:

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.
Seriously? You are happy with the situation where some people get their
patches rejected because they disagree/forogot/don't_care about single
statement braces while the patches of others make it through?

Yeah, I'm neglecting the fact that we're not consistent as maintainers and I'm all for dropping it from CODING_STYLE.

Regards,

Anthony Liguori



reply via email to

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