Streamlining CodingStyle

Submitted by Jeremy
on October 1, 2007 - 5:15pm

Recent efforts to add additional details to Documentation/CodingStyle prompted Linus Torvalds to reply, "I'm not very happy with this." He explained:

"'CodingStyle' should be about the big issues, not about details. Yes, we've messed that up over the years, but let's not continue that. In other words, I'd suggest *removing* lines from CodingStyle, not adding them. The file has already gone from a 'good general principles' to 'lots of stupid details'. Let's not make it worse."

Erez Zadok pointed out, "there's a lot of good value in having all those details, as it helps people standardize on more common practices, including details. I think removing all those details may only increase the amount-of less-accepted code to be posted, resulting in more lkml people having to repeat themselves on what not to do." He went on to ask, "would you prefer if CodingStyle was reorganized or even split into (1) general principles and (2) details? Perhaps we need a CodingStylePrinciples and a CodingStyleDetails?" Linus responded favorably, "I'm certainly ok with the split into two files. What I'm not ok with is really important stuff (indentation), and then mixing in silly rules (`parenthesis are bad in printk's`?)"


From: Erez Zadok
Subject: [PATCH] 0/3 coding standards documentation/code updates
Date: Sep 28, 2:31 pm 2007

The following is a series of patches related to coding standards.  The first
patch updates the CodingStandards document with respect to printk debugging
and un/likely use; the second patch updates the usage string for
checkpatch.pl; the third patch introduces a new small perl script that can
be used to check coding-standards compliance on multiple source files.  This
new script, called check-coding-standards.pl, produces output in a standard
compiler-error format, which can be parsed by assorted tools including text
editors and help users locate the file that had the error and the offending
line-number more quickly.

Erez Zadok (3):
      CodingStyle updates
      Update usage string for checkpatch.pl
      New script to check coding-style compliance on multiple regular files

 Documentation/CodingStyle         |   88 +++++++++++++++++++++++++++++++++++++-
 scripts/check-coding-standards.pl |   59 +++++++++++++++++++++++++
 scripts/checkpatch.pl             |    1 
 3 files changed, 146 insertions(+), 2 deletions(-)

---
Erez Zadok
ezk@cs.sunysb.edu
-

From: Linus Torvalds Subject: Re: [PATCH] 0/3 coding standards documentation/code updates Date: Sep 29, 11:18 am 2007 On Fri, 28 Sep 2007, Erez Zadok wrote: > > Documentation/CodingStyle | 88 +++++++++++++++++++++++++++++++++++++- I'm not very happy with this. "CodingStyle" should be about the big issues, not about details. Yes, we've messed that up over the years, but let's not continue that. In other words, I'd suggest *removing* lines from CodingStyle, not adding them. The file has already gone from a "good general principles" to "lots of stupid details". Let's not make it worse. Linus -
From: Erez Zadok Subject: Re: [PATCH] 0/3 coding standards documentation/code updates Date: Sep 29, 5:23 pm 2007 In message <alpine.LFD.0.999.0709291116180.3579@woody.linux-foundation.org>, Linus Torvalds writes: > > > On Fri, 28 Sep 2007, Erez Zadok wrote: > > > > Documentation/CodingStyle | 88 +++++++++++++++++++++++++++++++++++++- > > I'm not very happy with this. > > "CodingStyle" should be about the big issues, not about details. Yes, > we've messed that up over the years, but let's not continue that. > > In other words, I'd suggest *removing* lines from CodingStyle, not adding > them. The file has already gone from a "good general principles" to "lots > of stupid details". Let's not make it worse. > > Linus There's a lot of good value in having all those details, as it helps people standardize on more common practices, including details. I think removing all those details may only increase the amount-of less-accepted code to be posted, resulting in more lkml people having to repeat themselves on what not to do. Only now, they won't be able to point people to the CodingStyle file for what they should do right. Would you prefer if CodingStyle was reorganized or even split into (1) general principles and (2) details? Perhaps we need a CodingStylePrinciples and a CodingStyleDetails? IOW, where should that very useful info live? Erez. -
From: Linus Torvalds Subject: Re: [PATCH] 0/3 coding standards documentation/code updates Date: Sep 29, 5:49 pm 2007 On Sat, 29 Sep 2007, Erez Zadok wrote: > > Would you prefer if CodingStyle was reorganized or even split into (1) > general principles and (2) details? Perhaps we need a CodingStylePrinciples > and a CodingStyleDetails? I'm certainly ok with the split into two files. What I'm not ok with is really important stuff (indentation), and then mixing in silly rules ("parenthesis are bad in printk's"?) Linus -

