[PATCH/v2] CodingStyle updates

[PATCH/v2] CodingStyle updates

[Randy Dunlap]

From: Randy Dunlap <randy.dunlap@oracle.com>

Add some kernel coding style comments, mostly pulled from emails
by Andrew Morton, Jesper Juhl, and Randy Dunlap.

- add paragraph on switch/case indentation (with fixes)
- add paragraph on multiple-assignments
- add more on Braces
- add section on Spaces; add typeof, alignof, & __attribute__ with sizeof;
  add more on postfix/prefix increment/decrement operators
- add paragraph on function breaks in source files; add info on
  function prototype parameter names
- add paragraph on EXPORT_SYMBOL placement
- add section on /*-comment style, long-comment style, and data
  declarations and comments
- correct some chapter number references that were missed when
  chapters were renumbered

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Acked-by: Jesper Juhl <jesper.juhl@gmail.com>
---
 Documentation/CodingStyle |  125 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 120 insertions(+), 5 deletions(-)

--- linux-2.6.19-git11.orig/Documentation/CodingStyle
+++ linux-2.6.19-git11/Documentation/CodingStyle
@@ -35,12 +35,37 @@ In short, 8-char indents make things eas
 benefit of warning you when you're nesting your functions too deep.
 Heed that warning.
 
+The preferred way to ease multiple indentation levels in a switch
+statement is to align the "switch" and its subordinate "case" labels in
+the same column instead of "double-indenting" the "case" labels.  E.g.:
+
+	switch (suffix) {
+	case 'G':
+	case 'g':
+		mem <<= 30;
+		break;
+	case 'M':
+	case 'm':
+		mem <<= 20;
+		break;
+	case 'K':
+	case 'k':
+		mem <<= 10;
+		/* fall through */
+	default:
+		break;
+	}
+
+
 Don't put multiple statements on a single line unless you have
 something to hide:
 
 	if (condition) do_this;
 	  do_something_everytime;
 
+Don't put multiple assignments on a single line either.  Kernel
+coding style is super simple.  Avoid tricky expressions.
+
 Outside of comments, documentation and except in Kconfig, spaces are never
 used for indentation, and the above example is deliberately broken.
 
@@ -69,7 +94,7 @@ void fun(int a, int b, int c)
 		next_statement;
 }
 
-		Chapter 3: Placing Braces
+		Chapter 3: Placing Braces and Spaces
 
 The other issue that always comes up in C styling is the placement of
 braces.  Unlike the indent size, there are few technical reasons to
@@ -81,6 +106,20 @@ brace last on the line, and put the clos
 		we do y
 	}
 
+This applies to all non-function statement blocks (if, switch, for,
+while, do).  E.g.:
+
+	switch (action) {
+	case KOBJ_ADD:
+		return "add";
+	case KOBJ_REMOVE:
+		return "remove";
+	case KOBJ_CHANGE:
+		return "change";
+	default:
+		return NULL;
+	}
+
 However, there is one special case, namely functions: they have the
 opening brace at the beginning of the next line, thus:
 
@@ -121,6 +160,48 @@ supply of new-lines on your screen is no
 25-line terminal screens here), you have more empty lines to put
 comments on.
 
+		3.1:  Spaces
+
+Linux kernel style for use of spaces depends (mostly) on
+function-versus-keyword usage.  Use a space after (most) keywords.  The
+notable exceptions are sizeof, typeof, alignof, and __attribute__, which
+look somewhat like functions (and are usually used with parentheses in
+Linux, although they are not required in the language, as in: "sizeof info"
+after "struct fileinfo info;" is declared).
+
+So use a space after these keywords:
+	if, switch, case, for, do, while
+but not with sizeof, typeof, alignof, or __attribute__.  E.g.,
+	s = sizeof(struct file);
+
+Do not add spaces around (inside) parenthesized expressions.
+This example is *bad*:
+
+	s = sizeof( struct file );
+
+When declaring pointer data or a function that returns a pointer type,
+the preferred use of '*' is adjacent to the data name or function name
+and not adjacent to the type name.  Examples:
+
+	char *linux_banner;
+	unsigned long long memparse(char *ptr, char **retptr);
+	char *match_strdup(substring_t *s);
+
+Use one space around (on each side of) most binary and ternary operators,
+such as any of these:
+	=  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
+
+but no space after unary operators:
+	&  *  +  -  ~  !  sizeof  typeof  alignof  __attribute__  defined
+
+no space before the postfix increment & decrement unary operators:
+	++  --
+
+no space after the prefix increment & decrement unary operators:
+	++  --
+
+and no space around the '.' and "->" structure member operators.
+
 
 		Chapter 4: Naming
 
@@ -152,7 +233,7 @@ variable that is used to hold a temporar
 
 If you are afraid to mix up your local variable names, you have another
 problem, which is called the function-growth-hormone-imbalance syndrome.
