[Qemu-devel] [PATCH] CODING_STYLE: don't allow non-indented statements after if/else blocks

[Qemu-devel] [PATCH] CODING_STYLE: don't allow non-indented statements after if/else blocks

[Aurelien Jarno]

Rationale: The following code is difficult to read, but allowed by the
current coding style.

    if (a == 5) printf("a was 5.\n");
    else if (a == 6) printf("a was 6.\n");
    else printf("a was something else entirely.\n");

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 CODING_STYLE |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/CODING_STYLE b/CODING_STYLE
index a579cb1..3ffbc3d 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -51,11 +51,11 @@ QEMU coding style.
 
 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:
+Every control flow statement is followed by a new indented and braced
+block; 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:
 
     if (a == 5) {
         printf("a was 5.\n");
-- 
1.6.1.3

Re: [Qemu-devel] [PATCH] CODING_STYLE: don't allow non-indented statements after if/else blocks

[Blue Swirl]

On Mon, Oct 26, 2009 at 8:26 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> Rationale: The following code is difficult to read, but allowed by the
> current coding style.

Fully agree.

> +Every control flow statement is followed by a new indented and braced
> +block; 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:

I think an exception should be granted for "else if" case, otherwise
the style would require braces around "if", like:
    if (a == 5) {
        printf("a was 5.\n");
    } else {
        if (a == 6) {
            printf("a was 6.\n");
        }
    } else {
        printf("a was something else entirely.\n");
    }

Picking nits: "while" is a control flow statement, even in "do {}
while" statement and then it would illegal to require a braced block
after the "while" statement.

Re: [Qemu-devel] [PATCH] CODING_STYLE: don't allow non-indented statements after if/else blocks

[Aurelien Jarno]

On Mon, Oct 26, 2009 at 06:02:52PM +0200, Blue Swirl wrote:
> On Mon, Oct 26, 2009 at 8:26 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > Rationale: The following code is difficult to read, but allowed by the
> > current coding style.
> 
> Fully agree.
> 
> > +Every control flow statement is followed by a new indented and braced
> > +block; 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:
> 
> I think an exception should be granted for "else if" case, otherwise
> the style would require braces around "if", like:
>     if (a == 5) {
>         printf("a was 5.\n");
>     } else {
>         if (a == 6) {
>             printf("a was 6.\n");
>         }
>     } else {
>         printf("a was something else entirely.\n");
>     }
> 
> Picking nits: "while" is a control flow statement, even in "do {}
> while" statement and then it would illegal to require a braced block
> after the "while" statement.

Good point. Please find another try below:

From: Aurelien Jarno <aurelien@aurel32.net>

Rationale: The following code is difficult to read:

    if (a == 5) printf("a was 5.\n");
    else if (a == 6) printf("a was 6.\n");
    else printf("a was something else entirely.\n");

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 CODING_STYLE |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/CODING_STYLE b/CODING_STYLE
index a579cb1..c17c3f3 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -51,11 +51,13 @@ QEMU coding style.
 
 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:
+Every control flow statement is followed by a new indented and braced
+block, except if it is followed by another control flow statement (else
+if) or by a condition (do {} while ()); 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:
 
     if (a == 5) {
         printf("a was 5.\n");
-- 
1.6.1.3

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

Re: [Qemu-devel] [PATCH] CODING_STYLE: don't allow non-indented statements after if/else blocks

[Blue Swirl]

On Mon, Oct 26, 2009 at 10:03 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Mon, Oct 26, 2009 at 06:02:52PM +0200, Blue Swirl wrote:
>> On Mon, Oct 26, 2009 at 8:26 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
>> > Rationale: The following code is difficult to read, but allowed by the
>> > current coding style.
>>
>> Fully agree.
>>
>> > +Every control flow statement is followed by a new indented and braced
>> > +block; 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:
>>
>> I think an exception should be granted for "else if" case, otherwise
>> the style would require braces around "if", like:
>>     if (a == 5) {
>>         printf("a was 5.\n");
>>     } else {
>>         if (a == 6) {
>>             printf("a was 6.\n");
>>         }
>>     } else {
>>         printf("a was something else entirely.\n");
>>     }
>>
>> Picking nits: "while" is a control flow statement, even in "do {}
>> while" statement and then it would illegal to require a braced block
>> after the "while" statement.
>
> Good point. Please find another try below:
>
> From: Aurelien Jarno <aurelien@aurel32.net>
>
> Rationale: The following code is difficult to read:
>
>    if (a == 5) printf("a was 5.\n");
>    else if (a == 6) printf("a was 6.\n");
>    else printf("a was something else entirely.\n");
>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  CODING_STYLE |   12 +++++++-----
>  1 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/CODING_STYLE b/CODING_STYLE
> index a579cb1..c17c3f3 100644
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -51,11 +51,13 @@ QEMU coding style.
>
>  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:
> +Every control flow statement is followed by a new indented and braced
> +block, except if it is followed by another control flow statement (else
> +if) or by a condition (do {} while ()); 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:

Nice try, but does it prevent this:
    if (x) for (;;) do {
    } while (0);
?

Maybe also "break" and "goto" can be considered control flow statements.

How about something like "wherever C syntax allows potentially
ambiguous sequence of statements, braces must be used, with the
exception of 'else' followed by 'if'"? Now the problem becomes
defining ambiguous sequences of statements.

Re: [Qemu-devel] [PATCH] CODING_STYLE: don't allow non-indented statements after if/else blocks

[Aurelien Jarno]

On Mon, Oct 26, 2009 at 10:20:34PM +0200, Blue Swirl wrote:
> On Mon, Oct 26, 2009 at 10:03 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > On Mon, Oct 26, 2009 at 06:02:52PM +0200, Blue Swirl wrote:
> >> On Mon, Oct 26, 2009 at 8:26 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> >> > Rationale: The following code is difficult to read, but allowed by the
> >> > current coding style.
> >>
> >> Fully agree.
> >>
> >> > +Every control flow statement is followed by a new indented and braced
> >> > +block; 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:
> >>
> >> I think an exception should be granted for "else if" case, otherwise
> >> the style would require braces around "if", like:
> >>     if (a == 5) {
> >>         printf("a was 5.\n");
> >>     } else {
> >>         if (a == 6) {
> >>             printf("a was 6.\n");
> >>         }
> >>     } else {
> >>         printf("a was something else entirely.\n");
> >>     }
> >>
> >> Picking nits: "while" is a control flow statement, even in "do {}
> >> while" statement and then it would illegal to require a braced block
> >> after the "while" statement.
> >
> > Good point. Please find another try below:
> >
> > From: Aurelien Jarno <aurelien@aurel32.net>
> >
> > Rationale: The following code is difficult to read:
> >
> >    if (a == 5) printf("a was 5.\n");
> >    else if (a == 6) printf("a was 6.\n");
> >    else printf("a was something else entirely.\n");
> >
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> >  CODING_STYLE |   12 +++++++-----
> >  1 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/CODING_STYLE b/CODING_STYLE
> > index a579cb1..c17c3f3 100644
> > --- a/CODING_STYLE
> > +++ b/CODING_STYLE
> > @@ -51,11 +51,13 @@ QEMU coding style.
> >
> >  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:
> > +Every control flow statement is followed by a new indented and braced
> > +block, except if it is followed by another control flow statement (else
> > +if) or by a condition (do {} while ()); 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:
> 
> Nice try, but does it prevent this:
>     if (x) for (;;) do {
>     } while (0);
> ?

We should find a way to define "for (;;)" as a single statement.

> Maybe also "break" and "goto" can be considered control flow statements.

Correct, they should be excluded from there as they don't need to be
followed by braces.

> How about something like "wherever C syntax allows potentially
> ambiguous sequence of statements, braces must be used, with the
> exception of 'else' followed by 'if'"? Now the problem becomes
> defining ambiguous sequences of statements.

That's the problem. We have seen that people already take advantage of
the ambiguities, as I would have never have imagined someone writing the
code in the rationale of this patch to avoid putting braces.

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

Re: [Qemu-devel] [PATCH] CODING_STYLE: don't allow non-indented statements after if/else blocks

[Anthony Liguori]

Aurelien Jarno wrote:
> That's the problem. We have seen that people already take advantage of
> the ambiguities, as I would have never have imagined someone writing the
> code in the rationale of this patch to avoid putting braces.
>   

I appreciate the desire to be precise, but we aren't writing a compiler 
or an automated syntax checker.  We're writing a set of guidelines for 
reasonable people to follow.  If someone sets out to "defeat" 
CODING_STYLE by introducing some crazy syntax, the solution is 
simple--just don't take the patch.  Good sense must prevail regardless 
of what CODING_STYLE says.

Regards,

Anthony Liguori