From: Valdis.Kletnieks
Subject: Re: [PATCH] 0/3 coding standards documentation/code updates
Date: Sep 29, 7:24 pm 2007

On Sat, 29 Sep 2007 11:18:05 PDT, Linus Torvalds said:

> "CodingStyle" should be about the big issues, not about details. Yes, 
> we've messed that up over the years, but let's not continue that.
> 
> In other words, I'd suggest *removing* lines from CodingStyle, not adding 
> them. The file has already gone from a "good general principles" to "lots 
> of stupid details". Let's not make it worse.

I think there needs to be a "sense of fairness" attached here - CodingStyle
should cover all the stuff maintainers/reviewers are allowed to whinge about.
Otherwise, we get maintainers whinging about undocumented style rules, and
we get code submitters who are unhappy because they have no real way to tell
if their code really *is* stylistically lacking, or if they just have a
jerk maintainer who wants to keep their code out-of-tree for political
reasons.

From: Al Viro Subject: Re: [PATCH] 0/3 coding standards documentation/code updates Date: Sep 29, 8:27 pm 2007 On Sat, Sep 29, 2007 at 10:24:01PM -0400, Valdis.Kletnieks@vt.edu wrote: > On Sat, 29 Sep 2007 11:18:05 PDT, Linus Torvalds said: > > > "CodingStyle" should be about the big issues, not about details. Yes, > > we've messed that up over the years, but let's not continue that. > > > > In other words, I'd suggest *removing* lines from CodingStyle, not adding > > them. The file has already gone from a "good general principles" to "lots > > of stupid details". Let's not make it worse. > > I think there needs to be a "sense of fairness" attached here - CodingStyle > should cover all the stuff maintainers/reviewers are allowed to whinge about. > Otherwise, we get maintainers whinging about undocumented style rules, and > we get code submitters who are unhappy because they have no real way to tell > if their code really *is* stylistically lacking, or if they just have a > jerk maintainer who wants to keep their code out-of-tree for political > reasons. ... and no matter how many rules you put down, it's still possible to write a code that will be awful stylistically while adhering to all of them. Religiously. IOW, it's not going to eliminate that kind of fights. -
From: Linus Torvalds Subject: Re: [PATCH] 0/3 coding standards documentation/code updates Date: Sep 29, 8:00 pm 2007 On Sat, 29 Sep 2007, Valdis.Kletnieks@vt.edu wrote: > > I think there needs to be a "sense of fairness" attached here - CodingStyle > should cover all the stuff maintainers/reviewers are allowed to whinge about. No. People whine too much as is. Don't give them *license* to do so. Linus -
From: Valdis.Kletnieks Subject: Re: [PATCH] 0/3 coding standards documentation/code updates Date: Sep 29, 8:29 pm 2007 On Sat, 29 Sep 2007 20:00:01 PDT, Linus Torvalds said: > On Sat, 29 Sep 2007, Valdis.Kletnieks@vt.edu wrote: > > I think there needs to be a "sense of fairness" attached here - CodingStyle > > should cover all the stuff maintainers/reviewers are allowed to whinge about. > > No. > > People whine too much as is. Don't give them *license* to do so. I kind of meant the *maintainers* couldn't whine about things not in CodingStyle ;)
From: Linus Torvalds Subject: Re: [PATCH] 0/3 coding standards documentation/code updates Date: Sep 29, 8:35 pm 2007 On Sat, 29 Sep 2007, Valdis.Kletnieks@vt.edu wrote: > > I kind of meant the *maintainers* couldn't whine about things not in > CodingStyle ;) No, I understood you. I'm personally of the opinion that the automated style checking, and having detailed rules is always a mistake. It's *much* better to teach people the big things. And the small things will vary from person to person _anyway_, so it's not like you can really document them. Linus -

The dumbest smart people I know

Anonymous (not verified)
on
October 2, 2007 - 3:49am

When kernel developers don't like something they just build their own. Why not just build a formatting application that has to be run on the source code before it's accepted into git. That way each person can code to their preference yet the source control system only contains code formatted one way.

Read more closely:

Mr_Z
on
October 2, 2007 - 4:47am
From: Linus Torvalds 
Subject: Re: [PATCH] 0/3 coding standards documentation/code updates
Date: Sep 29, 6:35 pm 2007

On Sat, 29 Sep 2007, Valdis.Kletnieks@vt.edu wrote:
> 
> I kind of meant the *maintainers* couldn't whine about things not in 
> CodingStyle ;)

No, I understood you.

I'm personally of the opinion that the automated style checking, and 
having detailed rules is always a mistake. 

I tend to agree, personally. And I agree with the reasons, too, which is part of the reason you'll never see me program languages that enforce certain aspects of style, such as Python. It's just braindamaged and wrong.

For example, automatic formatters tend to screw up formatting on nice tabular stuff like this:

    if      (addr >= 4  && addr <= 6 ) data &= 0x0F;
    else if (addr == 9               ) data &= 0x1F;
    else if (addr == 10              ) data &= 0x0F;
    else if (addr >= 11 && addr <= 13) data &= 0x3F;
    else                               data &= 0xFF;

I think you'd be hard pressed to find someone who says that the output of GNU indent on this is more readable, regardless of the flags you give GNU indent. If you tried to take the baseline Linux Kernel coding guidelines, you'd end up with:

        if (addr >= 4 && addr <= 6) 
                data &= 0x0F;
        else if (addr == 9) 
                data &= 0x1F;
        else if (addr == 10) 
                data &= 0x0F;
        else if (addr >= 11 && addr <= 13) 
                data &= 0x3F;
        else
                data &= 0xFF;

That's much harder to read, because everything's interleaved. An automated tool might have some heuristics for guessing when to put the "then" clause on the same line as the "if", but it still probably won't format the conditions nicely:

        if (addr >= 4 && addr <= 6) data &= 0x0F;
        else if (addr == 9) data &= 0x1F;
        else if (addr == 10) data &= 0x0F;
        else if (addr >= 11 && addr <= 13) data &= 0x3F;
        else data &= 0xFF;

So, I totally see Linus' point. Have a sense of taste and you'll do ok. Keep sight of the broad issues (tabs vs. spaces, indentation levels, white space around operators and where the opening curly goes) and most of the rest should fall out.
--
Program Intellivision and play Space Patrol!

I agree with Linus. I agree

Anonymous (not verified)
on
October 2, 2007 - 4:46pm

I agree with Linus.
I agree with your post too, but only partially.

I vehemently disagree with this:

"which is part of the reason you'll never see me program languages that enforce certain aspects of style, such as Python. It's just braindamaged and wrong."

THAT OPINION IS WRONG.

You must understand that python enforces its way upon you, for the good or for the bad.
It IS NOT INHERENTLY BAD. It's YOU that is making it bad.

Now, I do not agree with some choices of python, but your opinion is way too extreme.
I am using Ruby instead of Python, but I DO think that enforcing a way is NOT a bad
way at all - you have several advantages, like not bothering to think for other
ways to a problem in the first case (and the "only" way is documented
nicely)

This is NOT inherently BAD.

But of course, Ruby as a language is better due to several OTHER decisions, such
as no need for the () if you dont *want* to, no indent, no implicit self.
But then again, joy comes with a little price to pay, like religious people
continually whining about speed issues, neglecting that ALL the scripting
languages are in the SAME speed-league ;-)

Bzzt.

Mr_Z
on
October 3, 2007 - 3:40am

One common debugging idiom I use is to put temporary debugging printf's and related code in column 0 so it stands out as temporary code. Works in many, many languages, and makes it quite clear what's supposed to stay and what's supposed to go.

Python, in its religious fervor about indentation, won't even forgive me if one line got indented with tabs and the next with spaces, let alone allow this useful idiom.

--
Program Intellivision and play Space Patrol!

You misunderstood my post

Anonymous (not verified)
on
October 3, 2007 - 3:59am

I agree that gnu indent or any other formatting tool may do a bad job. Just like CVS, Subversion, etc did a bad job for linux source control. What did linux developers do? They wrote their own version control system. Why is this not analogous? There aren't any code formatting tools out there that format the way the kernel developers like, right? So why not write one that does? I don't see how that can be harder than writing a kernel or version control system.

My point is there's always exceptions

Mr_Z
on
October 3, 2007 - 12:20pm

People won't be able to agree on the last decimal point, and in most cases it won't matter. Just like not all kernel developers use git, the kernel doesn't conform to a strict coding style down to the last comma. It doesn't make sense to harden something that's inherently soft.

--
Program Intellivision and play Space Patrol!

much ado about nothing

undefined
on
October 26, 2007 - 12:52pm
if a:
    # normal indented comment
    b = func1(a)
# DEBUG (single line)
    print "DEBUG: b = %s" % (b)
    c = func2(a)
    d = func3(b, c)
# DEBUG BEGIN (multiple lines)
    print "DEBUG: c = %s" % (c)
    print "DEBUG: d = %s" % (d)
# DEBUG END

which is part of the reason

intgr
on
October 2, 2007 - 8:08pm

which is part of the reason you'll never see me program languages that enforce certain aspects of style, such as Python.

Sorry for veering offtopic, but this is simply a common misconception/prejudice against Python. Python does not "enforce" you to use any particular style. While Python might resemble classical languages like BASIC, Pascal, etc., Python is in fact very liberal with regards to coding style.

The only thing it enforces compared to C is that you have to use indentation to delimit code blocks, iff you want the block to span more than one line — any form of indentation. And every bit of code accepted into a respected project will use indentation anyway.

Your example code is completely legitimate in Python (and even looks clearer than C):

if   addr >= 4  and addr <= 6 : data &= 0x0F
elif addr == 9                : data &= 0x1F
elif addr >= 11 and addr <= 13: data &= 0x3F
else                          : data &= 0xFF

You can have multiple statements on a single line delimited by ;, just as in C.

And contrary to popular belief, you can span a single statement over multiple lines, without ugly newline escapes. Indentation within brackets has no significance and this applies to any sort of brackets.

if (addr >= 4 and
    addr <= 6):
  data &= 0x0F

Polishing a turd

Lawrence D'Oliveiro (not verified)
on
October 13, 2007 - 2:28pm

I would never write a tabular layout in a free-format language like C. The fact that you even consider it is just ... brain-damaged. And I would rewrite your code as

switch (addr)
  {
case 4:
case 5:
case 6:
case 10:
    mask = 0x0f;
break;
case 9:
    mask = 0x1f;
break;
case 11:
case 12:
case 13:
    mask = 0x3f;
break;
default:
    mask = 0xff;
break;
  } /*switch*/
data &= mask;

Format before commit

Anonymous (not verified)
on
October 2, 2007 - 5:06am

Formatting code on the fly doesn't work well in practice. But CVS and Subversion have pre-commit hooks that can be rigged to do this. An added complication is that code is committed to many GIT trees before making it to Lignus. There are lots of code formatting tools. Most are deficient. The best, most flexible way to control indentation is though emacs. Use batch mode just like you would sed.

emacs -batch sample.c --eval '(indent-region nil)' -f save-buffer

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.