-See next chapter.
+See chapter 6 (Functions).
 
 
 		Chapter 5: Typedefs
@@ -258,6 +339,20 @@ generally easily keep track of about 7 d
 and it gets confused.  You know you're brilliant, but maybe you'd like
 to understand what you did 2 weeks from now.
 
+In source files, separate functions with one blank line.
+If the function is exported, the EXPORT* macro for it should follow
+immediately after the closing function brace line.  E.g.:
+
+int system_is_up(void)
+{
+	return system_state == SYSTEM_RUNNING;
+}
+EXPORT_SYMBOL(system_is_up);
+
+In function prototypes, include parameter names with their data types.
+Although this is not required by the C language, it is preferred in Linux
+because it is a simple way to add valuable information for the reader.
+
 
 		Chapter 7: Centralized exiting of functions
 
@@ -306,16 +401,36 @@ time to explain badly written code.
 Generally, you want your comments to tell WHAT your code does, not HOW.
 Also, try to avoid putting comments inside a function body: if the
 function is so complex that you need to separately comment parts of it,
-you should probably go back to chapter 5 for a while.  You can make
+you should probably go back to chapter 6 for a while.  You can make
 small comments to note or warn about something particularly clever (or
 ugly), but try to avoid excess.  Instead, put the comments at the head
 of the function, telling people what it does, and possibly WHY it does
 it.
 
-When commenting the kernel API functions, please use the kerneldoc format.
+When commenting the kernel API functions, please use the kernel-doc format.
 See the files Documentation/kernel-doc-nano-HOWTO.txt and scripts/kernel-doc
 for details.
 
+Linux style for comments is the C89 "/* ... */" style.
+Don't use C99-style "// ..." comments.
+
+The preferred style for long (multi-line) comments is:
+
+	/*
+	 * This is the preferred style for multi-line
+	 * comments in the Linux kernel source code.
+	 * Please use it consistently.
+	 *
+	 * Description:  A column of asterisks on the left side,
+	 * with beginning and ending almost-blank lines.
+	 */
+
+It's also important to comment data, whether they are basic types or
+derived types.  To this end, use just one data declaration per line
+(no commas for multiple data declarations).  This leaves you room for
+a small comment on each item, explaining its use.
+
+
 		Chapter 9: You've made a mess of it
 
 That's OK, we all do.  You've probably been told by your long-time Unix
@@ -591,4 +706,4 @@ Kernel CodingStyle, by greg@kroah.com at
 http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_talk/html/
 
 --
-Last updated on 30 April 2006.
+Last updated on 2006-December-06.


---

Re: [PATCH/v2] CodingStyle updates

[Jan Engelhardt]

On Dec 7 2006 16:55, Randy Dunlap wrote:

>Date: Thu, 7 Dec 2006 16:55:08 -0800
>From: Randy Dunlap <randy.dunlap@oracle.com>
>To: lkml <linux-kernel@vger.kernel.org>
>Cc: jesper.juhl@gmail.com, akpm <akpm@osdl.org>
>Subject: [PATCH/v2] CodingStyle updates
>
>From: Randy Dunlap <randy.dunlap@oracle.com>
>
>Add some kernel coding style comments, mostly pulled from emails
>by Andrew Morton, Jesper Juhl, and Randy Dunlap.
>
>- add paragraph on switch/case indentation (with fixes)
>- add paragraph on multiple-assignments
>- add more on Braces
>- add section on Spaces; add typeof, alignof, & __attribute__ with sizeof;
>  add more on postfix/prefix increment/decrement operators
>- add paragraph on function breaks in source files; add info on
>  function prototype parameter names
>- add paragraph on EXPORT_SYMBOL placement
>- add section on /*-comment style, long-comment style, and data
>  declarations and comments
>- correct some chapter number references that were missed when
>  chapters were renumbered
>
>Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
>Acked-by: Jesper Juhl <jesper.juhl@gmail.com>

Acked-by: Jan Engelhardt <jengelh@gmx.de>

	-`J'
-- 

Re: [PATCH/v2] CodingStyle updates

[Pavel Machek]

Hi!

> From: Randy Dunlap <randy.dunlap@oracle.com>
> 
> Add some kernel coding style comments, mostly pulled from emails
> by Andrew Morton, Jesper Juhl, and Randy Dunlap.
...
> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> Acked-by: Jesper Juhl <jesper.juhl@gmail.com>

ACK.

> +Use one space around (on each side of) most binary and ternary operators,
> +such as any of these:
> +	=  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :

Actually, this should not be hard rule. We want to allow

	j = 3*i + l<<2;
							Pavel
-- 
Thanks for all the (sleeping) penguins.

Re: [PATCH/v2] CodingStyle updates

[Stefan Richter]

Pavel Machek wrote:
>> From: Randy Dunlap <randy.dunlap@oracle.com>
>> +Use one space around (on each side of) most binary and ternary operators,
>> +such as any of these:
>> +	=  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
> 
> Actually, this should not be hard rule. We want to allow
> 
> 	j = 3*i + l<<2;

Which would be very misleading. This expression evaluates to

	j = (((3 * i) + l) << 2);

Binary + precedes <<.
-- 
Stefan Richter
-=====-=-==- ==-- -====
http://arcgraph.de/sr/

Re: [PATCH/v2] CodingStyle updates

[Pavel Machek]

Hi!

> Pavel Machek wrote:
> >> From: Randy Dunlap <randy.dunlap@oracle.com>
> >> +Use one space around (on each side of) most binary and ternary operators,
> >> +such as any of these:
> >> +	=  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
> > 
> > Actually, this should not be hard rule. We want to allow
> > 
> > 	j = 3*i + l<<2;
> 
> Which would be very misleading. This expression evaluates to
> 
> 	j = (((3 * i) + l) << 2);
> 
> Binary + precedes <<.

Aha, okay. So this one should be written as

	j = 3*i+l << 2;

(Well, parenthesses should really be used. Anyway, sometimes grouping
around operator is useful, even if I made mistake demonstrating that.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH/v2] CodingStyle updates

[Stefan Richter]

Pavel Machek wrote:
>> Pavel Machek wrote:
>> >> From: Randy Dunlap <randy.dunlap@oracle.com>
>> >> +Use one space around (on each side of) most binary and ternary operators,
>> >> +such as any of these:
>> >> +	=  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
>> > 
>> > Actually, this should not be hard rule.
[...]
> sometimes grouping around operator is useful,
[...]

I agree.

By the way, the longer CodingStyle becomes, the less people will read it.
-- 
Stefan Richter
-=====-=-==- ==-- -====
http://arcgraph.de/sr/

Re: [PATCH/v2] CodingStyle updates

[Scott Preece]

On 12/15/06, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
> > Pavel Machek wrote:
> > >> From: Randy Dunlap <randy.dunlap@oracle.com>
> > >> +Use one space around (on each side of) most binary and ternary operators,
> > >> +such as any of these:
> > >> +  =  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
> > >
> > > Actually, this should not be hard rule. We want to allow
> > >
> > >     j = 3*i + l<<2;
> >
> > Which would be very misleading. This expression evaluates to
> >
> >       j = (((3 * i) + l) << 2);
> >
> > Binary + precedes <<.
>
> Aha, okay. So this one should be written as
>
>         j = 3*i+l << 2;
>
> (Well, parenthesses should really be used. Anyway, sometimes grouping
> around operator is useful, even if I made mistake demonstrating that.
---

I think the mistake illuminates why parentheses should be the rule. If
you're thinking about using spacing to convey grouping, use
parentheses instead...

scott

Re: [PATCH/v2] CodingStyle updates

[Stefan Richter]

Scott Preece wrote:
> I think the mistake illuminates why parentheses should be the rule. If
> you're thinking about using spacing to convey grouping, use
> parentheses instead...

But then, we do reflect operator precedence by omitting whitespace
around . and ->, before [], or after unary & and *.
-- 
Stefan Richter
-=====-=-==- ==-- -====
http://arcgraph.de/sr/

Re: [PATCH/v2] CodingStyle updates

[Pavel Machek]

On Fri 2006-12-15 08:52:22, Scott Preece wrote:
> On 12/15/06, Pavel Machek <pavel@ucw.cz> wrote:
> >Hi!
> >
> >> Pavel Machek wrote:
> >> >> From: Randy Dunlap <randy.dunlap@oracle.com>
> >> >> +Use one space around (on each side of) most binary and ternary 
> >operators,
> >> >> +such as any of these:
> >> >> +  =  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
> >> >
> >> > Actually, this should not be hard rule. We want to allow
> >> >
> >> >     j = 3*i + l<<2;
> >>
> >> Which would be very misleading. This expression evaluates to
> >>
> >>       j = (((3 * i) + l) << 2);
> >>
> >> Binary + precedes <<.
> >
> >Aha, okay. So this one should be written as
> >
> >        j = 3*i+l << 2;
> >
> >(Well, parenthesses should really be used. Anyway, sometimes grouping
> >around operator is useful, even if I made mistake demonstrating that.
> ---
> 
> I think the mistake illuminates why parentheses should be the rule. If
> you're thinking about using spacing to convey grouping, use
> parentheses instead...

Not in simple cases.

	3*i + 2*j should be writen like that. Not like
	(3 * i) + (2 * j)

								Pavel


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH/v2] CodingStyle updates

[Scott Preece]

On 12/15/06, Pavel Machek <pavel@ucw.cz> wrote:
> On Fri 2006-12-15 08:52:22, Scott Preece wrote
> >
> > I think the mistake illuminates why parentheses should be the rule. If
> > you're thinking about using spacing to convey grouping, use
> > parentheses instead...
>
> Not in simple cases.
>
>         3*i + 2*j should be writen like that. Not like
>         (3 * i) + (2 * j)
---

Actually, my preference would be to use the parentheses AND drop the
spaces:   (3*i)+(2*j) . But, existing kernel code seems to prefer just
using spaces and adding parentheses when it gets complicated. Note
that the mistake in your example was in a relatively simple
expression.

I think the unary operator case is a little different - it's not so
much to make precedence clear as just to help the reader chunk the
pieces of the string more naturally (at least, it's more natural to
anyone who's ever done anything object-oriented). I agree that your
spacing example, above, also helps that way, but it seems better to do
it in a way that's meaningful o the compiler as well as to the reader.

scott

Re: [PATCH/v2] CodingStyle updates

[Randy Dunlap]

On Fri, 15 Dec 2006 16:07:17 +0100 Pavel Machek wrote:

> On Fri 2006-12-15 08:52:22, Scott Preece wrote:
> > On 12/15/06, Pavel Machek <pavel@ucw.cz> wrote:
> > >Hi!
> > >
> > >> Pavel Machek wrote:
> > >> >> From: Randy Dunlap <randy.dunlap@oracle.com>
> > >> >> +Use one space around (on each side of) most binary and ternary 
> > >operators,
> > >> >> +such as any of these:
> > >> >> +  =  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
> > >> >
> > >> > Actually, this should not be hard rule. We want to allow
> > >> >
> > >> >     j = 3*i + l<<2;
> > >>
> > >> Which would be very misleading. This expression evaluates to
> > >>
> > >>       j = (((3 * i) + l) << 2);
> > >>
> > >> Binary + precedes <<.
> > >
> > >Aha, okay. So this one should be written as
> > >
> > >        j = 3*i+l << 2;
> > >
> > >(Well, parenthesses should really be used. Anyway, sometimes grouping
> > >around operator is useful, even if I made mistake demonstrating that.
> > ---
> > 
> > I think the mistake illuminates why parentheses should be the rule. If
> > you're thinking about using spacing to convey grouping, use
> > parentheses instead...
> 
> Not in simple cases.
> 
> 	3*i + 2*j should be writen like that. Not like
> 	(3 * i) + (2 * j)

I would just write it as:
	3 * i + 2 * j

---
~Randy

Re: [PATCH/v2] CodingStyle updates

[Jan Engelhardt]

On Dec 15 2006 15:28, Stefan Richter wrote:
>Pavel Machek wrote:
>>> Pavel Machek wrote:
>>> >> From: Randy Dunlap <randy.dunlap@oracle.com>
>>> >> +Use one space around (on each side of) most binary and ternary operators,
>>> >> +such as any of these:
>>> >> +	=  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
>>> > 
>>> > Actually, this should not be hard rule.
>[...]
>> sometimes grouping around operator is useful,
>[...]
>
>I agree.
>
>By the way, the longer CodingStyle becomes, the less people will read it.

The bible is also quite long.



	-`J'
-- 

Re: [PATCH/v2] CodingStyle updates

[Randy Dunlap]

On Fri, 15 Dec 2006 15:28:44 +0100 Stefan Richter wrote:

> Pavel Machek wrote:
> >> Pavel Machek wrote:
> >> >> From: Randy Dunlap <randy.dunlap@oracle.com>
> >> >> +Use one space around (on each side of) most binary and ternary operators,
> >> >> +such as any of these:
> >> >> +	=  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
> >> > 
> >> > Actually, this should not be hard rule.
> [...]
> > sometimes grouping around operator is useful,
> [...]
> 
> I agree.
> 
> By the way, the longer CodingStyle becomes, the less people will read it.

That's a good point IMO.

Maybe we could just summarize it with something like:

Use spaces around binary operators for readability but not to imply
any kind of grouping.

But I suppose that Pavel doesn't agree with that.  Oh well.

---
~Randy

Re: [PATCH/v2] CodingStyle updates

[Jörn Engel]

On Fri, 15 December 2006 09:00:37 -0800, Randy Dunlap wrote:
> On Fri, 15 Dec 2006 16:07:17 +0100 Pavel Machek wrote:
> 
> > Not in simple cases.
> > 
> > 	3*i + 2*j should be writen like that. Not like
> > 	(3 * i) + (2 * j)
> 
> I would just write it as:
> 	3 * i + 2 * j

So would I.  But I definitely wouldn't write
	for (i = 0; i < 10; i += 2)
because I prefer the grouping in
	for (i=0; i<10; i+=2)

Pavel may have picked a bad example, but there are cases when spaces can
be used to group code.  Just as empty lines can be used to group code.
And in both cases the reason should be "readability".

Which variant is the most readable is a highly personal matter and may
alos change over time for any group of people.  I'd vote against a stone
tablet with 10 commandments of taste.  "Make it readable, use common
sense" is so much better, imo.

Jörn

-- 
The cheapest, fastest and most reliable components of a computer
system are those that aren't there.
-- Gordon Bell, DEC labratories

Re: [PATCH/v2] CodingStyle updates

[Randy Dunlap]

On Fri, 15 Dec 2006 20:11:27 +0000 Jörn Engel wrote:

> On Fri, 15 December 2006 09:00:37 -0800, Randy Dunlap wrote:
> > On Fri, 15 Dec 2006 16:07:17 +0100 Pavel Machek wrote:
> > 
> > > Not in simple cases.
> > > 
> > > 	3*i + 2*j should be writen like that. Not like
> > > 	(3 * i) + (2 * j)
> > 
> > I would just write it as:
> > 	3 * i + 2 * j
> 
> So would I.  But I definitely wouldn't write
> 	for (i = 0; i < 10; i += 2)

I would and do (the above).

> because I prefer the grouping in
> 	for (i=0; i<10; i+=2)
> 
> Pavel may have picked a bad example, but there are cases when spaces can
> be used to group code.  Just as empty lines can be used to group code.
> And in both cases the reason should be "readability".

Agreed.

> Which variant is the most readable is a highly personal matter and may
> alos change over time for any group of people.  I'd vote against a stone
> tablet with 10 commandments of taste.  "Make it readable, use common
> sense" is so much better, imo.

then send patches  :)

---
~Randy

Re: [PATCH/v2] CodingStyle updates

[Rafael J. Wysocki]

On Friday, 15 December 2006 18:00, Randy Dunlap wrote:
> On Fri, 15 Dec 2006 16:07:17 +0100 Pavel Machek wrote:
> 
> > On Fri 2006-12-15 08:52:22, Scott Preece wrote:
> > > On 12/15/06, Pavel Machek <pavel@ucw.cz> wrote:
> > > >Hi!
> > > >
> > > >> Pavel Machek wrote:
> > > >> >> From: Randy Dunlap <randy.dunlap@oracle.com>
> > > >> >> +Use one space around (on each side of) most binary and ternary 
> > > >operators,
> > > >> >> +such as any of these:
> > > >> >> +  =  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
> > > >> >
> > > >> > Actually, this should not be hard rule. We want to allow
> > > >> >
> > > >> >     j = 3*i + l<<2;
> > > >>
> > > >> Which would be very misleading. This expression evaluates to
> > > >>
> > > >>       j = (((3 * i) + l) << 2);
> > > >>
> > > >> Binary + precedes <<.
> > > >
> > > >Aha, okay. So this one should be written as
> > > >
> > > >        j = 3*i+l << 2;
> > > >
> > > >(Well, parenthesses should really be used. Anyway, sometimes grouping
> > > >around operator is useful, even if I made mistake demonstrating that.
> > > ---
> > > 
> > > I think the mistake illuminates why parentheses should be the rule. If
> > > you're thinking about using spacing to convey grouping, use
> > > parentheses instead...
> > 
> > Not in simple cases.
> > 
> > 	3*i + 2*j should be writen like that. Not like
> > 	(3 * i) + (2 * j)
> 
> I would just write it as:
> 	3 * i + 2 * j

\metoo

Re: [PATCH/v2] CodingStyle updates

[Dmitry Torokhov]

On 12/15/06, Jörn Engel <joern@lazybastard.org> wrote:
> On Fri, 15 December 2006 09:00:37 -0800, Randy Dunlap wrote:
> > On Fri, 15 Dec 2006 16:07:17 +0100 Pavel Machek wrote:
> >
> > > Not in simple cases.
> > >
> > >     3*i + 2*j should be writen like that. Not like
> > >     (3 * i) + (2 * j)
> >
> > I would just write it as:
> >       3 * i + 2 * j
>
> So would I.  But I definitely wouldn't write
>        for (i = 0; i < 10; i += 2)
> because I prefer the grouping in
>        for (i=0; i<10; i+=2)
>

Would you write:

       i+=2;

outside the loop? If not then it is better to keep style consistent
and not use condensed form in loops either.

-- 
Dmitry

Re: [PATCH/v2] CodingStyle updates

[Jan Engelhardt]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 865 bytes --]


On Dec 15 2006 15:56, Dmitry Torokhov wrote:
> On 12/15/06, Jörn Engel <joern@lazybastard.org> wrote:
>> On Fri, 15 December 2006 09:00:37 -0800, Randy Dunlap wrote:
>> > On Fri, 15 Dec 2006 16:07:17 +0100 Pavel Machek wrote:
>> > 
>> > > Not in simple cases.
>> > > 
>> > > 3*i + 2*j should be writen like that. Not like
>> > > (3 * i) + (2 * j)
>> > 
>> > I would just write it as:
>> > 3 * i + 2 * j
>> 
>> So would I.  But I definitely wouldn't write
>>       for (i = 0; i < 10; i += 2)
>> because I prefer the grouping in
>> for (i=0; i<10; i+=2)
>> 
>
> Would you write:
>
>      i+=2;
>
> outside the loop? If not then it is better to keep style consistent
> and not use condensed form in loops either.

Don't you all even think about leaving the whitespace away in the wrong
place, otherwise the kernel is likely to become an entry for IOCCC.


	-`J'
-- 

Re: [PATCH/v2] CodingStyle updates

[Jörn Engel]

On Fri, 15 December 2006 12:26:59 -0800, Randy Dunlap wrote:
> 
> then send patches  :)

Like so?  I manually edited the patch and weakened a few of the space
rules, basically the ones in dispute in this thread.

From: Randy Dunlap <randy.dunlap@oracle.com>

Add some kernel coding style comments, mostly pulled from emails
by Andrew Morton, Jesper Juhl, and Randy Dunlap.

- add paragraph on switch/case indentation (with fixes)
- add paragraph on multiple-assignments
- add more on Braces
- add section on Spaces; add typeof, alignof, & __attribute__ with sizeof;
  add more on postfix/prefix increment/decrement operators
- add paragraph on function breaks in source files; add info on
  function prototype parameter names
- add paragraph on EXPORT_SYMBOL placement
- add section on /*-comment style, long-comment style, and data
  declarations and comments
- correct some chapter number references that were missed when
  chapters were renumbered

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Acked-by: Jesper Juhl <jesper.juhl@gmail.com>
Acked-by: Jörn Engel <joern@lazybastard.org>
---
 Documentation/CodingStyle |  129 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 124 insertions(+), 5 deletions(-)

--- linux-2.6.19-git11.orig/Documentation/CodingStyle
+++ linux-2.6.19-git11/Documentation/CodingStyle
@@ -35,12 +35,37 @@ In short, 8-char indents make things eas
 benefit of warning you when you're nesting your functions too deep.
 Heed that warning.
 
+The preferred way to ease multiple indentation levels in a switch
+statement is to align the "switch" and its subordinate "case" labels in
+the same column instead of "double-indenting" the "case" labels.  E.g.:
+
+	switch (suffix) {
+	case 'G':
+	case 'g':
+		mem <<= 30;
+		break;
+	case 'M':
+	case 'm':
+		mem <<= 20;
+		break;
+	case 'K':
+	case 'k':
+		mem <<= 10;
+		/* fall through */
+	default:
+		break;
+	}
+
+
 Don't put multiple statements on a single line unless you have
 something to hide:
 
 	if (condition) do_this;
 	  do_something_everytime;
 
+Don't put multiple assignments on a single line either.  Kernel
+coding style is super simple.  Avoid tricky expressions.
+
 Outside of comments, documentation and except in Kconfig, spaces are never
 used for indentation, and the above example is deliberately broken.
 
@@ -69,7 +94,7 @@ void fun(int a, int b, int c)
 		next_statement;
 }
 
-		Chapter 3: Placing Braces
+		Chapter 3: Placing Braces and Spaces
 
 The other issue that always comes up in C styling is the placement of
 braces.  Unlike the indent size, there are few technical reasons to
@@ -81,6 +106,20 @@ brace last on the line, and put the clos
 		we do y
 	}
 
+This applies to all non-function statement blocks (if, switch, for,
+while, do).  E.g.:
+
+	switch (action) {
+	case KOBJ_ADD:
+		return "add";
+	case KOBJ_REMOVE:
+		return "remove";
+	case KOBJ_CHANGE:
+		return "change";
+	default:
+		return NULL;
+	}
+
 However, there is one special case, namely functions: they have the
 opening brace at the beginning of the next line, thus:
 
@@ -121,6 +160,52 @@ supply of new-lines on your screen is no
 25-line terminal screens here), you have more empty lines to put
 comments on.
 
+		3.1:  Spaces
+
+Linux kernel style for use of spaces depends (mostly) on
+function-versus-keyword usage.  Use a space after (most) keywords.  The
+notable exceptions are sizeof, typeof, alignof, and __attribute__, which
+look somewhat like functions (and are usually used with parentheses in
+Linux, although they are not required in the language, as in: "sizeof info"
+after "struct fileinfo info;" is declared).
+
+So use a space after these keywords:
+	if, switch, case, for, do, while
+but not with sizeof, typeof, alignof, or __attribute__.  E.g.,
+	s = sizeof(struct file);
+
+Do not add spaces around (inside) parenthesized expressions.
+This example is *bad*:
+
+	s = sizeof( struct file );
+
+When declaring pointer data or a function that returns a pointer type,
+the preferred use of '*' is adjacent to the data name or function name
+and not adjacent to the type name.  Examples:
+
+	char *linux_banner;
+	unsigned long long memparse(char *ptr, char **retptr);
+	char *match_strdup(substring_t *s);
+
+When placing spaces around operators, try to make the code as readable
+as possible.  While readability is a highly personal matter, the
+following rules are a good starting point (and _not_ the ultimate
+wisdom in all cases, mind you ;):
+Tend to use one space around (on each side of) most binary and ternary
+operators, such as any of these:
+	=  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
+
+avoid spaces after unary operators:
+	&  *  +  -  ~  !  sizeof  typeof  alignof  __attribute__  defined
+
+no spaces before the postfix increment & decrement unary operators:
+	++  --
+
+avoid spaces after the prefix increment & decrement unary operators:
+	++  --
+
+and no space around the '.' and "->" structure member operators.
+
 
 		Chapter 4: Naming
 
