* [PATCH] 0/3 coding standards documentation/code updates
@ 2007-09-28 21:31 Erez Zadok
2007-09-28 21:32 ` [PATCH 1/3] CodingStyle updates Erez Zadok
` (3 more replies)
0 siblings, 4 replies; 30+ messages in thread
From: Erez Zadok @ 2007-09-28 21:31 UTC (permalink / raw)
To: torvalds, akpm; +Cc: linux-kernel
The following is a series of patches related to coding standards. The first
patch updates the CodingStandards document with respect to printk debugging
and un/likely use; the second patch updates the usage string for
checkpatch.pl; the third patch introduces a new small perl script that can
be used to check coding-standards compliance on multiple source files. This
new script, called check-coding-standards.pl, produces output in a standard
compiler-error format, which can be parsed by assorted tools including text
editors and help users locate the file that had the error and the offending
line-number more quickly.
Erez Zadok (3):
CodingStyle updates
Update usage string for checkpatch.pl
New script to check coding-style compliance on multiple regular files
Documentation/CodingStyle | 88 +++++++++++++++++++++++++++++++++++++-
scripts/check-coding-standards.pl | 59 +++++++++++++++++++++++++
scripts/checkpatch.pl | 1
3 files changed, 146 insertions(+), 2 deletions(-)
---
Erez Zadok
ezk@cs.sunysb.edu
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 1/3] CodingStyle updates 2007-09-28 21:31 [PATCH] 0/3 coding standards documentation/code updates Erez Zadok @ 2007-09-28 21:32 ` Erez Zadok 2007-09-28 21:46 ` Randy Dunlap ` (3 more replies) 2007-09-28 21:32 ` [PATCH 2/3] Update usage string for checkpatch.pl Erez Zadok ` (2 subsequent siblings) 3 siblings, 4 replies; 30+ messages in thread From: Erez Zadok @ 2007-09-28 21:32 UTC (permalink / raw) To: torvalds, akpm Cc: linux-kernel, Kok, Auke, Kyle Moffett, Jan Engelhardt, Adrian Bunk, roel, Erez Zadok, Kyle Moffett, Jan Engelhardt, Adrian Bunk, roel 1. Updates chapter 13 (printing kernel messages) to expand on the use of pr_debug()/pr_info(), what to avoid, and how to hook your debug code with kernel.h. 2. New chapter 19, branch prediction optimizations, discusses the whole un/likely issue. Cc: "Kok, Auke" <auke-jan.h.kok@intel.com> Cc: Kyle Moffett <mrmacman_g4@mac.com> Cc: Jan Engelhardt <jengelh@computergmbh.de> Cc: Adrian Bunk <bunk@kernel.org> Cc: roel <12o3l@tiscali.nl> Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu> --- Documentation/CodingStyle | 88 +++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 86 insertions(+), 2 deletions(-) diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle index 7f1730f..00b29e4 100644 --- a/Documentation/CodingStyle +++ b/Documentation/CodingStyle @@ -643,8 +643,26 @@ Printing numbers in parentheses (%d) adds no value and should be avoided. There are a number of driver model diagnostic macros in <linux/device.h> which you should use to make sure messages are matched to the right device and driver, and are tagged with the right level: dev_err(), dev_warn(), -dev_info(), and so forth. For messages that aren't associated with a -particular device, <linux/kernel.h> defines pr_debug() and pr_info(). +dev_info(), and so forth. + +A number of people often like to define their own debugging printf's, +wrapping printk's in #ifdef's that get turned on only when subsystem +debugging is compiled in (e.g., dprintk, Dprintk, DPRINTK, etc.). Please +don't reinvent the wheel but use existing mechanisms. For messages that +aren't associated with a particular device, <linux/kernel.h> defines +pr_debug() and pr_info(); the latter two translate to printk(KERN_DEBUG) and +printk(KERN_INFO), respectively. However, to get pr_debug() to actually +emit the message, you'll need to turn on DEBUG in your code, which can be +done as follows in your subsystem Makefile: + +ifeq ($(CONFIG_WHATEVER_DEBUG),y) +EXTRA_CFLAGS += -DDEBUG +endif + +In this way, you can create a Kconfig parameter to turn on debugging at +compile time, which will also turn on DEBUG, to enable pr_debug() to emit +actual messages; conversely, when CONFIG_WHATEVER_DEBUG is off, DEBUG is +off, and pr_debug() will display nothing. Coming up with good debugging messages can be quite a challenge; and once you have them, they can be a huge help for remote troubleshooting. Such @@ -779,6 +797,69 @@ includes markers for indentation and mode configuration. People may use their own custom mode, or may have some other magic method for making indentation work correctly. + Chapter 19: branch prediction optimizations + +The kernel includes macros called likely() and unlikely(), which can be used +as hints to the compiler to optimize branch prediction. They operate by +asking gcc to shuffle the code around so that the more favorable outcome +executes linearly, avoiding a JMP instruction; this can improve cache +pipeline efficiency. For technical details how these macros work, see the +References section at the end of this document. + +An example use of this as as follows: + + ptr = kmalloc(size, GFP_KERNEL); + if (unlikely(!ptr)) + ... + +or + err = some_function(...); + if (likely(!err)) + ... + +The main two problems with using un/likely() are that (a) programmers can +easily be wrong about their code's likelihood to take one branch +vs. another, and (b) on average, gcc will do a much better job optimizing +branches that the programmer can. The benefit on some systems for +predicting correctly can be in saving a few instructions. But the penalty +for wrong use of un/likely() can be very significant (possibly dozens of +instructions), as you may be doing a JMP instruction, hurting your pipeline +cache, EACH time you get to the branch in question! + +Therefore, use un/likely() sparingly, consider it primarily for hot paths, +use it only when you are certain that the condition in question rarely +happens, be sure that it happens with roughly the same probability under +most/all user conditions. One rule of thumb suggested is that the +probability of the branch un/taken should exceed 99% (although some would +consider 95% as well). Of course, beware of silly mistakes such as +intending to use likely() and using unlikely() instead. + +A good example of this is the above kmalloc(GFP_KERNEL) call. The chances +of kmalloc() returning NULL are rather small, because even if it doesn't +have memory to return to you at the moment, with GFP_KERNEL/__GFP_WAIT +passed, kmalloc() will wait and suspend your thread, while it goes off to +find some free memory (purging caches, flushing buffers, etc.). In other +words, kmalloc() tries very hard to give you the memory you asked for by the +time it return. + +Consider the next, bad example. Suppose you're developing a file system +which performs logically different actions on different types of entities: +files, directories, symlinks, devices, etc. and you use this code: + + if (unlikely(S_ISBLK(mode)) + ... + +On first glance, the above use of unlikely() seems right. After all, most +file system objects are files and directories, and very few of them tend to +be block devices. So this optimization should work well, no? Although it's +true that it'll work well for most users, what about some user who happens +to have a file system with lots of block devices? Or what if the user has +only one block device object on the file system, but the user has an +application which causes the above conditional to be traversed very +frequently (e.g., a shell script that deals with devices)? Such users will +be penalized heavily for going [sic] down the wrong path... Therefore, you +should consider also whether a seemingly-rare condition is indeed rare ALL +the time. Appendix I: References @@ -804,6 +885,9 @@ language C, URL: http://www.open-std.org/JTC1/SC22/WG14/ Kernel CodingStyle, by greg@kroah.com at OLS 2002: http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_talk/html/ +FAQ/LikelyUnlikely: +http://kernelnewbies.org/FAQ/LikelyUnlikely + -- Last updated on 2007-July-13. -- 1.5.2.2 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] CodingStyle updates 2007-09-28 21:32 ` [PATCH 1/3] CodingStyle updates Erez Zadok @ 2007-09-28 21:46 ` Randy Dunlap 2007-09-29 14:43 ` Shawn Bohrer ` (2 subsequent siblings) 3 siblings, 0 replies; 30+ messages in thread From: Randy Dunlap @ 2007-09-28 21:46 UTC (permalink / raw) To: Erez Zadok Cc: torvalds, akpm, linux-kernel, Kok, Auke, Kyle Moffett, Jan Engelhardt, Adrian Bunk, roel On Fri, 28 Sep 2007 17:32:00 -0400 Erez Zadok wrote: > 1. Updates chapter 13 (printing kernel messages) to expand on the use of > pr_debug()/pr_info(), what to avoid, and how to hook your debug code with > kernel.h. > > 2. New chapter 19, branch prediction optimizations, discusses the whole > un/likely issue. > > Cc: "Kok, Auke" <auke-jan.h.kok@intel.com> > Cc: Kyle Moffett <mrmacman_g4@mac.com> > Cc: Jan Engelhardt <jengelh@computergmbh.de> > Cc: Adrian Bunk <bunk@kernel.org> > Cc: roel <12o3l@tiscali.nl> > > Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu> A few comments below... Acked-by: Randy Dunlap <randy.dunlap@oracle.com> > --- > Documentation/CodingStyle | 88 +++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 86 insertions(+), 2 deletions(-) > > diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle > index 7f1730f..00b29e4 100644 > --- a/Documentation/CodingStyle > +++ b/Documentation/CodingStyle ... > @@ -779,6 +797,69 @@ includes markers for indentation and mode configuration. People may use their > own custom mode, or may have some other magic method for making indentation > work correctly. > > + Chapter 19: branch prediction optimizations > + > +The kernel includes macros called likely() and unlikely(), which can be used > +as hints to the compiler to optimize branch prediction. They operate by > +asking gcc to shuffle the code around so that the more favorable outcome > +executes linearly, avoiding a JMP instruction; this can improve cache > +pipeline efficiency. For technical details how these macros work, see the > +References section at the end of this document. > + > +An example use of this as as follows: > + > + ptr = kmalloc(size, GFP_KERNEL); > + if (unlikely(!ptr)) > + ... > + > +or > + err = some_function(...); > + if (likely(!err)) > + ... > + > +The main two problems with using un/likely() are that (a) programmers can > +easily be wrong about their code's likelihood to take one branch > +vs. another, and (b) on average, gcc will do a much better job optimizing > +branches that the programmer can. The benefit on some systems for > +predicting correctly can be in saving a few instructions. But the penalty > +for wrong use of un/likely() can be very significant (possibly dozens of > +instructions), as you may be doing a JMP instruction, hurting your pipeline > +cache, EACH time you get to the branch in question! > + > +Therefore, use un/likely() sparingly, consider it primarily for hot paths, > +use it only when you are certain that the condition in question rarely > +happens, be sure that it happens with roughly the same probability under > +most/all user conditions. One rule of thumb suggested is that the > +probability of the branch un/taken should exceed 99% (although some would > +consider 95% as well). Of course, beware of silly mistakes such as > +intending to use likely() and using unlikely() instead. > + > +A good example of this is the above kmalloc(GFP_KERNEL) call. The chances > +of kmalloc() returning NULL are rather small, because even if it doesn't > +have memory to return to you at the moment, with GFP_KERNEL/__GFP_WAIT > +passed, kmalloc() will wait and suspend your thread, while it goes off to > +find some free memory (purging caches, flushing buffers, etc.). In other > +words, kmalloc() tries very hard to give you the memory you asked for by the > +time it return. returns. > + > +Consider the next, bad example. Suppose you're developing a file system ^comma seems odd here > +which performs logically different actions on different types of entities: > +files, directories, symlinks, devices, etc. and you use this code: > + > + if (unlikely(S_ISBLK(mode)) > + ... > + > +On first glance, the above use of unlikely() seems right. After all, most > +file system objects are files and directories, and very few of them tend to > +be block devices. So this optimization should work well, no? Although it's > +true that it'll work well for most users, what about some user who happens > +to have a file system with lots of block devices? Or what if the user has > +only one block device object on the file system, but the user has an > +application which causes the above conditional to be traversed very > +frequently (e.g., a shell script that deals with devices)? Such users will > +be penalized heavily for going [sic] down the wrong path... Therefore, you > +should consider also whether a seemingly-rare condition is indeed rare ALL > +the time. > > > Appendix I: References > @@ -804,6 +885,9 @@ language C, URL: http://www.open-std.org/JTC1/SC22/WG14/ > Kernel CodingStyle, by greg@kroah.com at OLS 2002: > http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_talk/html/ > > +FAQ/LikelyUnlikely: > +http://kernelnewbies.org/FAQ/LikelyUnlikely > + > -- --- ~Randy Phaedrus says that Quality is about caring. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] CodingStyle updates 2007-09-28 21:32 ` [PATCH 1/3] CodingStyle updates Erez Zadok 2007-09-28 21:46 ` Randy Dunlap @ 2007-09-29 14:43 ` Shawn Bohrer 2007-09-29 15:59 ` Scott Preece 2007-09-29 18:01 ` Randy Dunlap 3 siblings, 0 replies; 30+ messages in thread From: Shawn Bohrer @ 2007-09-29 14:43 UTC (permalink / raw) To: Erez Zadok Cc: torvalds, akpm, linux-kernel, Kok, Auke, Kyle Moffett, Jan Engelhardt, Adrian Bunk, roel On Fri, Sep 28, 2007 at 05:32:00PM -0400, Erez Zadok wrote: > 1. Updates chapter 13 (printing kernel messages) to expand on the use of > pr_debug()/pr_info(), what to avoid, and how to hook your debug code with > kernel.h. > > 2. New chapter 19, branch prediction optimizations, discusses the whole > un/likely issue. > > Cc: "Kok, Auke" <auke-jan.h.kok@intel.com> > Cc: Kyle Moffett <mrmacman_g4@mac.com> > Cc: Jan Engelhardt <jengelh@computergmbh.de> > Cc: Adrian Bunk <bunk@kernel.org> > Cc: roel <12o3l@tiscali.nl> > > Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu> > --- > Documentation/CodingStyle | 88 +++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 86 insertions(+), 2 deletions(-) > > diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle > index 7f1730f..00b29e4 100644 > --- a/Documentation/CodingStyle > +++ b/Documentation/CodingStyle > @@ -643,8 +643,26 @@ Printing numbers in parentheses (%d) adds no value and should be avoided. > There are a number of driver model diagnostic macros in <linux/device.h> > which you should use to make sure messages are matched to the right device > and driver, and are tagged with the right level: dev_err(), dev_warn(), > -dev_info(), and so forth. For messages that aren't associated with a > -particular device, <linux/kernel.h> defines pr_debug() and pr_info(). > +dev_info(), and so forth. > + > +A number of people often like to define their own debugging printf's, > +wrapping printk's in #ifdef's that get turned on only when subsystem > +debugging is compiled in (e.g., dprintk, Dprintk, DPRINTK, etc.). Please > +don't reinvent the wheel but use existing mechanisms. For messages that > +aren't associated with a particular device, <linux/kernel.h> defines > +pr_debug() and pr_info(); the latter two translate to printk(KERN_DEBUG) and The latter two? Since there are only two presented I think there is no reason to say "latter". > +printk(KERN_INFO), respectively. However, to get pr_debug() to actually > +emit the message, you'll need to turn on DEBUG in your code, which can be > +done as follows in your subsystem Makefile: > + > +ifeq ($(CONFIG_WHATEVER_DEBUG),y) > +EXTRA_CFLAGS += -DDEBUG > +endif > + > +In this way, you can create a Kconfig parameter to turn on debugging at > +compile time, which will also turn on DEBUG, to enable pr_debug() to emit > +actual messages; conversely, when CONFIG_WHATEVER_DEBUG is off, DEBUG is > +off, and pr_debug() will display nothing. > > Coming up with good debugging messages can be quite a challenge; and once > you have them, they can be a huge help for remote troubleshooting. Such > @@ -779,6 +797,69 @@ includes markers for indentation and mode configuration. People may use their > own custom mode, or may have some other magic method for making indentation > work correctly. > > + Chapter 19: branch prediction optimizations > + > +The kernel includes macros called likely() and unlikely(), which can be used > +as hints to the compiler to optimize branch prediction. They operate by > +asking gcc to shuffle the code around so that the more favorable outcome > +executes linearly, avoiding a JMP instruction; this can improve cache > +pipeline efficiency. For technical details how these macros work, see the > +References section at the end of this document. > + > +An example use of this as as follows: ^^^^^^ > + > + ptr = kmalloc(size, GFP_KERNEL); > + if (unlikely(!ptr)) > + ... > + > +or > + err = some_function(...); > + if (likely(!err)) > + ... -- Shawn ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] CodingStyle updates 2007-09-28 21:32 ` [PATCH 1/3] CodingStyle updates Erez Zadok 2007-09-28 21:46 ` Randy Dunlap 2007-09-29 14:43 ` Shawn Bohrer @ 2007-09-29 15:59 ` Scott Preece 2007-09-29 18:01 ` Randy Dunlap 3 siblings, 0 replies; 30+ messages in thread From: Scott Preece @ 2007-09-29 15:59 UTC (permalink / raw) To: Erez Zadok Cc: torvalds, akpm, linux-kernel, Kok, Auke, Kyle Moffett, Jan Engelhardt, Adrian Bunk, roel A couple of comments interspersed... On 9/28/07, Erez Zadok <ezk@cs.sunysb.edu> wrote: ... > There are a number of driver model diagnostic macros in <linux/device.h> > which you should use to make sure messages are matched to the right device --- I think this "which" is non-restrictive, so it should have a comma after it (I realize that's not part of your patch). It's also possible to read it as restrictive, in which case "that" would be preferable. --- > and driver, and are tagged with the right level: dev_err(), dev_warn(), > -dev_info(), and so forth. For messages that aren't associated with a > -particular device, <linux/kernel.h> defines pr_debug() and pr_info(). > +dev_info(), and so forth. > + > +A number of people often like to define their own debugging printf's, --- "A number of people often like to..." is awkward. How about "Developers sometimes..." or "Too many people..." --- > +wrapping printk's in #ifdef's that get turned on only when subsystem > + Chapter 19: branch prediction optimizations > + > +The kernel includes macros called likely() and unlikely(), which can be used --- You might say "The kernel provides the macros likely()..." --- > +as hints to the compiler to optimize branch prediction. They operate by ... > +A good example of this is the above kmalloc(GFP_KERNEL) call. The chances > +of kmalloc() returning NULL are rather small, because even if it doesn't > +have memory to return to you at the moment, with GFP_KERNEL/__GFP_WAIT > +passed, kmalloc() will wait and suspend your thread, while it goes off to --- The commas after "passed" and "thread" is unnecessary. --- > +find some free memory (purging caches, flushing buffers, etc.). In other > +words, kmalloc() tries very hard to give you the memory you asked for by the > +time it return. --- "...by the time it return." should be "...by the time it returns." --- > + > +Consider the next, bad example. Suppose you're developing a file system > +which performs logically different actions on different types of entities: --- This "which" is restrictive; it would be preferable to use "that" instead. --- > +files, directories, symlinks, devices, etc. and you use this code: > + ... > +be penalized heavily for going [sic] down the wrong path... Therefore, you > +should consider also whether a seemingly-rare condition is indeed rare ALL --- The hyphen isn't necessary when the first word of the compount adjective is an adverb ending in "-ly", so, just "seemingly rare"; or switch to "apparently rare". --- > +the time. ... -- scott preece ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] CodingStyle updates 2007-09-28 21:32 ` [PATCH 1/3] CodingStyle updates Erez Zadok ` (2 preceding siblings ...) 2007-09-29 15:59 ` Scott Preece @ 2007-09-29 18:01 ` Randy Dunlap 2007-09-29 18:29 ` Sam Ravnborg 3 siblings, 1 reply; 30+ messages in thread From: Randy Dunlap @ 2007-09-29 18:01 UTC (permalink / raw) To: Erez Zadok Cc: torvalds, akpm, linux-kernel, Kok, Auke, Kyle Moffett, Jan Engelhardt, Adrian Bunk, roel On Fri, 28 Sep 2007 17:32:00 -0400 Erez Zadok wrote: > 1. Updates chapter 13 (printing kernel messages) to expand on the use of > pr_debug()/pr_info(), what to avoid, and how to hook your debug code with > kernel.h. > > Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu> > --- > Documentation/CodingStyle | 88 +++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 86 insertions(+), 2 deletions(-) > > diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle > index 7f1730f..00b29e4 100644 > --- a/Documentation/CodingStyle > +++ b/Documentation/CodingStyle > @@ -643,8 +643,26 @@ Printing numbers in parentheses (%d) adds no value and should be avoided. > There are a number of driver model diagnostic macros in <linux/device.h> > which you should use to make sure messages are matched to the right device > and driver, and are tagged with the right level: dev_err(), dev_warn(), > -dev_info(), and so forth. For messages that aren't associated with a > -particular device, <linux/kernel.h> defines pr_debug() and pr_info(). > +dev_info(), and so forth. > + > +A number of people often like to define their own debugging printf's, > +wrapping printk's in #ifdef's that get turned on only when subsystem > +debugging is compiled in (e.g., dprintk, Dprintk, DPRINTK, etc.). Please > +don't reinvent the wheel but use existing mechanisms. For messages that > +aren't associated with a particular device, <linux/kernel.h> defines > +pr_debug() and pr_info(); the latter two translate to printk(KERN_DEBUG) and > +printk(KERN_INFO), respectively. However, to get pr_debug() to actually > +emit the message, you'll need to turn on DEBUG in your code, which can be > +done as follows in your subsystem Makefile: > + > +ifeq ($(CONFIG_WHATEVER_DEBUG),y) > +EXTRA_CFLAGS += -DDEBUG > +endif Alternatively, that can be done in your source file, but doing this in the Makefile makes good sense if you have more than one source file. At any rate, it is done in some source files, and when it's done in source files, #define-ing DEBUG should be done before #include <linux/kernel.h> so that the macros are defined as expected. > +In this way, you can create a Kconfig parameter to turn on debugging at > +compile time, which will also turn on DEBUG, to enable pr_debug() to emit > +actual messages; conversely, when CONFIG_WHATEVER_DEBUG is off, DEBUG is > +off, and pr_debug() will display nothing. > > Coming up with good debugging messages can be quite a challenge; and once > you have them, they can be a huge help for remote troubleshooting. Such --- ~Randy Phaedrus says that Quality is about caring. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] CodingStyle updates 2007-09-29 18:01 ` Randy Dunlap @ 2007-09-29 18:29 ` Sam Ravnborg 2007-09-29 20:21 ` [OT] kbuild syntax extension for ccflags and asflags (was: [PATCH 1/3] CodingStyle updates) Ingo Oeser 0 siblings, 1 reply; 30+ messages in thread From: Sam Ravnborg @ 2007-09-29 18:29 UTC (permalink / raw) To: Randy Dunlap Cc: Erez Zadok, torvalds, akpm, linux-kernel, Kok, Auke, Kyle Moffett, Jan Engelhardt, Adrian Bunk, roel On Sat, Sep 29, 2007 at 11:01:29AM -0700, Randy Dunlap wrote: > On Fri, 28 Sep 2007 17:32:00 -0400 Erez Zadok wrote: > > > 1. Updates chapter 13 (printing kernel messages) to expand on the use of > > pr_debug()/pr_info(), what to avoid, and how to hook your debug code with > > kernel.h. > > > > Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu> > > --- > > Documentation/CodingStyle | 88 +++++++++++++++++++++++++++++++++++++++++++- > > 1 files changed, 86 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle > > index 7f1730f..00b29e4 100644 > > --- a/Documentation/CodingStyle > > +++ b/Documentation/CodingStyle > > @@ -643,8 +643,26 @@ Printing numbers in parentheses (%d) adds no value and should be avoided. > > There are a number of driver model diagnostic macros in <linux/device.h> > > which you should use to make sure messages are matched to the right device > > and driver, and are tagged with the right level: dev_err(), dev_warn(), > > -dev_info(), and so forth. For messages that aren't associated with a > > -particular device, <linux/kernel.h> defines pr_debug() and pr_info(). > > +dev_info(), and so forth. > > + > > +A number of people often like to define their own debugging printf's, > > +wrapping printk's in #ifdef's that get turned on only when subsystem > > +debugging is compiled in (e.g., dprintk, Dprintk, DPRINTK, etc.). Please > > +don't reinvent the wheel but use existing mechanisms. For messages that > > +aren't associated with a particular device, <linux/kernel.h> defines > > +pr_debug() and pr_info(); the latter two translate to printk(KERN_DEBUG) and > > +printk(KERN_INFO), respectively. However, to get pr_debug() to actually > > +emit the message, you'll need to turn on DEBUG in your code, which can be > > +done as follows in your subsystem Makefile: > > + > > +ifeq ($(CONFIG_WHATEVER_DEBUG),y) > > +EXTRA_CFLAGS += -DDEBUG > > +endif The abvoe hurst my eyes each time I see it. Lately I have considered extending the kbuild syntax a bit. Introducing ccflags-y asflags-y [with same functionality as the EXTRA_CFLAGS, EXTRA_AFLAGS] would allow us to do: ccflags-$(CONFIG_WHATEVER_DEBUG) := -DDEBUG Much nicer IMHO. Sam ^ permalink raw reply [flat|nested] 30+ messages in thread
* [OT] kbuild syntax extension for ccflags and asflags (was: [PATCH 1/3] CodingStyle updates) 2007-09-29 18:29 ` Sam Ravnborg @ 2007-09-29 20:21 ` Ingo Oeser 2007-09-29 20:24 ` Sam Ravnborg 0 siblings, 1 reply; 30+ messages in thread From: Ingo Oeser @ 2007-09-29 20:21 UTC (permalink / raw) To: Sam Ravnborg Cc: Randy Dunlap, Erez Zadok, torvalds, akpm, linux-kernel, Kok, Auke, Kyle Moffett, Jan Engelhardt, Adrian Bunk, roel Hi Sam, On Saturday 29 September 2007, Sam Ravnborg wrote: > Lately I have considered extending the kbuild syntax a bit. > > Introducing > ccflags-y > asflags-y > > [with same functionality as the EXTRA_CFLAGS, EXTRA_AFLAGS] > would allow us to do: > > ccflags-$(CONFIG_WHATEVER_DEBUG) := -DDEBUG Please do! That is very useful for testing and developing new modules. I learnt a lot from you in this regard and used that kind of syntax to the extreme in some other non-kernel project of mine. There it included also ccflags, asflags and so on. I further split that into -debug-y and -optimize-y flags, but that was just for my own convenience. Best Regards Ingo Oeser ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [OT] kbuild syntax extension for ccflags and asflags (was: [PATCH 1/3] CodingStyle updates) 2007-09-29 20:21 ` [OT] kbuild syntax extension for ccflags and asflags (was: [PATCH 1/3] CodingStyle updates) Ingo Oeser @ 2007-09-29 20:24 ` Sam Ravnborg 0 siblings, 0 replies; 30+ messages in thread From: Sam Ravnborg @ 2007-09-29 20:24 UTC (permalink / raw) To: Ingo Oeser Cc: Randy Dunlap, Erez Zadok, torvalds, akpm, linux-kernel, Kok, Auke, Kyle Moffett, Jan Engelhardt, Adrian Bunk, roel > > Lately I have considered extending the kbuild syntax a bit. > > > > Introducing > > ccflags-y > > asflags-y > > > > [with same functionality as the EXTRA_CFLAGS, EXTRA_AFLAGS] > > would allow us to do: > > > > ccflags-$(CONFIG_WHATEVER_DEBUG) := -DDEBUG > > Please do! > > That is very useful for testing and developing new modules. > I learnt a lot from you in this regard and used that kind > of syntax to the extreme in some other non-kernel project > of mine. There it included also ccflags, asflags and so on. > > I further split that into -debug-y and -optimize-y flags, > but that was just for my own convenience. Thanks for feedback Ingo. I just started a new thread were I outlined the ideas I have for further extending (some may say uglifying) the kbuild syntax. I will see the outcome of this thread before implmenting something. PS. The implementation part is obviously trivial, the hard part is the documentation :-) Sam ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2/3] Update usage string for checkpatch.pl 2007-09-28 21:31 [PATCH] 0/3 coding standards documentation/code updates Erez Zadok 2007-09-28 21:32 ` [PATCH 1/3] CodingStyle updates Erez Zadok @ 2007-09-28 21:32 ` Erez Zadok 2007-09-28 21:32 ` [PATCH 3/3] New script to check coding-style compliance on multiple regular files Erez Zadok 2007-09-29 18:18 ` [PATCH] 0/3 coding standards documentation/code updates Linus Torvalds 3 siblings, 0 replies; 30+ messages in thread From: Erez Zadok @ 2007-09-28 21:32 UTC (permalink / raw) To: torvalds, akpm; +Cc: linux-kernel, Erez Zadok Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu> --- scripts/checkpatch.pl | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index dae7d30..ecbb030 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -33,6 +33,7 @@ if ($#ARGV < 0) { print "version: $V\n"; print "options: -q => quiet\n"; print " --no-tree => run without a kernel tree\n"; + print " --no-signoff => don't check for signed-off-by\n"; exit(1); } -- 1.5.2.2 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 3/3] New script to check coding-style compliance on multiple regular files 2007-09-28 21:31 [PATCH] 0/3 coding standards documentation/code updates Erez Zadok 2007-09-28 21:32 ` [PATCH 1/3] CodingStyle updates Erez Zadok 2007-09-28 21:32 ` [PATCH 2/3] Update usage string for checkpatch.pl Erez Zadok @ 2007-09-28 21:32 ` Erez Zadok 2007-09-29 10:10 ` Sam Ravnborg 2007-09-29 18:18 ` [PATCH] 0/3 coding standards documentation/code updates Linus Torvalds 3 siblings, 1 reply; 30+ messages in thread From: Erez Zadok @ 2007-09-28 21:32 UTC (permalink / raw) To: torvalds, akpm; +Cc: linux-kernel, Erez Zadok The script calls checkpatch.pl on each file, and formats any error messages to comply with standard compiler error messages: file_name:line_number:error_message This is particularly useful when run from within a text editor which can parse these error messages and show the user a buffer with the file in question, placing the cursor on the offending line (e.g., Emacs's "M-x next-error" command, bound by default to C-x `). Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu> --- scripts/check-coding-standards.pl | 59 +++++++++++++++++++++++++++++++++++++ 1 files changed, 59 insertions(+), 0 deletions(-) create mode 100755 scripts/check-coding-standards.pl diff --git a/scripts/check-coding-standards.pl b/scripts/check-coding-standards.pl new file mode 100755 index 0000000..a1ba597 --- /dev/null +++ b/scripts/check-coding-standards.pl @@ -0,0 +1,59 @@ +#!/usr/bin/perl -w +# (c) 2007, Erez Zadok <ezk@cs.sunysb.edu> +# Licensed under the terms of the GNU GPL License version 2 +# +# Check one more more files for compliance with coding standards. +# Outputs standard compiler-error format, one error per line, as follows +# filename:lineno:errormsg +# This standard error message can be parsed by various tools, including +# Emacs's M-x next-error +# +# Usage: check-coding-standards.pl file [files...] + + +if (!defined($ARGV[0])) { + printf("Usage: $0 file [files...]\n"); + exit(1); +} + +$msg=""; +$lineno=0; +foreach $file (@ARGV) { + die "$file: $!" unless (-f $file); + open(FILE, "diff -u /dev/null $file | perl scripts/checkpatch.pl --no-signoff - |") || die "$file: $!"; + while (($line = <FILE>)) { + chop $line; + next if ($line =~ /^$/); + if ($line =~ /^ERROR: /) { + $msg = $line; + next; + } + if ($line =~ /^WARNING: /) { + $msg = $line; + next; + } + if ($line =~ /^CHECK: /) { + $msg = $line; + next; + } + if ($line =~ /^#/) { + $lineno = (split(/:/, $line))[3]; + next; + } + if ($line =~ /^\+/) { + if ($lineno > 0) { + printf(STDOUT "%s:%d:%s\n", $file, $lineno, $msg); + $msg=""; + $lineno=0; + } + next; + } + next if ($line =~ /^\s+\^/); + next if ($line =~ /^Your patch has style problems, please review/); + next if ($line =~ /^are false positives report them to the/); + next if ($line =~ /^CHECKPATCH in MAINTAINERS/); + next if ($line =~ /^Your patch has no obvious style problems/); + die "unknown output from checkpatch: $line: $."; + } + close(FILE); +} -- 1.5.2.2 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 3/3] New script to check coding-style compliance on multiple regular files 2007-09-28 21:32 ` [PATCH 3/3] New script to check coding-style compliance on multiple regular files Erez Zadok @ 2007-09-29 10:10 ` Sam Ravnborg 0 siblings, 0 replies; 30+ messages in thread From: Sam Ravnborg @ 2007-09-29 10:10 UTC (permalink / raw) To: Erez Zadok; +Cc: torvalds, akpm, linux-kernel, apw, Randy Dunlap Hi Erez. On Fri, Sep 28, 2007 at 05:32:02PM -0400, Erez Zadok wrote: > The script calls checkpatch.pl on each file, and formats any error messages > to comply with standard compiler error messages: > > file_name:line_number:error_message > > This is particularly useful when run from within a text editor which can > parse these error messages and show the user a buffer with the file in > question, placing the cursor on the offending line (e.g., Emacs's "M-x > next-error" command, bound by default to C-x `). The concept is wrong here. If we want checkpatch to generate message in standard gcc format then we should fix checkpatch and not add a wrapper script around it. For checkpatch related patches please forward them to the checkpatch maintainers. >From MAINTAINERS: CHECKPATCH P: Andy Whitcroft M: apw@shadowen.org P: Randy Dunlap M: rdunlap@xenotime.net P: Joel Schopp M: jschopp@austin.ibm.com S: Supported Sam ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-28 21:31 [PATCH] 0/3 coding standards documentation/code updates Erez Zadok ` (2 preceding siblings ...) 2007-09-28 21:32 ` [PATCH 3/3] New script to check coding-style compliance on multiple regular files Erez Zadok @ 2007-09-29 18:18 ` Linus Torvalds 2007-09-29 19:56 ` J. Bruce Fields ` (3 more replies) 3 siblings, 4 replies; 30+ messages in thread From: Linus Torvalds @ 2007-09-29 18:18 UTC (permalink / raw) To: Erez Zadok; +Cc: akpm, linux-kernel On Fri, 28 Sep 2007, Erez Zadok wrote: > > Documentation/CodingStyle | 88 +++++++++++++++++++++++++++++++++++++- I'm not very happy with this. "CodingStyle" should be about the big issues, not about details. Yes, we've messed that up over the years, but let's not continue that. In other words, I'd suggest *removing* lines from CodingStyle, not adding them. The file has already gone from a "good general principles" to "lots of stupid details". Let's not make it worse. Linus ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-29 18:18 ` [PATCH] 0/3 coding standards documentation/code updates Linus Torvalds @ 2007-09-29 19:56 ` J. Bruce Fields 2007-09-29 20:14 ` Randy Dunlap 2007-09-30 2:06 ` Theodore Tso 2007-09-29 21:56 ` Robert P. J. Day ` (2 subsequent siblings) 3 siblings, 2 replies; 30+ messages in thread From: J. Bruce Fields @ 2007-09-29 19:56 UTC (permalink / raw) To: Linus Torvalds; +Cc: Erez Zadok, akpm, linux-kernel On Sat, Sep 29, 2007 at 11:18:05AM -0700, Linus Torvalds wrote: > I'm not very happy with this. > > "CodingStyle" should be about the big issues, not about details. Yes, > we've messed that up over the years, but let's not continue that. > > In other words, I'd suggest *removing* lines from CodingStyle, not adding > them. The file has already gone from a "good general principles" to "lots > of stupid details". Let's not make it worse. It'd be nice to split the current CodingStyle into two documents: - A shorter CodingStyle that gives the spirit of the style (short functions, minimal nesting, logic as straightforward as possible, etc.), and addresses the most commonly repeated mistakes, without so much detail that people's eyes glaze over. You want to be able to recommend it to your students (or whoever) in reasonable confidence that they'll actually read it and have fun (leave the jokes in!). Currently I'm suspicious that it's becoming something that everybody recommends but noone bothers to sit down and read anymore unless they're working on it. - A CodingStyleReference that's just a long dry list of rules, organized to make it easy to look up an individual rule when needed. That'd also take the pressure of CodingStyle to accept every new detail. It'd be a start just to revert CodingStyle to its original content and move the rest to CodingStyleReference. But someone would want to skim through the CodingStyle history for any legimate corrections that we want to keep. --b. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-29 19:56 ` J. Bruce Fields @ 2007-09-29 20:14 ` Randy Dunlap 2007-09-30 2:06 ` Theodore Tso 1 sibling, 0 replies; 30+ messages in thread From: Randy Dunlap @ 2007-09-29 20:14 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Linus Torvalds, Erez Zadok, akpm, linux-kernel On Sat, 29 Sep 2007 15:56:38 -0400 J. Bruce Fields wrote: > On Sat, Sep 29, 2007 at 11:18:05AM -0700, Linus Torvalds wrote: > > I'm not very happy with this. > > > > "CodingStyle" should be about the big issues, not about details. Yes, > > we've messed that up over the years, but let's not continue that. > > > > In other words, I'd suggest *removing* lines from CodingStyle, not adding > > them. The file has already gone from a "good general principles" to "lots > > of stupid details". Let's not make it worse. > > It'd be nice to split the current CodingStyle into two documents: I agree. This is just what I was thinking during lunch. > - A shorter CodingStyle that gives the spirit of the style > (short functions, minimal nesting, logic as straightforward as > possible, etc.), and addresses the most commonly repeated > mistakes, without so much detail that people's eyes glaze > over. You want to be able to recommend it to your students > (or whoever) in reasonable confidence that they'll actually > read it and have fun (leave the jokes in!). Currently I'm > suspicious that it's becoming something that everybody > recommends but noone bothers to sit down and read anymore > unless they're working on it. > > - A CodingStyleReference that's just a long dry list of rules, > organized to make it easy to look up an individual rule when > needed. That'd also take the pressure of CodingStyle to > accept every new detail. > > It'd be a start just to revert CodingStyle to its original content and > move the rest to CodingStyleReference. But someone would want to skim > through the CodingStyle history for any legimate corrections that we > want to keep. --- ~Randy Phaedrus says that Quality is about caring. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-29 19:56 ` J. Bruce Fields 2007-09-29 20:14 ` Randy Dunlap @ 2007-09-30 2:06 ` Theodore Tso 2007-09-30 3:28 ` Erez Zadok 1 sibling, 1 reply; 30+ messages in thread From: Theodore Tso @ 2007-09-30 2:06 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Linus Torvalds, Erez Zadok, akpm, linux-kernel On Sat, Sep 29, 2007 at 03:56:38PM -0400, J. Bruce Fields wrote: > It'd be a start just to revert CodingStyle to its original content and > move the rest to CodingStyleReference. But someone would want to skim > through the CodingStyle history for any legimate corrections that we > want to keep. How about CodingStyleSuggestions? And let's make it clear they are suggestions, and not things that checkpatch should be flaming about unless people request the all of the annoying associated false positives via --spam-me-harder. :-) - Ted ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-30 2:06 ` Theodore Tso @ 2007-09-30 3:28 ` Erez Zadok 0 siblings, 0 replies; 30+ messages in thread From: Erez Zadok @ 2007-09-30 3:28 UTC (permalink / raw) To: Theodore Tso Cc: J. Bruce Fields, Linus Torvalds, Erez Zadok, akpm, linux-kernel In message <20070930020637.GA29996@thunk.org>, Theodore Tso writes: > On Sat, Sep 29, 2007 at 03:56:38PM -0400, J. Bruce Fields wrote: > > It'd be a start just to revert CodingStyle to its original content and > > move the rest to CodingStyleReference. But someone would want to skim > > through the CodingStyle history for any legimate corrections that we > > want to keep. > > How about CodingStyleSuggestions? And let's make it clear they are > suggestions, and not things that checkpatch should be flaming about > unless people request the all of the annoying associated false > positives via --spam-me-harder. :-) FWIW, the checkpatch script is woefully out of sync with CodingStyle. I assume that checkpatch.pl is generally more authoritative (sans "(%d)" :-) > - Ted Erez. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-29 18:18 ` [PATCH] 0/3 coding standards documentation/code updates Linus Torvalds 2007-09-29 19:56 ` J. Bruce Fields @ 2007-09-29 21:56 ` Robert P. J. Day 2007-09-30 0:23 ` Erez Zadok 2007-09-30 2:24 ` Valdis.Kletnieks 3 siblings, 0 replies; 30+ messages in thread From: Robert P. J. Day @ 2007-09-29 21:56 UTC (permalink / raw) To: Linus Torvalds; +Cc: Erez Zadok, akpm, linux-kernel On Sat, 29 Sep 2007, Linus Torvalds wrote: > > > On Fri, 28 Sep 2007, Erez Zadok wrote: > > > > Documentation/CodingStyle | 88 +++++++++++++++++++++++++++++++++++++- > > I'm not very happy with this. > > "CodingStyle" should be about the big issues, not about details. > Yes, we've messed that up over the years, but let's not continue > that. > > In other words, I'd suggest *removing* lines from CodingStyle, not > adding them. The file has already gone from a "good general > principles" to "lots of stupid details". Let's not make it worse. here's a radical idea -- what about splitting the content across two documents? you know ... perhaps "coding style aesthetics" versus "coding distinctions" or something like that. oh, wait ... http://lkml.org/lkml/2007/1/1/18 :-) rday -- ======================================================================== Robert P. J. Day Linux Consulting, Training and Annoying Kernel Pedantry Waterloo, Ontario, CANADA http://crashcourse.ca ======================================================================== ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-29 18:18 ` [PATCH] 0/3 coding standards documentation/code updates Linus Torvalds 2007-09-29 19:56 ` J. Bruce Fields 2007-09-29 21:56 ` Robert P. J. Day @ 2007-09-30 0:23 ` Erez Zadok 2007-09-30 0:49 ` Linus Torvalds 2007-09-30 2:24 ` Valdis.Kletnieks 3 siblings, 1 reply; 30+ messages in thread From: Erez Zadok @ 2007-09-30 0:23 UTC (permalink / raw) To: Linus Torvalds; +Cc: Erez Zadok, akpm, linux-kernel In message <alpine.LFD.0.999.0709291116180.3579@woody.linux-foundation.org>, Linus Torvalds writes: > > > On Fri, 28 Sep 2007, Erez Zadok wrote: > > > > Documentation/CodingStyle | 88 +++++++++++++++++++++++++++++++++++++- > > I'm not very happy with this. > > "CodingStyle" should be about the big issues, not about details. Yes, > we've messed that up over the years, but let's not continue that. > > In other words, I'd suggest *removing* lines from CodingStyle, not adding > them. The file has already gone from a "good general principles" to "lots > of stupid details". Let's not make it worse. > > Linus There's a lot of good value in having all those details, as it helps people standardize on more common practices, including details. I think removing all those details may only increase the amount-of less-accepted code to be posted, resulting in more lkml people having to repeat themselves on what not to do. Only now, they won't be able to point people to the CodingStyle file for what they should do right. Would you prefer if CodingStyle was reorganized or even split into (1) general principles and (2) details? Perhaps we need a CodingStylePrinciples and a CodingStyleDetails? IOW, where should that very useful info live? Erez. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-30 0:23 ` Erez Zadok @ 2007-09-30 0:49 ` Linus Torvalds 2007-09-30 4:01 ` Erez Zadok 0 siblings, 1 reply; 30+ messages in thread From: Linus Torvalds @ 2007-09-30 0:49 UTC (permalink / raw) To: Erez Zadok; +Cc: akpm, linux-kernel On Sat, 29 Sep 2007, Erez Zadok wrote: > > Would you prefer if CodingStyle was reorganized or even split into (1) > general principles and (2) details? Perhaps we need a CodingStylePrinciples > and a CodingStyleDetails? I'm certainly ok with the split into two files. What I'm not ok with is really important stuff (indentation), and then mixing in silly rules ("parenthesis are bad in printk's"?) Linus ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-30 0:49 ` Linus Torvalds @ 2007-09-30 4:01 ` Erez Zadok 2007-09-30 17:40 ` Mark Lord 0 siblings, 1 reply; 30+ messages in thread From: Erez Zadok @ 2007-09-30 4:01 UTC (permalink / raw) To: Linus Torvalds; +Cc: Erez Zadok, akpm, linux-kernel In message <alpine.LFD.0.999.0709291748160.3579@woody.linux-foundation.org>, Linus Torvalds writes: > > > On Sat, 29 Sep 2007, Erez Zadok wrote: > > > > Would you prefer if CodingStyle was reorganized or even split into (1) > > general principles and (2) details? Perhaps we need a CodingStylePrinciples > > and a CodingStyleDetails? > > I'm certainly ok with the split into two files. > > What I'm not ok with is really important stuff (indentation), and then > mixing in silly rules ("parenthesis are bad in printk's"?) > > Linus OK, looking at CodingStyle, I see two kinds of chapters. The first is stuff that's more generic to C, and the other is more specific to Linux and how one codes in the linux kernel. So I propose the following: 1. we create a new file called CodingSuggestions 2. we keep in CodingStyle the following chapters Chapter 1: Indentation Chapter 2: Breaking long lines and strings Chapter 3: Placing Braces and Spaces Chapter 4: Naming Chapter 5: Typedefs Chapter 6: Functions Chapter 7: Centralized exiting of functions Chapter 8: Commenting Chapter 9: You've made a mess of it Note: I'd suggest we rename the title of ch9 to "Custom Editor Programming/Indentation Modes" or something more descriptive. Chapter 10: Kconfig configuration files Chapter 11: Data structures Chapter 12: Macros, Enums and RTL Chapter 15: The inline disease Chapter 16: Function return values and names Chapter 18: Editor modelines and other cruft 3. move the following chapters to CodingSuggestions: Chapter 13: Printing kernel messages Note: ch13 is the one which mentions the don't put parentheses around %d. Chapter 14: Allocating memory Chapter 17: Don't re-invent the kernel macros Chapter 19: branch prediction optimizations (the un/likely debacle) 4. We go through checkpatch.pl and ensure that every test in checkpatch is documented either in CodingStyle or in CodingSuggestions, determined by whether checkpatch considers it an ERROR, WARNING, or just CHECK. I think the above chapter split will be a reasonable start, and of course we can tweak things over time. The general idea is that CodingStyle will remain largely unchanged (until such day as the kernel is rewritten in Java :-), while CodingSuggestions will evolve over time and be kept in sync with checkpatch. But, there's something really nice abuot having to point people to just one file instead of two. We could also keep it all in one file, but split it into two parts: Part 1: Mandatory Coding Style Chapter 1: indentation ... Part 2: Coding Style Suggestions Chapter n: printing kernel messages ... Erez. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-30 4:01 ` Erez Zadok @ 2007-09-30 17:40 ` Mark Lord 2007-09-30 17:59 ` Randy Dunlap 0 siblings, 1 reply; 30+ messages in thread From: Mark Lord @ 2007-09-30 17:40 UTC (permalink / raw) To: Erez Zadok; +Cc: Linus Torvalds, akpm, linux-kernel Erez Zadok wrote: > In message <alpine.LFD.0.999.0709291748160.3579@woody.linux-foundation.org>, Linus Torvalds writes: >> >> On Sat, 29 Sep 2007, Erez Zadok wrote: >>> Would you prefer if CodingStyle was reorganized or even split into (1) >>> general principles and (2) details? Perhaps we need a CodingStylePrinciples >>> and a CodingStyleDetails? >> I'm certainly ok with the split into two files. >> >> What I'm not ok with is really important stuff (indentation), and then >> mixing in silly rules ("parenthesis are bad in printk's"?) >> >> Linus > > OK, looking at CodingStyle, I see two kinds of chapters. The first is stuff > that's more generic to C, and the other is more specific to Linux and how > one codes in the linux kernel. So I propose the following: > > 1. we create a new file called CodingSuggestions > > 2. we keep in CodingStyle the following chapters > > Chapter 1: Indentation > Chapter 2: Breaking long lines and strings > Chapter 3: Placing Braces and Spaces > Chapter 4: Naming > Chapter 5: Typedefs > Chapter 6: Functions > Chapter 7: Centralized exiting of functions > Chapter 8: Commenting > Chapter 9: You've made a mess of it > > Note: I'd suggest we rename the title of ch9 to "Custom Editor > Programming/Indentation Modes" or something more descriptive. > > Chapter 10: Kconfig configuration files > Chapter 11: Data structures > Chapter 12: Macros, Enums and RTL > Chapter 15: The inline disease > Chapter 16: Function return values and names > Chapter 18: Editor modelines and other cruft > > 3. move the following chapters to CodingSuggestions: > > Chapter 13: Printing kernel messages > Note: ch13 is the one which mentions the don't put parentheses around %d. > > Chapter 14: Allocating memory > Chapter 17: Don't re-invent the kernel macros > Chapter 19: branch prediction optimizations (the un/likely debacle) Super. And then, in the spirit of Linus's request, we can submit another patch that simply removes that second file completely from the kernel source tree! Yay! The suits actually lose for once! (okay, so I can fantasize if I want to). ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-30 17:40 ` Mark Lord @ 2007-09-30 17:59 ` Randy Dunlap 0 siblings, 0 replies; 30+ messages in thread From: Randy Dunlap @ 2007-09-30 17:59 UTC (permalink / raw) To: Mark Lord; +Cc: Erez Zadok, Linus Torvalds, akpm, linux-kernel On Sun, 30 Sep 2007 13:40:13 -0400 Mark Lord wrote: > Erez Zadok wrote: > > In message <alpine.LFD.0.999.0709291748160.3579@woody.linux-foundation.org>, Linus Torvalds writes: > >> > >> On Sat, 29 Sep 2007, Erez Zadok wrote: > >>> Would you prefer if CodingStyle was reorganized or even split into (1) > >>> general principles and (2) details? Perhaps we need a CodingStylePrinciples > >>> and a CodingStyleDetails? > >> I'm certainly ok with the split into two files. > >> > >> What I'm not ok with is really important stuff (indentation), and then > >> mixing in silly rules ("parenthesis are bad in printk's"?) > >> > >> Linus > > > > OK, looking at CodingStyle, I see two kinds of chapters. The first is stuff > > that's more generic to C, and the other is more specific to Linux and how > > one codes in the linux kernel. So I propose the following: > > > > 1. we create a new file called CodingSuggestions > > > > 2. we keep in CodingStyle the following chapters > > > > Chapter 1: Indentation > > Chapter 2: Breaking long lines and strings > > Chapter 3: Placing Braces and Spaces > > Chapter 4: Naming > > Chapter 5: Typedefs > > Chapter 6: Functions > > Chapter 7: Centralized exiting of functions > > Chapter 8: Commenting > > Chapter 9: You've made a mess of it > > > > Note: I'd suggest we rename the title of ch9 to "Custom Editor > > Programming/Indentation Modes" or something more descriptive. > > > > Chapter 10: Kconfig configuration files > > Chapter 11: Data structures > > Chapter 12: Macros, Enums and RTL > > Chapter 15: The inline disease > > Chapter 16: Function return values and names > > Chapter 18: Editor modelines and other cruft > > > > 3. move the following chapters to CodingSuggestions: > > > > Chapter 13: Printing kernel messages > > Note: ch13 is the one which mentions the don't put parentheses around %d. > > > > Chapter 14: Allocating memory > > Chapter 17: Don't re-invent the kernel macros > > Chapter 19: branch prediction optimizations (the un/likely debacle) > > Super. And then, in the spirit of Linus's request, > we can submit another patch that simply removes that second file > completely from the kernel source tree! or just revert CodingStyle to its contents of say 4 years ago. > Yay! The suits actually lose for once! > (okay, so I can fantasize if I want to). Yep, you can. The other losers are new contributors who get rejected for not understanding implicit kernel coding styles and people who get to try to explain to them on a repeating basis (if even that happens). or the kernel maintainers if coding style doesn't matter so much and uglier code begins to be merged. --- ~Randy ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-29 18:18 ` [PATCH] 0/3 coding standards documentation/code updates Linus Torvalds ` (2 preceding siblings ...) 2007-09-30 0:23 ` Erez Zadok @ 2007-09-30 2:24 ` Valdis.Kletnieks 2007-09-30 3:00 ` Linus Torvalds 2007-09-30 3:27 ` Al Viro 3 siblings, 2 replies; 30+ messages in thread From: Valdis.Kletnieks @ 2007-09-30 2:24 UTC (permalink / raw) To: Linus Torvalds; +Cc: Erez Zadok, akpm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 861 bytes --] On Sat, 29 Sep 2007 11:18:05 PDT, Linus Torvalds said: > "CodingStyle" should be about the big issues, not about details. Yes, > we've messed that up over the years, but let's not continue that. > > In other words, I'd suggest *removing* lines from CodingStyle, not adding > them. The file has already gone from a "good general principles" to "lots > of stupid details". Let's not make it worse. I think there needs to be a "sense of fairness" attached here - CodingStyle should cover all the stuff maintainers/reviewers are allowed to whinge about. Otherwise, we get maintainers whinging about undocumented style rules, and we get code submitters who are unhappy because they have no real way to tell if their code really *is* stylistically lacking, or if they just have a jerk maintainer who wants to keep their code out-of-tree for political reasons. [-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --] ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-30 2:24 ` Valdis.Kletnieks @ 2007-09-30 3:00 ` Linus Torvalds 2007-09-30 3:29 ` Valdis.Kletnieks 2007-09-30 3:27 ` Al Viro 1 sibling, 1 reply; 30+ messages in thread From: Linus Torvalds @ 2007-09-30 3:00 UTC (permalink / raw) To: Valdis.Kletnieks; +Cc: Erez Zadok, akpm, linux-kernel On Sat, 29 Sep 2007, Valdis.Kletnieks@vt.edu wrote: > > I think there needs to be a "sense of fairness" attached here - CodingStyle > should cover all the stuff maintainers/reviewers are allowed to whinge about. No. People whine too much as is. Don't give them *license* to do so. Linus ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-30 3:00 ` Linus Torvalds @ 2007-09-30 3:29 ` Valdis.Kletnieks 2007-09-30 3:35 ` Linus Torvalds 2007-09-30 17:57 ` Theodore Tso 0 siblings, 2 replies; 30+ messages in thread From: Valdis.Kletnieks @ 2007-09-30 3:29 UTC (permalink / raw) To: Linus Torvalds; +Cc: Erez Zadok, akpm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 436 bytes --] On Sat, 29 Sep 2007 20:00:01 PDT, Linus Torvalds said: > On Sat, 29 Sep 2007, Valdis.Kletnieks@vt.edu wrote: > > I think there needs to be a "sense of fairness" attached here - CodingStyle > > should cover all the stuff maintainers/reviewers are allowed to whinge about. > > No. > > People whine too much as is. Don't give them *license* to do so. I kind of meant the *maintainers* couldn't whine about things not in CodingStyle ;) [-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --] ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-30 3:29 ` Valdis.Kletnieks @ 2007-09-30 3:35 ` Linus Torvalds 2007-09-30 17:57 ` Theodore Tso 1 sibling, 0 replies; 30+ messages in thread From: Linus Torvalds @ 2007-09-30 3:35 UTC (permalink / raw) To: Valdis.Kletnieks; +Cc: Erez Zadok, akpm, linux-kernel On Sat, 29 Sep 2007, Valdis.Kletnieks@vt.edu wrote: > > I kind of meant the *maintainers* couldn't whine about things not in > CodingStyle ;) No, I understood you. I'm personally of the opinion that the automated style checking, and having detailed rules is always a mistake. It's *much* better to teach people the big things. And the small things will vary from person to person _anyway_, so it's not like you can really document them. Linus ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-30 3:29 ` Valdis.Kletnieks 2007-09-30 3:35 ` Linus Torvalds @ 2007-09-30 17:57 ` Theodore Tso 1 sibling, 0 replies; 30+ messages in thread From: Theodore Tso @ 2007-09-30 17:57 UTC (permalink / raw) To: Valdis.Kletnieks; +Cc: Linus Torvalds, Erez Zadok, akpm, linux-kernel On Sat, Sep 29, 2007 at 11:29:33PM -0400, Valdis.Kletnieks@vt.edu wrote: > On Sat, 29 Sep 2007 20:00:01 PDT, Linus Torvalds said: > > On Sat, 29 Sep 2007, Valdis.Kletnieks@vt.edu wrote: > > > I think there needs to be a "sense of fairness" attached here - CodingStyle > > > should cover all the stuff maintainers/reviewers are allowed to whinge about. > > > > No. > > > > People whine too much as is. Don't give them *license* to do so. > > I kind of meant the *maintainers* couldn't whine about things not in CodingStyle ;) > And yet people want to give license for maintainers to whine about parenthesis in printk's? Wow. - Ted ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-30 2:24 ` Valdis.Kletnieks 2007-09-30 3:00 ` Linus Torvalds @ 2007-09-30 3:27 ` Al Viro 2007-09-30 3:39 ` Valdis.Kletnieks 1 sibling, 1 reply; 30+ messages in thread From: Al Viro @ 2007-09-30 3:27 UTC (permalink / raw) To: Valdis.Kletnieks; +Cc: Linus Torvalds, Erez Zadok, akpm, linux-kernel On Sat, Sep 29, 2007 at 10:24:01PM -0400, Valdis.Kletnieks@vt.edu wrote: > On Sat, 29 Sep 2007 11:18:05 PDT, Linus Torvalds said: > > > "CodingStyle" should be about the big issues, not about details. Yes, > > we've messed that up over the years, but let's not continue that. > > > > In other words, I'd suggest *removing* lines from CodingStyle, not adding > > them. The file has already gone from a "good general principles" to "lots > > of stupid details". Let's not make it worse. > > I think there needs to be a "sense of fairness" attached here - CodingStyle > should cover all the stuff maintainers/reviewers are allowed to whinge about. > Otherwise, we get maintainers whinging about undocumented style rules, and > we get code submitters who are unhappy because they have no real way to tell > if their code really *is* stylistically lacking, or if they just have a > jerk maintainer who wants to keep their code out-of-tree for political > reasons. ... and no matter how many rules you put down, it's still possible to write a code that will be awful stylistically while adhering to all of them. Religiously. IOW, it's not going to eliminate that kind of fights. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] 0/3 coding standards documentation/code updates 2007-09-30 3:27 ` Al Viro @ 2007-09-30 3:39 ` Valdis.Kletnieks 0 siblings, 0 replies; 30+ messages in thread From: Valdis.Kletnieks @ 2007-09-30 3:39 UTC (permalink / raw) To: Al Viro; +Cc: Linus Torvalds, Erez Zadok, akpm, linux-kernel [-- Attachment #1: Type: text/plain, Size: 358 bytes --] On Sun, 30 Sep 2007 04:27:24 BST, Al Viro said: > ... and no matter how many rules you put down, it's still possible to > write a code that will be awful stylistically while adhering to all of > them. Religiously. I've run into those sort of programmers. Unfortunately, there's no real cure for them short of a baseball/cricket bat or similar implement. [-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --] ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2007-09-30 18:00 UTC | newest] Thread overview: 30+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-09-28 21:31 [PATCH] 0/3 coding standards documentation/code updates Erez Zadok 2007-09-28 21:32 ` [PATCH 1/3] CodingStyle updates Erez Zadok 2007-09-28 21:46 ` Randy Dunlap 2007-09-29 14:43 ` Shawn Bohrer 2007-09-29 15:59 ` Scott Preece 2007-09-29 18:01 ` Randy Dunlap 2007-09-29 18:29 ` Sam Ravnborg 2007-09-29 20:21 ` [OT] kbuild syntax extension for ccflags and asflags (was: [PATCH 1/3] CodingStyle updates) Ingo Oeser 2007-09-29 20:24 ` Sam Ravnborg 2007-09-28 21:32 ` [PATCH 2/3] Update usage string for checkpatch.pl Erez Zadok 2007-09-28 21:32 ` [PATCH 3/3] New script to check coding-style compliance on multiple regular files Erez Zadok 2007-09-29 10:10 ` Sam Ravnborg 2007-09-29 18:18 ` [PATCH] 0/3 coding standards documentation/code updates Linus Torvalds 2007-09-29 19:56 ` J. Bruce Fields 2007-09-29 20:14 ` Randy Dunlap 2007-09-30 2:06 ` Theodore Tso 2007-09-30 3:28 ` Erez Zadok 2007-09-29 21:56 ` Robert P. J. Day 2007-09-30 0:23 ` Erez Zadok 2007-09-30 0:49 ` Linus Torvalds 2007-09-30 4:01 ` Erez Zadok 2007-09-30 17:40 ` Mark Lord 2007-09-30 17:59 ` Randy Dunlap 2007-09-30 2:24 ` Valdis.Kletnieks 2007-09-30 3:00 ` Linus Torvalds 2007-09-30 3:29 ` Valdis.Kletnieks 2007-09-30 3:35 ` Linus Torvalds 2007-09-30 17:57 ` Theodore Tso 2007-09-30 3:27 ` Al Viro 2007-09-30 3:39 ` Valdis.Kletnieks
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox