2007-05-08 19:39:43

by Randy Dunlap

[permalink] [raw]
Subject: Re: CodingStyle: start flamewar about use of braces

On Tue, 8 May 2007 19:03:10 GMT Linux Kernel Mailing List wrote:

> Gitweb: http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=e659ba4a0d2d471c0d73590f78e1a1b5a1eede48
> Commit: e659ba4a0d2d471c0d73590f78e1a1b5a1eede48
> Parent: 28be5abb400e5e082f5225105fdc69337ec0c0b4
> Author: Oliver Neukum <[email protected]>
> AuthorDate: Tue May 8 00:30:34 2007 -0700
> Committer: Linus Torvalds <[email protected]>
> CommitDate: Tue May 8 11:15:12 2007 -0700
>
> CodingStyle: start flamewar about use of braces

It worked somewhat. We never did reach any kind of
concensus about this....


> Signed-off-by: Oliver Neukum <[email protected]>
> Cc: Tilman Schmidt <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> Signed-off-by: Linus Torvalds <[email protected]>
> ---
> Documentation/CodingStyle | 15 +++++++++++++++
> 1 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
> index 9069189..e7f5fc6 100644
> --- a/Documentation/CodingStyle
> +++ b/Documentation/CodingStyle
> @@ -160,6 +160,21 @@ supply of new-lines on your screen is not a renewable resource (think
> 25-line terminal screens here), you have more empty lines to put
> comments on.
>
> +Do not unnecessarily use braces where a single statement will do.
> +
> +if (condition)
> + action();
> +
> +This does not apply if one branch of a conditional statement is a single
> +statement. Use braces in both branches.
> +
> +if (condition) {
> + do_this();
> + do_that();
> +} else {
> + otherwise();
> +}
> +
> 3.1: Spaces
>
> Linux kernel style for use of spaces depends (mostly) on


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***


2007-05-08 21:19:55

by Jeff Garzik

[permalink] [raw]
Subject: Re: CodingStyle: start flamewar about use of braces

Randy Dunlap wrote:
> On Tue, 8 May 2007 19:03:10 GMT Linux Kernel Mailing List wrote:
>
>> Gitweb: http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=e659ba4a0d2d471c0d73590f78e1a1b5a1eede48
>> Commit: e659ba4a0d2d471c0d73590f78e1a1b5a1eede48
>> Parent: 28be5abb400e5e082f5225105fdc69337ec0c0b4
>> Author: Oliver Neukum <[email protected]>
>> AuthorDate: Tue May 8 00:30:34 2007 -0700
>> Committer: Linus Torvalds <[email protected]>
>> CommitDate: Tue May 8 11:15:12 2007 -0700
>>
>> CodingStyle: start flamewar about use of braces
>
> It worked somewhat. We never did reach any kind of
> concensus about this....
>
>
>> Signed-off-by: Oliver Neukum <[email protected]>
>> Cc: Tilman Schmidt <[email protected]>
>> Signed-off-by: Andrew Morton <[email protected]>
>> Signed-off-by: Linus Torvalds <[email protected]>
>> ---
>> Documentation/CodingStyle | 15 +++++++++++++++
>> 1 files changed, 15 insertions(+), 0 deletions(-)
>>
>> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
>> index 9069189..e7f5fc6 100644
>> --- a/Documentation/CodingStyle
>> +++ b/Documentation/CodingStyle
>> @@ -160,6 +160,21 @@ supply of new-lines on your screen is not a renewable resource (think
>> 25-line terminal screens here), you have more empty lines to put
>> comments on.
>>
>> +Do not unnecessarily use braces where a single statement will do.
>> +
>> +if (condition)
>> + action();
>> +
>> +This does not apply if one branch of a conditional statement is a single
>> +statement. Use braces in both branches.
>> +
>> +if (condition) {
>> + do_this();
>> + do_that();
>> +} else {
>> + otherwise();
>> +}

If anyone tries to add braces to my code's 'else' statements where they
are not required, that patch will get NAK'd in a heartbeat.

Jeff


2007-05-08 22:25:48

by Lennart Sorensen

[permalink] [raw]
Subject: Re: CodingStyle: start flamewar about use of braces