@@ -152,7 +233,7 @@ variable that is used to hold a temporar
 
 If you are afraid to mix up your local variable names, you have another
 problem, which is called the function-growth-hormone-imbalance syndrome.
-See next chapter.
+See chapter 6 (Functions).
 
 
 		Chapter 5: Typedefs
@@ -258,6 +339,20 @@ generally easily keep track of about 7 d
 and it gets confused.  You know you're brilliant, but maybe you'd like
 to understand what you did 2 weeks from now.
 
+In source files, separate functions with one blank line.
+If the function is exported, the EXPORT* macro for it should follow
+immediately after the closing function brace line.  E.g.:
+
+int system_is_up(void)
+{
+	return system_state == SYSTEM_RUNNING;
+}
+EXPORT_SYMBOL(system_is_up);
+
+In function prototypes, include parameter names with their data types.
+Although this is not required by the C language, it is preferred in Linux
+because it is a simple way to add valuable information for the reader.
+
 
 		Chapter 7: Centralized exiting of functions
 
@@ -306,16 +401,36 @@ time to explain badly written code.
 Generally, you want your comments to tell WHAT your code does, not HOW.
 Also, try to avoid putting comments inside a function body: if the
 function is so complex that you need to separately comment parts of it,
-you should probably go back to chapter 5 for a while.  You can make
+you should probably go back to chapter 6 for a while.  You can make
 small comments to note or warn about something particularly clever (or
 ugly), but try to avoid excess.  Instead, put the comments at the head
 of the function, telling people what it does, and possibly WHY it does
 it.
 
