| Message ID | 20091026200326.GA9669@volta.aurel32.net |
|---|---|
| State | New |
| Headers | show |
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.
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 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
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");