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

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

[malc]

History has shown that this particular rule is unenforcable.

Signed-off-by: malc <av1474@comtv.ru>
---
 CODING_STYLE |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/CODING_STYLE b/CODING_STYLE
index 92036f3..e0b5376 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -54,16 +54,17 @@ readers that they are seeing a wrapped version; otherwise avoid this prefix.
 
 4. Block structure
 
-Every indented statement is braced; even if the block contains just one
-statement.  The opening brace is on the line that contains the control
-flow statement that introduces the new block; the closing brace is on the
-same line as the else keyword, or on a line by itself if there is no else
-keyword.  Example:
+The opening brace is on the line that contains the control flow
+statement that introduces the new block; the closing brace is on the
+same line as the else keyword, or on a line by itself if there is no
+else keyword. Example:
 
     if (a == 5) {
         printf("a was 5.\n");
+        do5stuff();
     } else if (a == 6) {
         printf("a was 6.\n");
+        do6stuff();
     } else {
         printf("a was something else entirely.\n");
     }
-- 
1.6.6.1

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

[Aurelien Jarno]

On Sat, Jul 31, 2010 at 08:23:34PM +0400, malc wrote:
> History has shown that this particular rule is unenforcable.
> 
> Signed-off-by: malc <av1474@comtv.ru>
> ---
>  CODING_STYLE |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/CODING_STYLE b/CODING_STYLE
> index 92036f3..e0b5376 100644
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -54,16 +54,17 @@ readers that they are seeing a wrapped version; otherwise avoid this prefix.
>  
>  4. Block structure
>  
> -Every indented statement is braced; even if the block contains just one
> -statement.  The opening brace is on the line that contains the control
> -flow statement that introduces the new block; the closing brace is on the
> -same line as the else keyword, or on a line by itself if there is no else
> -keyword.  Example:
> +The opening brace is on the line that contains the control flow
> +statement that introduces the new block; the closing brace is on the
> +same line as the else keyword, or on a line by itself if there is no
> +else keyword. Example:
>  
>      if (a == 5) {
>          printf("a was 5.\n");
> +        do5stuff();
>      } else if (a == 6) {
>          printf("a was 6.\n");
> +        do6stuff();
>      } else {
>          printf("a was something else entirely.\n");
>      }

I am "neutral" on this particular rule.

OTOH, my opinion is that we really should try to enforce the rules. The
fact that we are humans and make mistakes (some patches not enforcing
all the rules are committed), is not enough to just ignore the rules. If
a rule is not enforceable, it should be removed from CODING_STYLE, not
removed.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

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

[Aurelien Jarno]

On Sat, Jul 31, 2010 at 06:47:29PM +0200, Aurelien Jarno wrote:
> On Sat, Jul 31, 2010 at 08:23:34PM +0400, malc wrote:
> > History has shown that this particular rule is unenforcable.
> > 
> > Signed-off-by: malc <av1474@comtv.ru>
> > ---
> >  CODING_STYLE |   11 ++++++-----
> >  1 files changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/CODING_STYLE b/CODING_STYLE
> > index 92036f3..e0b5376 100644
> > --- a/CODING_STYLE
> > +++ b/CODING_STYLE
> > @@ -54,16 +54,17 @@ readers that they are seeing a wrapped version; otherwise avoid this prefix.
> >  
> >  4. Block structure
> >  
> > -Every indented statement is braced; even if the block contains just one
> > -statement.  The opening brace is on the line that contains the control
> > -flow statement that introduces the new block; the closing brace is on the
> > -same line as the else keyword, or on a line by itself if there is no else
> > -keyword.  Example:
> > +The opening brace is on the line that contains the control flow
> > +statement that introduces the new block; the closing brace is on the
> > +same line as the else keyword, or on a line by itself if there is no
> > +else keyword. Example:
> >  
> >      if (a == 5) {
> >          printf("a was 5.\n");
> > +        do5stuff();
> >      } else if (a == 6) {
> >          printf("a was 6.\n");
> > +        do6stuff();
> >      } else {
> >          printf("a was something else entirely.\n");
> >      }
> 
> I am "neutral" on this particular rule.
> 
> OTOH, my opinion is that we really should try to enforce the rules. The
> fact that we are humans and make mistakes (some patches not enforcing
> all the rules are committed), is not enough to just ignore the rules. If
> a rule is not enforceable, it should be removed from CODING_STYLE, not
> removed.
  ^^^^^^^
Oops, you should read "ignored" here.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

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

[Blue Swirl]

On Sat, Jul 31, 2010 at 4:23 PM, malc <av1474@comtv.ru> wrote:
> History has shown that this particular rule is unenforcable.
>
> Signed-off-by: malc <av1474@comtv.ru>
> ---
>  CODING_STYLE |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)

Not again:
http://lists.nongnu.org/archive/html/qemu-devel/2009-12/msg00484.html

There are plenty of ways to make the rule enforceable, for example we
could agree to start to revert commits which introduce new
CODING_STYLE violations.

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

[malc]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1099 bytes --]

On Sat, 31 Jul 2010, Blue Swirl wrote:

> On Sat, Jul 31, 2010 at 4:23 PM, malc <av1474@comtv.ru> wrote:
> > History has shown that this particular rule is unenforcable.
> >
> > Signed-off-by: malc <av1474@comtv.ru>
> > ---
> >  CODING_STYLE |   11 ++++++-----
> >  1 files changed, 6 insertions(+), 5 deletions(-)
> 
> Not again:
> http://lists.nongnu.org/archive/html/qemu-devel/2009-12/msg00484.html

I'm not yet senile enough to forget that, let me quote some:

"By picking a single coding style we've pretty much guaranteed that most 
people will disagree with some of it. IMO consistency is more important."

We don't have consistency, people, myself included (which was a suprise),
keep applying patches that break things in this particular regard, it
should just go.

> 
> There are plenty of ways to make the rule enforceable, for example we
> could agree to start to revert commits which introduce new
> CODING_STYLE violations.
> 

There might be plenty of ways, but none were/are used, we somehow
managed to enforce tab rule, but the braces remain elusive.

-- 
mailto:av1474@comtv.ru

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

[Aurelien Jarno]

On Sat, Jul 31, 2010 at 08:23:55PM +0000, Blue Swirl wrote:
> On Sat, Jul 31, 2010 at 4:23 PM, malc <av1474@comtv.ru> wrote:
> > History has shown that this particular rule is unenforcable.
> >
> > Signed-off-by: malc <av1474@comtv.ru>
> > ---
> >  CODING_STYLE |   11 ++++++-----
> >  1 files changed, 6 insertions(+), 5 deletions(-)
> 
> Not again:
> http://lists.nongnu.org/archive/html/qemu-devel/2009-12/msg00484.html
> 
> There are plenty of ways to make the rule enforceable, for example we
> could agree to start to revert commits which introduce new
> CODING_STYLE violations.
> 

It seems to be possible to add a pre-applypatch script to the git hook
directory, that will verify the commit and reject it if it doesn't
comply with the coding rules. Of course it's possible to commit a patch
anyway by using --no-verify.

The good point of this approach is that the rule is enforced by a
script, which is not suppose to make mistakes, and that it can be shared
between patch submitters and patch committers: both side can make
mistakes and it is always better to know that as early as possible.

Of course someone as to translate the coding rules in a set of regular
expressions able to catch errors.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

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

[Anthony Liguori]

On 07/31/2010 06:49 PM, Aurelien Jarno wrote:
> On Sat, Jul 31, 2010 at 08:23:55PM +0000, Blue Swirl wrote:
>    
>> On Sat, Jul 31, 2010 at 4:23 PM, malc<av1474@comtv.ru>  wrote:
>>      
>>> History has shown that this particular rule is unenforcable.
>>>
>>> Signed-off-by: malc<av1474@comtv.ru>
>>> ---
>>>   CODING_STYLE |   11 ++++++-----
>>>   1 files changed, 6 insertions(+), 5 deletions(-)
>>>        
>> Not again:
>> http://lists.nongnu.org/archive/html/qemu-devel/2009-12/msg00484.html
>>
>> There are plenty of ways to make the rule enforceable, for example we
>> could agree to start to revert commits which introduce new
>> CODING_STYLE violations.
>>
>>      
> It seems to be possible to add a pre-applypatch script to the git hook
> directory, that will verify the commit and reject it if it doesn't
> comply with the coding rules. Of course it's possible to commit a patch
> anyway by using --no-verify.
>    