-When commenting the kernel API functions, please use the kerneldoc format.
+When commenting the kernel API functions, please use the kernel-doc format.
 See the files Documentation/kernel-doc-nano-HOWTO.txt and scripts/kernel-doc
 for details.
 
+Linux style for comments is the C89 "/* ... */" style.
+Don't use C99-style "// ..." comments.
+
+The preferred style for long (multi-line) comments is:
+
+	/*
+	 * This is the preferred style for multi-line
+	 * comments in the Linux kernel source code.
+	 * Please use it consistently.
+	 *
+	 * Description:  A column of asterisks on the left side,
+	 * with beginning and ending almost-blank lines.
+	 */
+
+It's also important to comment data, whether they are basic types or
+derived types.  To this end, use just one data declaration per line
+(no commas for multiple data declarations).  This leaves you room for
+a small comment on each item, explaining its use.
+
+
 		Chapter 9: You've made a mess of it
 
 That's OK, we all do.  You've probably been told by your long-time Unix
@@ -591,4 +706,4 @@ Kernel CodingStyle, by greg@kroah.com at
 http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_talk/html/
 
 --
-Last updated on 30 April 2006.
+Last updated on 2006-December-06.



Jörn

-- 
Fancy algorithms are slow when n is small, and n is usually small.
Fancy algorithms have big constants. Until you know that n is
frequently going to be big, don't get fancy.
-- Rob Pike

