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

[Anthony Liguori <anthony@codemonkey.ws>]

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