There are certain aspects of CODING_STYLE that I think are pretty 
important.  For instance, space vs. tabs can really screw things up for 
people that have non-standard tabs.  This is something that enforcing at 
patch submission time seems to be really important.

Type naming seems important too because it's often not isolated.  IOW, a 
poor choice in one file can end up polluting other files quickly that 
require interacting.  The result is a mess of naming styles.

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.

So really, I think the problem is that we're enforcing the words of 
CODING_STYLE instead of the intent.  The intent of CODING_STYLE is to 
improve the readability of the code.  I think it requires a certain 
amount of taste to be applied.

Rejecting a big patch because braces aren't used in single line if 
statements seems to be an unnecessary barrier to me.

Regards,

Anthony Liguori

> The good point of this approach is that the rule is enforced by a
> script, which is not suppose to make mistakes, and that it can be shared
> between patch submitters and patch committers: both side can make
> mistakes and it is always better to know that as early as possible.
>
> Of course someone as to translate the coding rules in a set of regular
> expressions able to catch errors.
>
>    

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

[Kevin Wolf]

Am 02.08.2010 17:20, schrieb Anthony Liguori:
> On 07/31/2010 06:49 PM, Aurelien Jarno wrote:
>> On Sat, Jul 31, 2010 at 08:23:55PM +0000, Blue Swirl wrote:
>>    
>>> On Sat, Jul 31, 2010 at 4:23 PM, malc<av1474@comtv.ru>  wrote:
>>>      
>>>> History has shown that this particular rule is unenforcable.
>>>>
>>>> Signed-off-by: malc<av1474@comtv.ru>
>>>> ---
>>>>   CODING_STYLE |   11 ++++++-----
>>>>   1 files changed, 6 insertions(+), 5 deletions(-)
>>>>        
>>> Not again:
>>> http://lists.nongnu.org/archive/html/qemu-devel/2009-12/msg00484.html
>>>
>>> There are plenty of ways to make the rule enforceable, for example we
>>> could agree to start to revert commits which introduce new
>>> CODING_STYLE violations.
>>>
>>>      
>> It seems to be possible to add a pre-applypatch script to the git hook
>> directory, that will verify the commit and reject it if it doesn't
>> comply with the coding rules. Of course it's possible to commit a patch
>> anyway by using --no-verify.
>>    
> 
> There are certain aspects of CODING_STYLE that I think are pretty 
> important.  For instance, space vs. tabs can really screw things up for 
> people that have non-standard tabs.  This is something that enforcing at 
> patch submission time seems to be really important.
> 
> Type naming seems important too because it's often not isolated.  IOW, a 
> poor choice in one file can end up polluting other files quickly that 
> require interacting.  The result is a mess of naming styles.
> 
> 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.

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 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.

> So really, I think the problem is that we're enforcing the words of
> CODING_STYLE instead of the intent.  The intent of CODING_STYLE is to 
> improve the readability of the code.  I think it requires a certain 
> amount of taste to be applied.
> 
> Rejecting a big patch because braces aren't used in single line if 
> statements seems to be an unnecessary barrier to me.

Taking such patches anyway is basically what we're doing today, right?
And what malc is complaining about.

Kevin

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

[Anthony Liguori]

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

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

[malc]

On Mon, 2 Aug 2010, Kevin Wolf wrote:

> Am 02.08.2010 17:20, schrieb Anthony Liguori:
> > On 07/31/2010 06:49 PM, Aurelien Jarno wrote:
> >> On Sat, Jul 31, 2010 at 08:23:55PM +0000, Blue Swirl wrote:

[..snip..]

> 
> > So really, I think the problem is that we're enforcing the words of
> > CODING_STYLE instead of the intent.  The intent of CODING_STYLE is to 
> > improve the readability of the code.  I think it requires a certain 
> > amount of taste to be applied.
>
> > Rejecting a big patch because braces aren't used in single line if 
> > statements seems to be an unnecessary barrier to me.
> 
> Taking such patches anyway is basically what we're doing today, right?
> And what malc is complaining about.