Re: [PATCH/v2] CodingStyle updates

[Jörn Engel]

On Fri, 15 December 2006 21:10:14 +0000, Jörn Engel wrote:
> 
> Like so?  I manually edited the patch and weakened a few of the space
> rules, basically the ones in dispute in this thread.

Btw, this doesn't apply to my git tree at all (just pulled):
Hunk #1 FAILED at 35.
Hunk #2 FAILED at 94.
Hunk #3 succeeded at 145 with fuzz 1 (offset 39 lines).
Hunk #4 succeeded at 242 with fuzz 2 (offset 82 lines).
Hunk #5 FAILED at 315.
Hunk #6 succeeded at 435 with fuzz 2 (offset 96 lines).
Hunk #7 FAILED at 497.
Hunk #8 FAILED at 802.
5 out of 8 hunks FAILED -- saving rejects to file
Documentation/CodingStyle.rej

Is it against -mm or something such?

Jörn

-- 
When in doubt, punt.  When somebody actually complains, go back and fix it...
The 90% solution is a good thing.
-- Rob Landley

Re: [PATCH/v2] CodingStyle updates

[Randy Dunlap]

Jörn Engel wrote:
> On Fri, 15 December 2006 21:10:14 +0000, Jörn Engel wrote:
>> Like so?  I manually edited the patch and weakened a few of the space
>> rules, basically the ones in dispute in this thread.
> 
> Btw, this doesn't apply to my git tree at all (just pulled):
> Hunk #1 FAILED at 35.
> Hunk #2 FAILED at 94.
> Hunk #3 succeeded at 145 with fuzz 1 (offset 39 lines).
> Hunk #4 succeeded at 242 with fuzz 2 (offset 82 lines).
> Hunk #5 FAILED at 315.
> Hunk #6 succeeded at 435 with fuzz 2 (offset 96 lines).
> Hunk #7 FAILED at 497.
> Hunk #8 FAILED at 802.
> 5 out of 8 hunks FAILED -- saving rejects to file
> Documentation/CodingStyle.rej
> 
> Is it against -mm or something such?