On Tue, May 08, 2007 at 05:19:45PM -0400, Jeff Garzik wrote:
> >>+Do not unnecessarily use braces where a single statement will do.
> >>+
> >>+if (condition)
> >>+ action();
> >>+
> >>+This does not apply if one branch of a conditional statement is a single
> >>+statement. Use braces in both branches.
> >>+
> >>+if (condition) {
> >>+ do_this();
> >>+ do_that();
> >>+} else {
> >>+ otherwise();
> >>+}
>
> If anyone tries to add braces to my code's 'else' statements where they
> are not required, that patch will get NAK'd in a heartbeat.

Oh isn't coding style fun. I personally hate code that doesn't ALWAYS
have the braces everywhere since it makes adding a print statement
or other debuging to the condition such a pain since you then have to
add braces to the condition to avoid breaking the code just to insert
a print statement. It is one of the few things I disagree with in the
linux kernel coding style.

--
Len Sorensen

2007-05-08 22:50:41

by Stephen Clark

[permalink] [raw]
Subject: Re: CodingStyle: start flamewar about use of braces

Lennart Sorensen wrote:

>On Tue, May 08, 2007 at 05:19:45PM -0400, Jeff Garzik wrote:
>
>
>>>>+Do not unnecessarily use braces where a single statement will do.
>>>>+
>>>>+if (condition)
>>>>+ action();
>>>>+
>>>>+This does not apply if one branch of a conditional statement is a single
>>>>+statement. Use braces in both branches.
>>>>+
>>>>+if (condition) {
>>>>+ do_this();
>>>>+ do_that();
>>>>+} else {
>>>>+ otherwise();
>>>>+}
>>>>
>>>>
>>If anyone tries to add braces to my code's 'else' statements where they
>>are not required, that patch will get NAK'd in a heartbeat.
>>
>>
>
>Oh isn't coding style fun. I personally hate code that doesn't ALWAYS
>have the braces everywhere since it makes adding a print statement
>or other debuging to the condition such a pain since you then have to
>add braces to the condition to avoid breaking the code just to insert
>a print statement. It is one of the few things I disagree with in the
>linux kernel coding style.
>
>--
>Len Sorensen
>-
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/
>
>
>
I agree - it is merely defensive programming to always have the braces,
it prevents someone
from sticking in a diag and forgetting to add braces so things go haywire.

My $.02
Steve

--

"They that give up essential liberty to obtain temporary safety,
deserve neither liberty nor safety." (Ben Franklin)

"The course of history shows that as a government grows, liberty
decreases." (Thomas Jefferson)



2007-05-08 23:01:34

by Jeff Garzik

[permalink] [raw]
Subject: Re: CodingStyle: start flamewar about use of braces

Stephen Clark wrote:
> Lennart Sorensen wrote:
>> On Tue, May 08, 2007 at 05:19:45PM -0400, Jeff Garzik wrote:
>>> If anyone tries to add braces to my code's 'else' statements where
>>> they are not required, that patch will get NAK'd in a heartbeat.

>> Oh isn't coding style fun. I personally hate code that doesn't ALWAYS
>> have the braces everywhere since it makes adding a print statement
>> or other debuging to the condition such a pain since you then have to
>> add braces to the condition to avoid breaking the code just to insert
>> a print statement. It is one of the few things I disagree with in the
>> linux kernel coding style.

> I agree - it is merely defensive programming to always have the braces,
> it prevents someone
> from sticking in a diag and forgetting to add braces so things go haywire.


If you compulsively add braces where not needed, the number of lines in
the source file explodes. Net result is code that is more difficult to
read, because far fewer lines of code fit on a single page/screen.

But hey, it's personal preference. I'm not going to enforce the
make-code-more-readable rule on others' code... :)

Jeff


2007-05-08 23:37:38

by Satyam Sharma

[permalink] [raw]
Subject: Re: CodingStyle: start flamewar about use of braces

