From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1N2W3u-0002dS-SF for qemu-devel@nongnu.org; Mon, 26 Oct 2009 16:21:06 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1N2W3p-0002aG-BW for qemu-devel@nongnu.org; Mon, 26 Oct 2009 16:21:05 -0400 Received: from [199.232.76.173] (port=55443 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1N2W3p-0002a6-5Z for qemu-devel@nongnu.org; Mon, 26 Oct 2009 16:21:01 -0400 Received: from mail-fx0-f211.google.com ([209.85.220.211]:39807) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1N2W3o-0002z4-21 for qemu-devel@nongnu.org; Mon, 26 Oct 2009 16:21:01 -0400 Received: by fxm7 with SMTP id 7so11665983fxm.34 for ; Mon, 26 Oct 2009 13:20:54 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20091026200326.GA9669@volta.aurel32.net> References: <20091026062637.GA21562@volta.aurel32.net> <20091026200326.GA9669@volta.aurel32.net> From: Blue Swirl Date: Mon, 26 Oct 2009 22:20:34 +0200 Message-ID: Subject: Re: [Qemu-devel] [PATCH] CODING_STYLE: don't allow non-indented statements after if/else blocks Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: qemu-devel@nongnu.org On Mon, Oct 26, 2009 at 10:03 PM, Aurelien Jarno wro= te: > On Mon, Oct 26, 2009 at 06:02:52PM +0200, Blue Swirl wrote: >> On Mon, Oct 26, 2009 at 8:26 AM, Aurelien Jarno w= rote: >> > 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. =C2=A0The openi= ng brace >> > +is on the line that contains the control flow statement that introduc= es >> > +the new block; the closing brace is on the same line as the else keyw= ord, >> > +or on a line by itself if there is no else keyword. =C2=A0Example: >> >> I think an exception should be granted for "else if" case, otherwise >> the style would require braces around "if", like: >> =C2=A0 =C2=A0 if (a =3D=3D 5) { >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 printf("a was 5.\n"); >> =C2=A0 =C2=A0 } else { >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (a =3D=3D 6) { >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 printf("a was 6.\n"); >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 } >> =C2=A0 =C2=A0 } else { >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 printf("a was something else entirely.\n"); >> =C2=A0 =C2=A0 } >> >> 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 > > Rationale: The following code is difficult to read: > > =C2=A0 =C2=A0if (a =3D=3D 5) printf("a was 5.\n"); > =C2=A0 =C2=A0else if (a =3D=3D 6) printf("a was 6.\n"); > =C2=A0 =C2=A0else printf("a was something else entirely.\n"); > > Signed-off-by: Aurelien Jarno > --- > =C2=A0CODING_STYLE | =C2=A0 12 +++++++----- > =C2=A01 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. > > =C2=A04. Block structure > > -Every indented statement is braced; even if the block contains just one > -statement. =C2=A0The opening brace is on the line that contains the cont= rol > -flow statement that introduces the new block; the closing brace is on th= e > -same line as the else keyword, or on a line by itself if there is no els= e > -keyword. =C2=A0Example: > +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. =C2=A0The 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. =C2=A0Example: 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.