Malc is mainly complaining about "Quod licet Jovi non licet bovi"
situation.

-- 
mailto:av1474@comtv.ru

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

[Anthony Liguori]

On 08/02/2010 10:55 AM, malc wrote:
>>> Rejecting a big patch because braces aren't used in single line if
>>> statements seems to be an unnecessary barrier to me.
>>>        
>> Taking such patches anyway is basically what we're doing today, right?
>> And what malc is complaining about.
>>      
> Malc is mainly complaining about "Quod licet Jovi non licet bovi"
> situation.
>    

Very fair point.

Regards,

Anthony Liguori

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

[malc]

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?

-- 
mailto:av1474@comtv.ru

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

[Anthony Liguori]

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

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

[Blue Swirl]

On Mon, Aug 2, 2010 at 3:20 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 07/31/2010 06:49 PM, Aurelien Jarno wrote:
>>
>> On Sat, Jul 31, 2010 at 08:23:55PM +0000, Blue Swirl wrote:
>>
>>>
>>> On Sat, Jul 31, 2010 at 4:23 PM, malc<av1474@comtv.ru>  wrote:
>>>
>>>>
>>>> History has shown that this particular rule is unenforcable.
>>>>
>>>> Signed-off-by: malc<av1474@comtv.ru>
>>>> ---
>>>>  CODING_STYLE |   11 ++++++-----
>>>>  1 files changed, 6 insertions(+), 5 deletions(-)
>>>>
>>>
>>> Not again:
>>> http://lists.nongnu.org/archive/html/qemu-devel/2009-12/msg00484.html
>>>
>>> There are plenty of ways to make the rule enforceable, for example we
>>> could agree to start to revert commits which introduce new
>>> CODING_STYLE violations.
>>>
>>>
>>
>> It seems to be possible to add a pre-applypatch script to the git hook
>> directory, that will verify the commit and reject it if it doesn't
>> comply with the coding rules. Of course it's possible to commit a patch
>> anyway by using --no-verify.
>>
>
> There are certain aspects of CODING_STYLE that I think are pretty important.
>  For instance, space vs. tabs can really screw things up for people that
> have non-standard tabs.  This is something that enforcing at patch
> submission time seems to be really important.
>
> Type naming seems important too because it's often not isolated.  IOW, a
> poor choice in one file can end up polluting other files quickly that
> require interacting.  The result is a mess of naming styles.
>
> 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.
>
> So really, I think the problem is that we're enforcing the words of
> CODING_STYLE instead of the intent.  The intent of CODING_STYLE is to
> improve the readability of the code.  I think it requires a certain amount
> of taste to be applied.

The problem with that approach is that CODING_STYLE is not written
like that at all. All four requirements equally state that they
describe QEMU coding style. There is also no room left for
interpretation: "Every indented statement is braced; even if the block
contains just one
statement."

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

[Blue Swirl]

On Mon, Aug 2, 2010 at 4:18 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> 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.

I'd rather expand the document. For example, I like the approach libvirt takes:
http://libvirt.org/git/?p=libvirt.git;a=blob_plain;f=HACKING;hb=HEAD

Not specifically to braces, but they describe types and memory
allocation etc. which is FAQ stuff for us too.

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

[Anthony Liguori]

On 08/02/2010 11:29 AM, Blue Swirl wrote:
>> Yeah, I'm neglecting the fact that we're not consistent as maintainers and
>> I'm all for dropping it from CODING_STYLE.
>>      
> I'd rather expand the document. For example, I like the approach libvirt takes:
> http://libvirt.org/git/?p=libvirt.git;a=blob_plain;f=HACKING;hb=HEAD
>
> Not specifically to braces, but they describe types and memory
> allocation etc. which is FAQ stuff for us too.
>    

I agree 100%.  There are a lot of idioms in QEMU that would be good to 
document for people learning the code base.

But I think that's orthogonal to the discussion of whether we want to 
keep the if rule and whether we want to enforce all of coding style at 
commit time.

Regards,

Anthony Liguori