On 5/9/07, Jeff Garzik <[email protected]> wrote:
> Randy Dunlap wrote:
> > On Tue, 8 May 2007 19:03:10 GMT Linux Kernel Mailing List wrote:
> >
> >> Gitweb: http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=e659ba4a0d2d471c0d73590f78e1a1b5a1eede48
> >> Commit: e659ba4a0d2d471c0d73590f78e1a1b5a1eede48
> >> Parent: 28be5abb400e5e082f5225105fdc69337ec0c0b4
> >> Author: Oliver Neukum <[email protected]>
> >> AuthorDate: Tue May 8 00:30:34 2007 -0700
> >> Committer: Linus Torvalds <[email protected]>
> >> CommitDate: Tue May 8 11:15:12 2007 -0700
> >>
> >> CodingStyle: start flamewar about use of braces
> >
> > It worked somewhat. We never did reach any kind of
> > concensus about this....
> >
> >
> >> Signed-off-by: Oliver Neukum <[email protected]>
> >> Cc: Tilman Schmidt <[email protected]>
> >> Signed-off-by: Andrew Morton <[email protected]>
> >> Signed-off-by: Linus Torvalds <[email protected]>
> >> ---
> >> Documentation/CodingStyle | 15 +++++++++++++++
> >> 1 files changed, 15 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
> >> index 9069189..e7f5fc6 100644
> >> --- a/Documentation/CodingStyle
> >> +++ b/Documentation/CodingStyle
> >> @@ -160,6 +160,21 @@ supply of new-lines on your screen is not a renewable resource (think
> >> 25-line terminal screens here), you have more empty lines to put
> >> comments on.
> >>
> >> +Do not unnecessarily use braces where a single statement will do.
> >> +
> >> +if (condition)
> >> + action();
> >> +
> >> +This does not apply if one branch of a conditional statement is a single
> >> +statement. Use braces in both branches.
> >> +
> >> +if (condition) {
> >> + do_this();
> >> + do_that();
> >> +} else {
> >> + otherwise();
> >> +}
>
> If anyone tries to add braces to my code's 'else' statements where they
> are not required, that patch will get NAK'd in a heartbeat.

Yes. This patch shouldn't go in. There's no ambiguity in
Documentation/CodingStyle that it fixes anyway, only *imposes* a
"style", which:

(1) there is no consensus about *at all*, and

(2) blatantly goes against _all_ similar examples in K&R.

It's quite funny to worship them as "prophets" and adopt their style
for some stuff (with a simple "Rationale: K&R.") one instant and then
suggest a style completely opposite to theirs one screenful later in
the same file.

2007-05-09 08:17:56

by Stefan Richter

[permalink] [raw]
Subject: Re: CodingStyle: start flamewar about use of braces

Satyam Sharma wrote:
> It's quite funny to worship them as "prophets" and adopt their style
> for some stuff (with a simple "Rationale: K&R.") one instant and then
> suggest a style completely opposite to theirs one screenful later in
> the same file.

That's what happens to a text which is incrementally written by people
with different writing styles (and different opinions).

As a slightly off-topic side note: CodingStyle was once a guideline
which sort of worked because it was a compact and entertaining read; so
some people took the time to read it and didn't mind too much to adopt
the preferred style. It appears that CodingStyle is now being
transformed into a norm which the authors want to make work by being
comprehensive, and by being able to point offenders to a chapter/
section/ clause which they breached. Alas only a few people will be
able and willing to memorize CodingStyle's rules then.

I don't know which form is better suited to the goal of well
maintainable code --- the compact, easy to read, winning guideline, or
the comprehensive norm.
--
Stefan Richter
-=====-=-=== -=-= -=--=
http://arcgraph.de/sr/

2007-05-09 08:59:45

by Pekka Enberg

[permalink] [raw]
Subject: Re: CodingStyle: start flamewar about use of braces

On 5/9/07, Stefan Richter <[email protected]> wrote:
> I don't know which form is better suited to the goal of well
> maintainable code --- the compact, easy to read, winning guideline, or
> the comprehensive norm.

Bah, it's a slippery slope. Too little guidelines and we end up people
submitting code that is "obviously" violating Linux coding standards
on the basis of that it's not forbidden by Documentation/CodingStyle.
And with too much detail, we end up with something that nobody wants
to read let alone follow. So what CodingStyle should say, really, is
that maintainers have different preference and you're usually better
off following them than fighting against them ;-).