Re: kernel guide to space (updated)
Jan Engelhardt <[EMAIL PROTECTED]> writes: >>3i. if/else/do/while/for/switch >> space between if/else/do/while and following/preceeding >> statements/expressions, if any > > Why this? if(a) {} is not any worse than if (a). Well it's harder to read (because it makes control constructs look more like function calls). And a bit uglier. But anyway, coding styles are rarely about "worse" versus "better", they're about keeping things consistent so that it's easier to read code. -Miles -- Any man who is a triangle, has thee right, when in Cartesian Space, to have angles, which when summed, come to know more, nor no less, than nine score degrees, should he so wish. [TEMPLE OV THEE LEMUR] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space (updated)
>> >Ehrm, yes, I'm perfectly aware of that. Note the "for consistency" in >> >that sentence. If we add an extra space in front of the labels that >> >have an indentation level of 0, we'd better do it with the labels that >> >have an indentation level > 0 too. >> >> Labels at level > 0??? > >A case in a switch construct is a label. Oh hm. I only meant "goto" labels. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space (updated)
On Sat, Jul 30, 2005 at 01:41:55PM +0200, Jan Engelhardt wrote: > > >Ehrm, yes, I'm perfectly aware of that. Note the "for consistency" in > >that sentence. If we add an extra space in front of the labels that > >have an indentation level of 0, we'd better do it with the labels that > >have an indentation level > 0 too. > > Labels at level > 0??? A case in a switch construct is a label. Regards: David Weinehall -- /) David Weinehall <[EMAIL PROTECTED]> /) Northern lights wander (\ // Maintainer of the v2.0 kernel // Dance across the winter sky // \) http://www.acc.umu.se/~tao/(/ Full colour fire (/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space (updated)
>Ehrm, yes, I'm perfectly aware of that. Note the "for consistency" in >that sentence. If we add an extra space in front of the labels that >have an indentation level of 0, we'd better do it with the labels that >have an indentation level > 0 too. Labels at level > 0??? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space (updated)
On Fri, Jul 29, 2005 at 09:40:14AM +0200, Jan Engelhardt wrote: > >3i. if/else/do/while/for/switch > > space between if/else/do/while and following/preceeding > > statements/expressions, if any > > Why this? if(a) {} is not any worse than if (a). I would make this an option. please no, it's really better to have a unified CodingStyle. and "if (" is a lot more used than "if(". $ grep -r "if (" linux-2.6.12/* | wc -l 288027 $ grep -r "if(" linux-2.6.12/* | wc -l 20682 -- Vincent Hanquez - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space (updated)
On Fri, Jul 29, 2005 at 09:52:01PM +0200, Jan Engelhardt wrote: > > >I'd definitely call that a joe-bug. Adding extra space for no good > >reason is just silly; for consistency we'd have to add a space for the > >case : labels also... > > No, the word "case" never starts at column 1 (counting from 1), but at least > in column , where minimalIndent is the default indent for that > particular piece of code. Ehrm, yes, I'm perfectly aware of that. Note the "for consistency" in that sentence. If we add an extra space in front of the labels that have an indentation level of 0, we'd better do it with the labels that have an indentation level > 0 too. Regards: David -- /) David Weinehall <[EMAIL PROTECTED]> /) Northern lights wander (\ // Maintainer of the v2.0 kernel // Dance across the winter sky // \) http://www.acc.umu.se/~tao/(/ Full colour fire (/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space (updated)
>I'd definitely call that a joe-bug. Adding extra space for no good >reason is just silly; for consistency we'd have to add a space for the >case : labels also... No, the word "case" never starts at column 1 (counting from 1), but at least in column , where minimalIndent is the default indent for that particular piece of code. Jan Engelhardt -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space (updated)
On Fri, Jul 29, 2005 at 09:40:14AM +0200, Jan Engelhardt wrote: [snip] > Would it be reasonable to say that the first column can be a space? > Some editors (e.g. joe) list the function in some status bar and do > that based on the fact that all C code in a function is indented, and > only the function header is non-indented. Putting a label statement > fools the algorithm. joe-bug or option for freedom? I'd definitely call that a joe-bug. Adding extra space for no good reason is just silly; for consistency we'd have to add a space for the case : labels also... Regards: David -- /) David Weinehall <[EMAIL PROTECTED]> /) Northern lights wander (\ // Maintainer of the v2.0 kernel // Dance across the winter sky // \) http://www.acc.umu.se/~tao/(/ Full colour fire (/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space (updated)
Jan Engelhardt wrote: >>3e. sizeof >> space after the operator >> no space if the operand is in barces >> >> > >braces > > > >>3f. Braces etc >> () [] -> . >> >> > >() parentheses (short form: parens) >[] square brackets >{} braces ><> dunno their name :p > angle brackets, it's all nicely explained at http://en.wikipedia.org/wiki/Bracket -- Joost smime.p7s Description: S/MIME Cryptographic Signature
Re: kernel guide to space (updated)
>3e. sizeof > space after the operator > no space if the operand is in barces braces >3f. Braces etc > () [] -> . () parentheses (short form: parens) [] square brackets {} braces <> dunno their name :p >3i. if/else/do/while/for/switch > space between if/else/do/while and following/preceeding > statements/expressions, if any Why this? if(a) {} is not any worse than if (a). I would make this an option. >3j. return > space between return and following expression, > even if the operand is in barces parentheses > return (a); No unnecessary parentheses: return (3 + 7) * 5; return 1; instead of return ((3 + 7) * 5); return (1); >3k. 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(); >} Would it be reasonable to say that the first column can be a space? Some editors (e.g. joe) list the function in some status bar and do that based on the fact that all C code in a function is indented, and only the function header is non-indented. Putting a label statement fools the algorithm. joe-bug or option for freedom? >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; > } switch(foo) { default: { int somevar = dosomething; break; } } What now? You've got two }} after another. Jan Engelhardt -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space (updated)
>> 9. The following is helpful with VIM >>set cinoptions=(0:0 >> > >And this will highlight whitespace damage: > >highlight RedundantSpaces ctermbg=red guibg=red >match RedundantSpaces /\s\+$\| \+\ze\t/ find linux -type f -print0 | xargs -0 egrep '[[:space:]]+$' With `wc -l`, returns 132409* (wow, that's a lot of violations). Too bad that (e)grep does not support \s for space. * Number might vary. Jan Engelhardt -- | Alphagate Systems, http://alphagate.hopto.org/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space (updated)
On Thu, Jul 28, 2005 at 11:52:25AM -0400, linux-os (Dick Johnson) wrote: > > On Thu, 28 Jul 2005, Michael S. Tsirkin wrote: > > > > > 7. Comments > > Don't use C99 // comments. > > I don't think this is correct. In fact, I remember a discussion > where // was preferred for new code. Ick...I hope not... John -- John W. Linville [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space (updated)
On 7/28/05, Michael S. Tsirkin <[EMAIL PROTECTED]> wrote: > > 9. The following is helpful with VIM >set cinoptions=(0:0 > And this will highlight whitespace damage: highlight RedundantSpaces ctermbg=red guibg=red match RedundantSpaces /\s\+$\| \+\ze\t/ -- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space (updated)
On Thu, 28 Jul 2005, Michael S. Tsirkin wrote: > > 7. Comments > Don't use C99 // comments. I don't think this is correct. In fact, I remember a discussion where // was preferred for new code. Cheers, Dick Johnson Penguin : Linux version 2.6.12 on an i686 machine (5537.79 BogoMips). Warning : 98.36% of all statistics are fiction. . I apologize for the following. I tried to kill it with the above dot : 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 [EMAIL PROTECTED] - and destroy all copies of this information, including any attachments, without reading or disclosing them. Thank you. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
kernel guide to space (updated)
> 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. Here's an updated version of the boring list. My thanks to everyone who commented on the first draft. --- 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 space between type name and * multiple * dont need additional space between them no space between * and its operand struct foo **bar; 3d. Conditional ?: spaces around both ? and : a ? b : c 3e. sizeof space after the operator no space if the operand is in barces sizeof *a sizeof(b) 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 ; space after semicolon (e.g. in a for loop) no space before semicolon for (i = 0; i < 10; ++i) 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. return space between return and following expression, even if the operand is in barces return (a); 3k. 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 are 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(..) + raboof() * barfoo(..)) { } 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. Files end with a newline. gcc warns if this is not so. Non-whitespace issues: 6. One-line statement does not need a {} block, so dont put it into one if (foo) bar; 7. Comments Don't 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_
Re: kernel guide to space
On 7/22/05, Jesper Juhl <[EMAIL PROTECTED]> wrote: > On 7/21/05, Jesper Juhl <[EMAIL PROTECTED]> wrote: > > On 7/21/05, linux-os (Dick Johnson) <[EMAIL PROTECTED]> wrote: > > > > > > On Thu, 21 Jul 2005, Jesper Juhl wrote: > > > > > > > On 7/21/05, Kyle Moffett <[EMAIL PROTECTED]> 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??) > > Yes, I am. > > > > > 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. > > > Heh, it takes a little more time than I had anticipated. I've got > ~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... > Still moving forward with this, but not yet done. I think I'm going to need one more day to finish this. -- Jesper Juhl <[EMAIL PROTECTED]> 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
On 7/22/05, Sam Ravnborg <[EMAIL PROTECTED]> wrote: > 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. > Ehh, that's exactely why I wrote "You can do some cleanup that way, but not everything..." Jesper - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
On 7/22/05, Patrick Draper <[EMAIL PROTECTED]> wrote: > 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 <[EMAIL PROTECTED]> 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
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. Sam - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
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. Patrick - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
Miles wrote: > CodingStyle is vague on the issue, though ... Perhaps we should call it "coding_style" . -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <[EMAIL PROTECTED]> 1.925.600.0401 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
"linux-os \(Dick Johnson\)" <[EMAIL PROTECTED]> writes: > 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! - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
On 7/21/05, Jesper Juhl <[EMAIL PROTECTED]> wrote: > On 7/21/05, linux-os (Dick Johnson) <[EMAIL PROTECTED]> wrote: > > > > On Thu, 21 Jul 2005, Jesper Juhl wrote: > > > > > On 7/21/05, Kyle Moffett <[EMAIL PROTECTED]> 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??) > Yes, I am. > > > 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. > Heh, it takes a little more time than I had anticipated. I've got ~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... -- Jesper Juhl <[EMAIL PROTECTED]> 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
On 7/21/05, linux-os (Dick Johnson) <[EMAIL PROTECTED]> wrote: > > On Thu, 21 Jul 2005, Jesper Juhl wrote: > > > On 7/21/05, Kyle Moffett <[EMAIL PROTECTED]> 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??) Yes, I am. > 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 <[EMAIL PROTECTED]> 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
On Thu, 21 Jul 2005, Jesper Juhl wrote: > On 7/21/05, Kyle Moffett <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> > 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 [EMAIL PROTECTED] - and destroy all copies of this information, including any attachments, without reading or disclosing them. Thank you. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
On 7/21/05, Kyle Moffett <[EMAIL PROTECTED]> 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. -- Jesper Juhl <[EMAIL PROTECTED]> 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
On Jul 20, 2005, at 20:45:21, Paul Jackson wrote: 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-- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
>> (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 -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
Jesper Juhl <[EMAIL PROTECTED]> wrote: > On 7/11/05, Michael S. Tsirkin <[EMAIL PROTECTED]> 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, ChileFax: +56 32 797513 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
Jan wrote: > (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 <[EMAIL PROTECTED]> 1.925.600.0401 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
Jesper Juhl <[EMAIL PROTECTED]> writes: > 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
> > 3c. * in types > > 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 <[EMAIL PROTECTED]> 1.925.600.0401 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
Jan Engelhardt <[EMAIL PROTECTED]> wrote: >> 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. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
Quoting r. Jesper Juhl <[EMAIL PROTECTED]>: > > 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". Doh. Right. Thanks! -- MST - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
On 7/11/05, Michael S. Tsirkin <[EMAIL PROTECTED]> wrote: > [snip] > kernel guide to space AKA a boring list of rules > http://www.mellanox.com/mst/boring.txt > [snip] > > 3c. * in types > 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; > 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 line" is wrong, "line line " 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 <[EMAIL PROTECTED]> 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
> 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 -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
On Jul 13, 2005, at 21:12:08, [EMAIL PROTECTED] wrote: 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-- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
>> 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) 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
Re: kernel guide to space
On Wed, Jul 13, 2005 at 01:22:04PM -0400, Lee Revell wrote: > 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. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
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... Lee - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
Lee wrote: > 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. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <[EMAIL PROTECTED]> 1.925.600.0401 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
Nice work - thanks. 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 <[EMAIL PROTECTED]> 1.925.600.0401 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
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) when the rest of us use 8 spaces per tab. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <[EMAIL PROTECTED]> 1.925.600.0401 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
On Tuesday 12 July 2005 22:36, Bodo Eggert wrote: > Denis Vlasenko <[EMAIL PROTECTED]> 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
Denis Vlasenko <[EMAIL PROTECTED]> 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. -- Ich danke GMX dafür, die Verwendung meiner Adressen mittels per SPF verbreiteten Lügen zu sabotieren. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
On Tue, 12 Jul 2005, Patrick McHardy wrote: 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. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
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. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
On 12/07/05 10:12 +0300, Denis Vlasenko wrote: > > 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. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
> 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. 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
On Monday 11 July 2005 18:34, Sander wrote: > 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
On Monday 11 July 2005 17:44, Dmitry Torokhov wrote: > >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)) { > } 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. And this matches with old shop owner calculations like: 1 + 2 + 3 6 which we all know since early math classes. Regards Ingo Oeser pgpC5TxreXsJl.pgp Description: PGP signature
Re: kernel guide to space
Hi, On 7/11/05, Michael S. Tsirkin <[EMAIL PROTECTED]> 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel guide to space
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? Sander -- Humilis IT Services and Solutions http://www.humilis.net - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
kernel guide to space
Hi! 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.