kernel guide to space
Michael S. Tsirkin
I've been tasked with edicating some new hires on linux kernel coding style.
While we have Documentation/CodingStyle, it skips detail that is supposed to
be learned by example.
Since I've been burned by this a couple of times myself till I learned,
I've put together a short list of rules complementing Documentation/CodingStyle.
This list is attached, below.
Please cc me directly with comments, if any.
Thanks,
MST
---
kernel guide to space AKA a boring list of rules
http://www.mellanox.com/mst/boring.txt
This text deals mostly with whitespace issues, hence the name.
Whitespace -- In computer science, a whitespace (or a whitespace character) is
any character which does not display itself but does take up space.
From Wikipedia, the free encyclopedia.
1. Read Documentation/CodingStyle. Yes, it applies to you.
When working on a specific driver/subsystem, try to follow
the style of the surrounding codebase.
2. The last character on a line is never a whitespace
Get a decent editor and don't leave whitespace at the end of
lines.
Documentation/CodingStyle
Whitespace issues:
3. Space rules for C
3a. Binary operators
+ - / * %
== != > < >= <= && ||
& | ^ << >>
= *= /= %= += -= <<= >>= &= ^= |=
spaces around the operator
a + b
3b. Unary operators
! ~
+ - *
&
no space between operator and operand
*a
3c. * in types
Leave space between name and * in types.
Multiple * dont need additional space between them.
struct foo **bar;
3d. Conditional
?:
spaces around both ? and :
a ? b : c
3e. sizeof
space after the operator
sizeof a
3f. Braces etc
() [] -> .
no space around any of these (but see 3h)
foo(bar)
3g. Comma
,
space after comma, no space before comma
foo, bar
3h. Semicolon
;
no space before semicolon
foo;
3i. if/else/do/while/for/switch
space between if/else/do/while and following/preceeding
statements/expressions, if any:
if (a) {
} else {
}
do {
} while (b);
3j. Labels
goto and case labels should have a line of their own (possibly
with a comment).
No space before colon in labels.
int foobar()
{
...
foolabel: /* short comment */
foo();
}
4. Indentation rules for C
Use tabs, not spaces, for indentation. Tabs should be 8 characters wide.
4a. Labels
case labels should be indented same as the switch statement.
statements occurring after a case label are indented by one level.
switch (foo) {
case foo:
bar();
default:
break;
}
4b. Global scope
Functions, type definitions/declarations, defines, global
variables etc are global scope. Start them at the first
character in a line (indent level 0).
static struct foo *foo_bar(struct foo *first, struct bar *second,
struct foobar* thirsd);
4c. Breaking long lines
Descendants are always substantially shorter than the parent
and are placed substantially to the right.
Documentation/CodingStyle
Descendant must be indented at least to the level of the innermost
compound expression in the parent. All descendants at the same level
are indented the same.
if (foobar(.................................) + barbar * foobar(bar +
foo *
oof)) {
}
5. Blank lines
One blank line between functions.
void foo()
{
}
/* comment */
void bar()
{
}
No more than one blank line in a row.
Last (or first) line in a file is never blank.
Non-whitespace issues:
6. One-line statement does not need a {} block, so dont put it into one
if (foo)
bar;
7. Comments
Dont use C99 // comments.
8. Return codes
Functions that return success/failure status, should use 0 for success,
a negative value for failure.
Error codes are in linux/errno.h .
if (do_something()) {
handle_error();
return -EINVAL;
}
Functions that test a condition return 1 if condition is satisfied,
0 if its not.
if (is_condition())
condition_true();
9. Data types
Standard linux types are in linux/types.h .
See also Linux Device Drivers, Third Edition,
Chapter 11: Data Types in the Kernel. http://lwn.net/images/pdf/LDD3/
9a. Integer types
int is the default integer type.
Use unsigned type if you perform bit operations (<<,>>,&,|,~).
Use unsigned long if you have to fit a pointer into integer.
long long is at least 64 bit wide on all platforms.
char is for ASCII characters and strings.
Use u8,u16,u32,u64 if you need an integer of a specific size.
9b. typedef
Using typedefs to hide the data type is generally discouraged.
typedefs to function types are ok, since these can get very long.
typedef struct foo *(foo_bar_handler)(struct foo *first, struct bar *second,
struct foobar* thirsd);
Editor configuration:
9. The following is helpful with VIM
set cinoptions=(0:0
Michael S. Tsirkin
--
MST
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Sander
> Use tabs, not spaces, for indentation. Tabs should be 8
> characters wide.
A tab is a tab. The editor/viewer can be configured to show 2, 3, 4, 8,
any amount of characters, right?
Sander
--
Humilis IT Services and Solutions
http://www.humilis.net
Dmitry Torokhov
On 7/11/05, Michael S. Tsirkin <m...@mellanox.co.il> wrote:
> 3e. sizeof
> space after the operator
> sizeof a
If braces are used no spaces please : sizeof(struct foo)
>
> 4c. Breaking long lines
> Descendants are always substantially shorter than the parent
> and are placed substantially to the right.
> Documentation/CodingStyle
>
> Descendant must be indented at least to the level of the innermost
> compound expression in the parent. All descendants at the same level
> are indented the same.
> if (foobar(.................................) + barbar * foobar(bar +
> foo *
> oof)) {
> }
Ugh, that's as ugly as it can get... Something like below is much
easier to read...
if (foobar(.................................) +
barbar * foobar(bar + foo * oof)) {
}
--
Dmitry
Ingo Oeser
> > Descendant must be indented at least to the level of the innermost
> > compound expression in the parent. All descendants at the same level
> > are indented the same.
> > if (foobar(.................................) + barbar * foobar(bar +
> > foo *
> > oof)) {
> > }
>
> Ugh, that's as ugly as it can get... Something like below is much
> easier to read...
>
> if (foobar(.................................) +
> barbar * foobar(bar + foo * oof)) {
> }
if (foobar(.................................)
+ barbar * foobar(bar + foo * oof)) {
}
since a statement cannot start with binary operators
and as such we are SURE that there must have been something before.
And this matches with old shop owner calculations like:
1
+ 2
+ 3
----
6
which we all know since early math classes.
Regards
Ingo Oeser
Denis Vlasenko
> Michael S. Tsirkin wrote (ao):
> > Use tabs, not spaces, for indentation. Tabs should be 8
> > characters wide.
>
> A tab is a tab. The editor/viewer can be configured to show 2, 3, 4, 8,
> any amount of characters, right?
text with 8-char tabs:
struct s {
int n; /* comment */
unsigned int u; /* comment */
};
Same text viewed with tabs set to 4-char width:
struct s {
int n; /* comment */
unsigned int u; /* comment */
};
Comments are not aligned anymore
--
vda
Denis Vlasenko
> Leave space between name and * in types.
> Multiple * dont need additional space between them.
>
> struct foo **bar;
unless you declare a fuction:
int*
function_style_for_easy_grep(...)
{
...
}
I like this style because I can grep for ^function_style_for_easy_grep
and quickly find function def.
int
*function_style_for_easy_grep(...) ..
would make it harder.
> 3e. sizeof
> space after the operator
> sizeof a
I use sizeof(a) always (both for sizeof(type) and sizeof(expr)).
> 3i. if/else/do/while/for/switch
> space between if/else/do/while and following/preceeding
> statements/expressions, if any:
>
> if (a) {
> } else {
> }
>
> do {
> } while (b);
What's wrong with if(expr) ? Rationale?
> 4c. Breaking long lines
> Descendants are always substantially shorter than the parent
> and are placed substantially to the right.
> Documentation/CodingStyle
>
> Descendant must be indented at least to the level of the innermost
> compound expression in the parent. All descendants at the same level
> are indented the same.
> if (foobar(.................................) + barbar * foobar(bar +
> foo *
> oof)) {
> }
Avoid this. If needed, use a temporary. Save a few brain cells of poor reader.
> 6. One-line statement does not need a {} block, so dont put it into one
> if (foo)
> bar;
Disagree. Common case of hard-to-notice bug:
if(foo)
bar()
...after some time code evolves into:
if(foo)
/*
* Wee need to barify it, or else pagecache gets foobar'ed
*/
bar();
...after some more time:
if(foo)
/*
* Wee need to barify it, or else pagecache gets foobar'ed.
* Also we need to bazify it.
*/
bar();
baz();
Thus we may be better to slighty encourage use of {}s even if they are
not needed:
if(foo) {
bar();
}
> 9a. Integer types
> Use unsigned long if you have to fit a pointer into integer.
This is a porting nightmare waiting to happen.
Why dont we have ptr_t instead?
> long long is at least 64 bit wide on all platforms.
hugeint or hugeint_t
And also irqflags_t for spinlock_irqsave(&lock, flags),
jiffies_t for jiffies.
> 9b. typedef
> Using typedefs to hide the data type is generally discouraged.
> typedefs to function types are ok, since these can get very long.
>
> typedef struct foo *(foo_bar_handler)(struct foo *first, struct bar *second,
> struct foobar* thirsd);
? did you mean struct foo (*foo_bar_handler)(... or struct foo* foo_bar_handler(...
--
vda
Domen Puncer
> > 3c. * in types
> > Leave space between name and * in types.
> > Multiple * dont need additional space between them.
> >
> > struct foo **bar;
>
> unless you declare a fuction:
>
> int*
> function_style_for_easy_grep(...)
> {
> ...
> }
>
> I like this style because I can grep for ^function_style_for_easy_grep
> and quickly find function def.
>
That's a pretty bad argument, since most functions aren't declared
that way, and there are much better source code navigational tools,
like cscope and ctags.
Patrick McHardy
> text with 8-char tabs:
>
> struct s {
> int n; /* comment */
> unsigned int u; /* comment */
> };
>
> Same text viewed with tabs set to 4-char width:
>
> struct s {
> int n; /* comment */
> unsigned int u; /* comment */
> };
>
> Comments are not aligned anymore
Best rule IMO is to use tabs for indentation and spaces for alignment.
This way tab size can be changed without breaking alignment.
Richard B. Johnson
> Denis Vlasenko wrote:
>> text with 8-char tabs:
>>
>> struct s {
>> int n; /* comment */
>> unsigned int u; /* comment */
>> };
>>
>> Same text viewed with tabs set to 4-char width:
>>
>> struct s {
>> int n; /* comment */
>> unsigned int u; /* comment */
>> };
>>
>> Comments are not aligned anymore
>
> Best rule IMO is to use tabs for indentation and spaces for alignment.
> This way tab size can be changed without breaking alignment.
Or just disallow tabs altogether. At Analogic we had several hundred
thousand lines of imaging code from various engineers around the world.
They would set their tabs so everything looked fine on their computers.
On other computers, it looked like hell so engineers who had to
merge code spent hundreds of wasted hours lining up the code. The
"cleanup" work never stopped! That was until, we made a rule that
all code would be run through a filter that would remove tabs and
substitute spaces (of the width the writer intended). No code that
is released contains even a single tab anymode.
The files are larger of course, but even lap-tops have gigabyte
drives now-days and the duplicate spaces give compression utilities
something to do.
Cheers,
Dick Johnson
Penguin : Linux version 2.6.12 on an i686 machine (5537.79 BogoMips).
Notice : All mail here is now cached for review by Dictator Bush.
98.36% of all statistics are fiction.
Bodo Eggert
> text with 8-char tabs:
>
> struct s {
> int n; /* comment */
> unsigned int u; /* comment */
> };
>
> Same text viewed with tabs set to 4-char width:
>
> struct s {
> int n; /* comment */
> unsigned int u; /* comment */
> };
>
> Comments are not aligned anymore
That's why you SHOULD NOT use tabs for aligning, but for indenting.
--
Ich danke GMX dafür, die Verwendung meiner Adressen mittels per SPF
verbreiteten Lügen zu sabotieren.
Denis Vlasenko
> Denis Vlasenko <v...@ilport.com.ua> wrote:
>
> > text with 8-char tabs:
> >
> > struct s {
> > int n; /* comment */
> > unsigned int u; /* comment */
> > };
> >
> > Same text viewed with tabs set to 4-char width:
> >
> > struct s {
> > int n; /* comment */
> > unsigned int u; /* comment */
> > };
> >
> > Comments are not aligned anymore
>
> That's why you SHOULD NOT use tabs for aligning, but for indenting.
Doesn't work either: 8-char tabs:
int i; /* comment */
};
int j; /* comment */
4-char tabs:
int i; /* comment */
};
int j; /* comment */
So we can either ban tabs altogether (unlikely) or agree
that pi is ~= 3.1415926535897932384626433832795028841971693993751058209749
and tab is strictly 8 chars.
--
vda
Paul Jackson
A couple of possible additions:
* Keep lines within 80 columns.
* Be consistent in use of tabs versus spaces. If the
rest of a file is indented using tabs, then any change
you make should be indented the same way, not with
spaces. It is easy to unknowingly introduce spaces in
a patch by cutting and pasting something in one of the
many desktop UI environments that dont preserve tabs
across a cut and paste.
* See also Documentation/kernel-doc-nano-HOWTO.txt for
formatting the documentation for external functions
and data to work with the kernels DocBook automatically
extractable documentation.
* Multiline comments are shown as:
/*
* This is a big comment. It is full of sound
* and fury, signifying nothing. Now is the time
* for all good men to come to the aid of their
* country.
*/
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <p...@sgi.com> 1.925.600.0401
Paul Jackson
> Or just disallow tabs altogether. At Analogic we ...
This is the Linux kernel, not Analogic.
We use tabs for indentation. You can set the number
of physical spaces per tab however you want in your
editor, but it had better look good (and stay within
80 columns) when the rest of us use 8 spaces per tab.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <p...@sgi.com> 1.925.600.0401
Paul Jackson
> I don't think there's a strict 80 column rule anymore. It's 2005...
Look back through the lkml archives. One will find repeated requests
to keep code within 80 columns. It maybe 2005, but this is the kernel.
Lee Revell
> Dick Johnson wrote:
> > Or just disallow tabs altogether. At Analogic we ...
>
> This is the Linux kernel, not Analogic.
>
> We use tabs for indentation. You can set the number
> of physical spaces per tab however you want in your
> editor, but it had better look good (and stay within
> 80 columns)
I don't think there's a strict 80 column rule anymore. It's 2005...
Lee
Marc Singer
> On Tue, 2005-07-12 at 23:58 -0700, Paul Jackson wrote:
> > Dick Johnson wrote:
> > > Or just disallow tabs altogether. At Analogic we ...
> >
> > This is the Linux kernel, not Analogic.
> >
> > We use tabs for indentation. You can set the number
> > of physical spaces per tab however you want in your
> > editor, but it had better look good (and stay within
> > 80 columns)
>
> I don't think there's a strict 80 column rule anymore. It's 2005...
Think again. There are a lot of people who use 80 column windows so
that we can see two code windows side-by-side.
li...@horizon.com
> Think again. There are a lot of people who use 80 column windows so
> that we can see two code windows side-by-side.
Agreed. If you're having trouble with width, it's a sign that the code
needs to be refactored.
Also, my personal rule is if that a source file exceeds 1000 lines, start
looking for a way to split it. It can go longer (indeed, there is little
reason to split the fs/nls/nls_cp9??.c files), but
(I will refrain from discussing drivers/scsi/advansys.c)
Comments on the rest of the thread:
> 3a. Binary operators
> + - / * %
> == != > < >= <= && ||
> & | ^ << >>
> = *= /= %= += -= <<= >>= &= ^= |=
>
> spaces around the operator
> a + b
Generally, yes, and if you violate this, take the spaces out around the
tightest-binding operators first!
>> I like this style because I can grep for ^function_style_for_easy_grep
>> and quickly find function def.
> That's a pretty bad argument, since most functions aren't declared
> that way, and there are much better source code navigational tools,
> like cscope and ctags.
Well, I use that style for everything I write, for exactly that reason,
so it's fairly popular. Yes, there are lots of tools, but it's convenient
not to need them.
Also, definition argument lists can be a little longer than declaration
argument lists (due to the presence of argument names and possible
const qualifiers), so an extra place to break the line helps.
And it provides a place to put the handy GCC __attribute__(()) extensions...
static unsigned __attribute__((nonnull, pure))
is_condition_true(struct hairy *p, unsigned priority)
{
...
}
Finally, if you are burdened with long argument names, a shorter fixed prefix
makes it easier to align the arguments. To pick a real-world example:
static sctp_disposition_t sctp_sf_do_5_2_6_stale(const struct sctp_endpoint *ep,
const struct sctp_association *
asoc,
const sctp_subtype_t type,
void *arg,
sctp_cmd_seq_t *commands)
I prefer to write
static sctp_disposition_t
sctp_sf_do_5_2_6_stale(const struct sctp_endpoint *ep,
const struct sctp_association *asoc,
const sctp_subtype_t type,
void *arg,
sctp_cmd_seq_t *commands)
Although in extreme cases, it's usually best to just to:
static sctp_disposition_t
sctp_sf_do_5_2_6_stale_bug_workaround(
const struct sctp_endpoint *ep,
const struct sctp_association *asoc,
const sctp_subtype_t type,
void *arg,
sctp_cmd_seq_t *commands)
>> 3e. sizeof
>> space after the operator
>> sizeof a
> I use sizeof(a) always (both for sizeof(type) and sizeof(expr)).
You can, but I prefer not to. Still, it behaves a lot "like a function",
so it's not too wrong. In fact, I'll usually avoid the sizeof(type)
version entirely. It's often clearer to replace, e.g.
char buffer[sizeof(struct sctp_errhdr)+sizeof(union sctp_addr_param)];
with
char buffer[sizeof *errhdr + sizeof *addrparm];
which (if you look at the code in sctp_sf_send_restart_abort), actually
reflects what's going on better.
What really gets my goat is
return(0);
return *is not a function*. Stop making it look syntactically like one!
That should be written
return 0;
>> 3i. if/else/do/while/for/switch
>> space between if/else/do/while and following/preceeding
>> statements/expressions, if any:
>>
>> if (a) {
>> } else {
>> }
>>
>> do {
>> } while (b);
> What's wrong with if(expr) ? Rationale?
- It's less visually distinct from a function call.
- The space makes the keyword (important things, keywords) stand out more
and makes it easier to pick out of a mass of code.
- (Subjective) it balances the space in the trailing ") {" better.
This matches my personal style.
>> 6. One-line statement does not need a {} block, so dont put it into one
>> if (foo)
>> bar;
> Disagree. Common case of hard-to-notice bug:
>
> if(foo)
> bar()
>...after some time code evolves into:
> if(foo)
> /*
> * We need to barify it, or else pagecache gets FUBAR'ed
> */
> bar();
The braces should have been added then. They are okay to omit when the
body contains one physical line of text, but my rule is that a comment or
broken expression requires braces:
if (foo) {
/* We need to barify it, or else pagecache gets FUBAR'ed */
bar();
}
if (foo) {
bar(p->foo[hash(garply) % LARGEPRIME]->head,
flags & ~(FLAG_FOO | FLAG_BAR | FLAG_BAZ | FLAG_QUUX));
}
> Thus we may be better to slighty encourage use of {}s even if they are
> not needed:
>
> if(foo) {
> bar();
> }
It's not horrible to include them, but it reduces clutter sometimes to
leave them out.
>> if (foobar(.................................) + barbar * foobar(bar +
>> foo *
>> oof)) {
>> }
>
> Ugh, that's as ugly as it can get... Something like below is much
> easier to read...
>
> if (foobar(.................................) +
> barbar * foobar(bar + foo * oof)) {
> }
Strongly agreed! If you have to break an expression, do it at the lowest
precedence point possible!
> Even easier is
> if (foobar(.................................)
> + barbar * foobar(bar + foo * oof)) {
> }
>
> Since a statement cannot start with binary operators
> and as such we are SURE that there must have been something before.
I don't tend to do this, but I see the merit. However, C uses a number
of operators (+ - * &) in both unary and binary forms, so it's
not always unambiguous.
In such cases, I'll usually move the brace onto its own line to make the
end of the condition clearer:
if (foobar(.................................) +
barbar * foobar(bar + foo * oof))
{
}
Of course, better yet is to use a temporary or something to shrink
the condition down to a sane size, but sometimes you just need
if (messy_condition_one &&
messy_condition_two &&
messy_condition_three)
{
Kyle Moffett
>>> I don't think there's a strict 80 column rule anymore. It's 2005...
>
>> Think again. There are a lot of people who use 80 column windows so
>> that we can see two code windows side-by-side.
>
> Agreed. If you're having trouble with width, it's a sign that the
> code
> needs to be refactored.
>
> Also, my personal rule is if that a source file exceeds 1000 lines,
> start
> looking for a way to split it. It can go longer (indeed, there is
> little
> reason to split the fs/nls/nls_cp9??.c files), but
> (I will refrain from discussing drivers/scsi/advansys.c)
A simple set of code refactoring rules that I try to abide by:
1) If a function is more than a few 25 or 40 line screens, it's likely
too big (unless a big switch statement or a list of initialization calls
or something). If necessary, use static inline functions to factor out
repetitive behavior.
2) If a file is more than 30-40 functions, it's likely too big, and you
should try to split it. It's _ok_ to have 4 source files implementing
code for manipulating a single struct.
3) If a normal line of code is more than 80 characters, one of the
following is probably true: you need to break the line up and use temps
for clarity, or your function is so big that you're tabbing over too
far.
Cheers,
Kyle Moffett
-----BEGIN GEEK CODE BLOCK-----
Version: 3.12
GCM/CS/IT/U d- s++: a18 C++++>$ UB/L/X/*++++(+)>$ P+++(++++)>$ L++++(+
++) E
W++(+) N+++(++) o? K? w--- O? M++ V? PS+() PE+(-) Y+ PGP+++ t+(+++) 5
X R?
tv-(--) b++++(++) DI+ D+ G e->++++$ h!*()>++$ r !y?(-)
------END GEEK CODE BLOCK------
Jan Engelhardt
> 3) If a normal line of code is more than 80 characters, one of the
> following is probably true: you need to break the line up and use temps
> for clarity, or your function is so big that you're tabbing over too
> far.
(Find source files, expand tab chars to their on-screen length, print if
>= 80, count lines)
~/linux-2.6.12 >
find . -type f "(" -iname "*.c" -o -iname "*.h" -o -iname "*.S" ")" -print0 |
xargs -0 perl -pe '1 while s/\t+/" "x(length($&)*8-length($`)%8)/e' | \
perl -ne 'print if/.{80}/' | \
wc -l
208420
If the indent was just 4 spc wide, the number of extending
lines is just 131925.
Jan Engelhardt
--
Michael S. Tsirkin
> > static struct foo *foo_bar(struct foo *first, struct bar *second,
> > struct foobar* thirsd);
> >
> *first" vs "struct foobar* thirsd". Common practice is "struct foo
> *first".
Doh. Right. Thanks!
Jesper Juhl
>
> 3c. * in types
> Leave space between name and * in types.
> Multiple * dont need additional space between them.
>
> struct foo **bar;
>
even if it's not a double pointer. int * foo; is ugly. Common
convention is int *foo;
> 3e. sizeof
> space after the operator
> sizeof a
>
I don't think that's a hard rule, there's plenty of code that does
"sizeof(type)" and not "sizeof (type)", and whitespace cleanup
patches I've done that change "sizeof (type)" into "sizeof(type)" have
generally been accepted.
[snip]
>
> 4. Indentation rules for C
> Use tabs, not spaces, for indentation. Tabs should be 8 characters wide.
>
A tab is a tab is a tab, how it's displayed is up to the editor
showing the file.
[snip]
>
> static struct foo *foo_bar(struct foo *first, struct bar *second,
> struct foobar* thirsd);
>
In this example you are not consistently placing your *'s, "struct foo
*first" vs "struct foobar* thirsd". Common practice is "struct foo
*first".
[snip]
>
> No more than one blank line in a row.
> Last (or first) line in a file is never blank.
>
Files should end with a newline. gcc will even warn (with -pedantic)
if this is not so.
"line<nl>
line"
is wrong,
"line<nl>
line<nl>
"
is right.
> Non-whitespace issues:
>
> 6. One-line statement does not need a {} block, so dont put it into one
> if (foo)
> bar;
>
Not always so, if `bar' is a macro adding {} may be safer. Also
sometimes adding {} improves readability, which is important.
> 7. Comments
> Dont use C99 // comments.
>
s/Dont/Don't/
> 9a. Integer types
> int is the default integer type.
> Use unsigned type if you perform bit operations (<<,>>,&,|,~).
> Use unsigned long if you have to fit a pointer into integer.
> long long is at least 64 bit wide on all platforms.
> char is for ASCII characters and strings.
> Use u8,u16,u32,u64 if you need an integer of a specific size.
u8,s8,u16,s16,u32,s32,u64,s64
--
Jesper Juhl <jespe...@gmail.com>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
Bodo Eggert
>> 3) If a normal line of code is more than 80 characters, one of the
>> following is probably true: you need to break the line up and use temps
>> for clarity, or your function is so big that you're tabbing over too
>> far.
>
> (Find source files, expand tab chars to their on-screen length, print if
>>= 80, count lines)
>
> ~/linux-2.6.12 >
> find . -type f "(" -iname "*.c" -o -iname "*.h" -o -iname "*.S" ")"
... -exec expand -t 8 '{}' \; | egrep '^.{80}' | wc -l
233941
You didn't take \t[^\t]\t into account.
--
Ich danke GMX dafür, die Verwendung meiner Adressen mittels per SPF
verbreiteten Lügen zu sabotieren.
Paul Jackson
> > Leave space between name and * in types.
> > Multiple * dont need additional space between them.
> >
> > struct foo **bar;
> >
> Don't put spaces between `*' and the name when declaring variables,
> even if it's not a double pointer. int * foo; is ugly. Common
> convention is int *foo;
The "Leave space between name and *" is confusing. Does 'name'
refer to the type or variable. I hope Michael was not recommending:
char * p;
How about saying:
Do not leave a space between a * and the following
variable name, nor between multiple *, when used
to declare pointers:
char *p;
struct foo **bar;
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <p...@sgi.com> 1.925.600.0401
Krzysztof Halasa
> I don't think that's a hard rule, there's plenty of code that does
> "sizeof(type)" and not "sizeof (type)", and whitespace cleanup
> patches I've done that change "sizeof (type)" into "sizeof(type)" have
> generally been accepted.
Sure, the common rule is that function names and alike (including "sizeof")
are not followed by a space. Statements such as "if", "while" etc. - are.
--
Krzysztof Halasa
Paul Jackson
> (Find source files, expand tab chars to their on-screen length, print if
> >= 80, count lines)
The bulk of the longest lines are in the sound and drivers subtrees.
One example on the "high end", with 546 chars in one line:
==
drivers/scsi/BusLogic.c:
%2d %5d %5d %5d %5d %5d %5d %5d %5d %5d\n", TargetID, TargetStatistics[TargetID].CommandAbortsRequested, TargetStatistics[TargetID].CommandAbortsAttempted, TargetStatistics[TargetID].CommandAbortsCompleted, TargetStatistics[TargetID].BusDeviceResetsRequested, TargetStatistics[TargetID].BusDeviceResetsAttempted, TargetStatistics[TargetID].BusDeviceResetsCompleted, TargetStatistics[TargetID].HostAdapterResetsRequested, TargetStatistics[TargetID].HostAdapterResetsAttempted, TargetStatistics[TargetID].HostAdapterResetsCompleted);
==
Clearly, it would be unrepresentative of certain coding styles in drivers
and sound to claim they closely followed an 80 column constraint.
Perhaps the spacing guide should acknowledge this, with some qualification
such as:
The core kernel code (as apposed to some driver code) tends to keep
source line lengths below 80 columns. When changing such code, respect
the line length constraints of nearby code.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <p...@sgi.com> 1.925.600.0401
Horst von Brand
> On 7/11/05, Michael S. Tsirkin <m...@mellanox.co.il> wrote:
> [snip]
> > 3e. sizeof
> > space after the operator
> > sizeof a
> I don't think that's a hard rule, there's plenty of code that does
> "sizeof(type)" and not "sizeof (type)", and whitespace cleanup
> patches I've done that change "sizeof (type)" into "sizeof(type)" have
> generally been accepted.
It is necessary /not/ to put space between "function name" and '(', because
if it is a function-like macro, it does matter. For uniformity, do it for
functions and operations like sizeof too.
> [snip]
> >
> > 4. Indentation rules for C
> > Use tabs, not spaces, for indentation. Tabs should be 8 characters wide.
> A tab is a tab is a tab, how it's displayed is up to the editor
> showing the file.
But editing a file with tab==4 and editing it later with tab==8 guarantees
a mess.
[...]
> > 6. One-line statement does not need a {} block, so dont put it into one
> > if (foo)
> > bar;
> Not always so, if `bar' is a macro adding {} may be safer.
Such macros are /very/ dangerous. That's one reason why multi-line macros
must be wrapped in "do { ... } while(0)"
> Also
> sometimes adding {} improves readability, which is important.
Or leaves an old C hand wondering what is going on...
> > 7. Comments
> > Dont use C99 // comments.
> >
>
> s/Dont/Don't/
>
>
> > 9a. Integer types
> > int is the default integer type.
> > Use unsigned type if you perform bit operations (<<,>>,&,|,~).
> > Use unsigned long if you have to fit a pointer into integer.
> > long long is at least 64 bit wide on all platforms.
> > char is for ASCII characters and strings.
> > Use u8,u16,u32,u64 if you need an integer of a specific size.
>
> u8,s8,u16,s16,u32,s32,u64,s64
Plus annotations for bitwise and such. See Documentation/sparse.txt
--
Dr. Horst H. von Brand User #22616 counter.li.org
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria +56 32 654239
Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513
Jan Engelhardt
>> (Find source files, expand tab chars to their on-screen length, print if
>> >= 80, count lines)
>
>The bulk of the longest lines are in the sound and drivers subtrees.
>One example on the "high end", with 546 chars in one line:
>
>==
>drivers/scsi/BusLogic.c:
> %2d %5d %5d %5d %5d %5d %5d %5d %5d %5d\n", TargetID, TargetStatistics[TargetID].CommandAbortsRequested, TargetStatistics[TargetID].CommandAbortsAttempted, TargetStatistics[TargetID].CommandAbortsCompleted, TargetStatistics[TargetID].BusDeviceResetsRequested, TargetStatistics[TargetID].BusDeviceResetsAttempted, TargetStatistics[TargetID].BusDeviceResetsCompleted, TargetStatistics[TargetID].HostAdapterResetsRequested, TargetStatistics[TargetID].HostAdapterResetsAttempted, TargetStatistics[TargetID].HostAdapterResetsCompleted);
>==
this is omg.
- the VLN BASIC way (very long variable names)
- it could have been splitted at the next possible space at 80, i.e.
mostly after a comma (my mail _reader_ automatically wrapped it, so it
looked rather ok - until I took an _editor_)
If I add a temporary (as suggested by rule 3)
(struct BusLogic_Statistics *tp = &TargetStatistics[TargetID]), the line
length loses a lot of weight: 339 chars.
> %2d %5d %5d %5d %5d %5d %5d %5d %5d %5d\n", TargetID, TargetStatistics[TargetID].CommandAbortsRequested, TargetStatistics[TargetID].CommandAbortsAttempted, TargetStatistics[TargetID].CommandAbortsCompleted, TargetStatistics[TargetID].BusDeviceResetsRequested, TargetStatistics[TargetID].BusDeviceResetsAttempted, TargetStatistics[TargetID].BusDeviceResetsCompleted, TargetStatistics[TargetID].HostAdapterResetsRequested, TargetStatistics[TargetID].HostAdapterResetsAttempted, TargetStatistics[TargetID].HostAdapterResetsCompleted);
Jan Engelhardt
--
Kyle Moffett
> drivers/scsi/BusLogic.c:
>
> %2d %5d %5d %5d %5d %5d %5d %5d %5d %5d\n",
> TargetID, TargetStatistics[TargetID].CommandAbortsRequested,
> TargetStatistics[TargetID].CommandAbortsAttempted, TargetStatistics
> [TargetID].CommandAbortsCompleted, TargetStatistics
> [TargetID].BusDeviceResetsRequested, TargetStatistics
> [TargetID].BusDeviceResetsAttempted, TargetStatistics
> [TargetID].BusDeviceResetsCompleted, TargetStatistics
> [TargetID].HostAdapterResetsRequested, TargetStatistics
> [TargetID].HostAdapterResetsAttempted, TargetStatistics
> [TargetID].HostAdapterResetsCompleted);
Ugh!!! From CodingStyle (although this is not always followed):
> The limit on the length of lines is 80 columns and this is a hard
> limit.
> Statements longer than 80 columns will be broken into sensible chunks.
> Descendants are always substantially shorter than the parent and
> are placed
> substantially to the right. The same applies to function headers
> with a long
> argument list. Long strings are as well broken into shorter strings.
> [example relevant to the above code snipped]
Also:
> C is a Spartan language, and so should your naming be. Unlike
> Modula-2 and
> Pascal programmers, C programmers do not use cute names like
> ThisVariableIsATemporaryCounter. A C programmer would call that
> variable
> "tmp", which is much easier to write, and not the least more
> difficult to
> understand.
>
> [...] mixed-case names are frowned upon [...]
*cough* TargetStatistics[TargetID].HostAdapterResetsCompleted *cough*
I suspect linus would be willing to accept a few cleanup patches for the
BusLogic.c file. Perhaps even one that renames BusLogic.c to buslogic.c
like all the other files in the source tree, instead of using nasty
StudlyCaps all over :-D
Cheers,
Kyle Moffett
-----BEGIN GEEK CODE BLOCK-----
Version: 3.12
GCM/CS/IT/U d- s++: a18 C++++>$ UB/L/X/*++++(+)>$ P+++(++++)>$ L++++(+
++) E
W++(+) N+++(++) o? K? w--- O? M++ V? PS+() PE+(-) Y+ PGP+++ t+(+++) 5
X R?
tv-(--) b++++(++) DI+ D+ G e->++++$ h!*()>++$ r !y?(-)
------END GEEK CODE BLOCK------
Jesper Juhl
> On Jul 20, 2005, at 20:45:21, Paul Jackson wrote:
> *cough* TargetStatistics[TargetID].HostAdapterResetsCompleted *cough*
>
> I suspect linus would be willing to accept a few cleanup patches for the
> BusLogic.c file. Perhaps even one that renames BusLogic.c to buslogic.c
> like all the other files in the source tree, instead of using nasty
> StudlyCaps all over :-D
>
To avoid people doing duplicate work, I just want to say that I've
started doing a CodingStyle/whitespace/VariableAndFunctionNaming
cleanup of the BusLogic driver, I'll send the patches to LKML in a few
hours.
--
Jesper Juhl <jespe...@gmail.com>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
linux-os (Dick Johnson)
On Thu, 21 Jul 2005, Jesper Juhl wrote:
> On 7/21/05, Kyle Moffett <mrmac...@mac.com> wrote:
>> On Jul 20, 2005, at 20:45:21, Paul Jackson wrote:
> [...snip...]
>> *cough* TargetStatistics[TargetID].HostAdapterResetsCompleted *cough*
>>
>> I suspect linus would be willing to accept a few cleanup patches for the
>> BusLogic.c file. Perhaps even one that renames BusLogic.c to buslogic.c
>> like all the other files in the source tree, instead of using nasty
>> StudlyCaps all over :-D
>>
>
> To avoid people doing duplicate work, I just want to say that I've
> started doing a CodingStyle/whitespace/VariableAndFunctionNaming
> cleanup of the BusLogic driver, I'll send the patches to LKML in a few
> hours.
>
Are you going to get rid of the BusLogic* in front of every variable
and function name? (yes please??) If so, you will need a few days!
It will take probably an hour to parse:
struct BusLogic_FetchHostAdapterLocalRAMReguest FetchHostAdapterLocalRAMRequest
^!)
> --
> Jesper Juhl <jespe...@gmail.com>
> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
> Plain text mails only, please http://www.expita.com/nomime.html
Cheers,
Dick Johnson
Penguin : Linux version 2.6.12 on an i686 machine (5537.79 BogoMips).
Notice : All mail here is now cached for review by Dictator Bush.
98.36% of all statistics are fiction.
****************************************************************
The information transmitted in this message is confidential and may be privileged. Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited. If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to Deliver...@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.
Thank you.
Jesper Juhl
>
> On Thu, 21 Jul 2005, Jesper Juhl wrote:
>
> > On 7/21/05, Kyle Moffett <mrmac...@mac.com> wrote:
> >> On Jul 20, 2005, at 20:45:21, Paul Jackson wrote:
> > [...snip...]
> >> *cough* TargetStatistics[TargetID].HostAdapterResetsCompleted *cough*
> >>
> >> I suspect linus would be willing to accept a few cleanup patches for the
> >> BusLogic.c file. Perhaps even one that renames BusLogic.c to buslogic.c
> >> like all the other files in the source tree, instead of using nasty
> >> StudlyCaps all over :-D
> >>
> >
> > To avoid people doing duplicate work, I just want to say that I've
> > started doing a CodingStyle/whitespace/VariableAndFunctionNaming
> > cleanup of the BusLogic driver, I'll send the patches to LKML in a few
> > hours.
> >
> Are you going to get rid of the BusLogic* in front of every variable
> and function name? (yes please??)
> If so, you will need a few days!
That may be, it sure turned into a bigger job than I had at first
expected. I'll break it into a few logical bits and submit them along
the way. First bits in a few hours - let's see how far I get :)
> It will take probably an hour to parse:
> struct BusLogic_FetchHostAdapterLocalRAMReguest
Yeah, it takes time, but I'll get it done.
>
> Thank you.
>
np.
--
Jesper Juhl <jespe...@gmail.com>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
Jesper Juhl
~300Kb of patches here already, and I'm only about 30% done
(estimated).
It makes little sense to post the patches I have at this time, since
they don't really finish the job and leave the files in a funky
intermediate state, so I'll hold off on posting them untill I'm a
little closer to the goal - hopefully tomorrow I'll finish it (right
now I need to get some sleep) - I'll post the patches as soon as I'm
done with them...
Miles Bader
> It will take probably an hour to parse:
> struct BusLogic_FetchHostAdapterLocalRAMReguest FetchHostAdapterLocalRAMRequest
> ^!)
Agh! My eyes!
The above names are way overdone by any measure, but is there any
consensus whether studly-caps in general are discouraged or not?
CodingStyle is vague on the issue, though it kind of implies you should
use underscores when multiple words are needed (the sole example of
studly caps is in a negative context, and a following recommended name
uses underlines). The kernel source seems pretty random -- they get
used here and there, but more often not; they seem more common in older
code.
If they are discouraged, it might be better to say so explicitly, as
there are many programmers these days who are used to using them.
-Miles
--
Run away! Run away!
Paul Jackson
> CodingStyle is vague on the issue, though ...
Perhaps we should call it "coding_style" <grin>.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <p...@sgi.com> 1.925.600.0401
Patrick Draper
as they like to write it, then format it automatically in a standard
way before it gets put into the kernel.
Patrick
Sam Ravnborg
> Why isn't a code formatting program used? People could write the code
> as they like to write it, then format it automatically in a standard
> way before it gets put into the kernel.
scripts/Lindent
But sometimes it fails to do the job properly and some hand formatting
is needed. Also much of the kernel is not new stuff but expanding or
fixing old stuff.
Sam
Jesper Juhl
> Why isn't a code formatting program used?
There's scripts/Lindent (which is just a wrapper around indent with
appropriate options.
>People could write the code
> as they like to write it, then format it automatically in a standard
> way before it gets put into the kernel.
>
You can do some cleanup that way, but not everything, and then if
people continue to write in their own FunkyStyle locally but what's in
the kernel has been beautified they are going to have a hell of a time
creating proper patches... It should just be cleaned up by whoever
submits it before it gets merged (and if you want to let a tool help
you out some of the way, then noone's stopping you).
--
Jesper Juhl <jespe...@gmail.com>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
Jesper Juhl
> On Fri, Jul 22, 2005 at 12:12:12PM -0500, Patrick Draper wrote:
> > Why isn't a code formatting program used? People could write the code
> > as they like to write it, then format it automatically in a standard
> > way before it gets put into the kernel.
> There is.
> scripts/Lindent
>
> But sometimes it fails to do the job properly and some hand formatting
> is needed. Also much of the kernel is not new stuff but expanding or
> fixing old stuff.
>
but not everything..."
Jesper
Jesper Juhl
need one more day to finish this.