It's already been merged, so you'll need to make new patches
against -current (as always).

-- 
~Randy

Re: [PATCH/v2] CodingStyle updates

[Jörn Engel]

On Fri, 15 December 2006 22:01:10 +0100, Jan Engelhardt wrote:
> On Dec 15 2006 15:56, Dmitry Torokhov wrote:
> >
> > Would you write:
> >
> >      i+=2;
> >
> > outside the loop? If not then it is better to keep style consistent
> > and not use condensed form in loops either.
> 
> Don't you all even think about leaving the whitespace away in the wrong
> place, otherwise the kernel is likely to become an entry for IOCCC.

Please, this is not religion.  No god has spoken "though shalt not...".

In 99% of all cases, what Randy wrote is the best choice.  But the
ultimate goal is not to follow his heavenly rules with fundamental
zealotry, the ultimate goal is to make the code readable.  If you
happen to stumble on the 1% case where the code can be more readable by
not following the rules, and you are absolutely sure about that, then
don't.  That simple. ;)

That said, people who are bereft of all common sense have my blessing to
follow the rules literally in their own code.  Just don't stomp over
mine with pitchforks and torches, ok?

Jörn

-- 
I can say that I spend most of my time fixing bugs even if I have lots
of new features to implement in mind, but I give bugs more priority.
-- Andrea Arcangeli, 2000

Re: [PATCH/v2] CodingStyle updates

[Jan Engelhardt]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1245 bytes --]


On Dec 15 2006 21:27, Jörn Engel wrote:
>On Fri, 15 December 2006 22:01:10 +0100, Jan Engelhardt wrote:
>> On Dec 15 2006 15:56, Dmitry Torokhov wrote:
>> >
>> > outside the loop? If not then it is better to keep style consistent
>> > and not use condensed form in loops either.
>> 
>> Don't you all even think about leaving the whitespace away in the wrong
>> place, otherwise the kernel is likely to become an entry for IOCCC.
>
>Please, this is not religion.  No god has spoken "though shalt not...".
>
>In 99% of all cases, what Randy wrote is the best choice.  But the

Of course it is - hey, I even "contributed" to it. Well, looks like I should be
adding in more smilies when making remarks like the above.

>ultimate goal is not to follow his heavenly rules with fundamental
>zealotry, the ultimate goal is to make the code readable.  If you
>happen to stumble on the 1% case where the code can be more readable by
>not following the rules, and you are absolutely sure about that, then
>don't.  That simple. ;)

I take it that people will automatically DTRT for obscure cases like
shown before. Well, and if they don't, hopefully some reviewer catches
things like 3*i + l<<2.

It's all right, I did not mean to step on toes.

	-`J'
-- 

2.6.20-rc1-mm1 suspicious prececence code ( was Re: [PATCH/v2] CodingStyle updates

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1130 bytes --]

On Fri, 15 Dec 2006 22:59:12 +0100, Jan Engelhardt said:

> I take it that people will automatically DTRT for obscure cases like
> shown before. Well, and if they don't, hopefully some reviewer catches
> things like 3*i + l<<2.

So I hacked up a few very ugly 'find|egrep' to look for some cases of that, and
found:

./include/asm-arm/arch-ebsa110/hardware.h:18: * Region 0 (addr = 0xf0000000 + io << 2)

Only one odd-looking use of +-*/ and <</>> - and it's in a comment.

And that's using a pattern like '\+[^,()=]*<<' (basically, any plus sign that
has a << after it, but no comma parens or equals to force grouping in between), and
then using /bin/eyeball to filter the resulting several hundred lines.
I admit I didn't try to catch expressions split over multiple lines, and
something of the form "foo * bar + (a-b) << 2" would have snuck by (but I
suspect if somebody bothered doing the (a-b), they would have another pair).

So either that sort of thing isn't really an error we make often, or the
reviewers are very good at catching it, or I'm a lot worse at finding them
than I thought I was... :)

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]