* [PATCH 1/2] [CodingStyle] Add comment to not put spaces before tabs. @ 2007-05-25 17:25 Auke Kok 2007-05-25 17:25 ` [PATCH 2/2] [condingstyle] Add chapter on tests Auke Kok 2007-05-25 17:32 ` [PATCH 1/2] [CodingStyle] Add comment to not put spaces before tabs Stefan Richter 0 siblings, 2 replies; 19+ messages in thread From: Auke Kok @ 2007-05-25 17:25 UTC (permalink / raw) To: randy.dunlap; +Cc: auke-jan.h.kok, linux-kernel Mixed space/tab indentation is wrong. Ironically this is what the coding style document actually shows ;) Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com> --- Documentation/CodingStyle | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle index afc2867..f518395 100644 --- a/Documentation/CodingStyle +++ b/Documentation/CodingStyle @@ -13,7 +13,7 @@ and NOT read it. Burn them, it's a great symbolic gesture. Anyway, here goes: - Chapter 1: Indentation + Chapter 1: Indentation Tabs are 8 characters, and thus indentations are also 8 characters. There are heretic movements that try to make indentations 4 (or even 2!) @@ -71,6 +71,8 @@ used for indentation, and the above example is deliberately broken. Get a decent editor and don't leave whitespace at the end of lines. +Don't put spaces before tabs or mix them. + Chapter 2: Breaking long lines and strings ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/2] [condingstyle] Add chapter on tests 2007-05-25 17:25 [PATCH 1/2] [CodingStyle] Add comment to not put spaces before tabs Auke Kok @ 2007-05-25 17:25 ` Auke Kok 2007-05-25 17:34 ` Stefan Richter ` (2 more replies) 2007-05-25 17:32 ` [PATCH 1/2] [CodingStyle] Add comment to not put spaces before tabs Stefan Richter 1 sibling, 3 replies; 19+ messages in thread From: Auke Kok @ 2007-05-25 17:25 UTC (permalink / raw) To: randy.dunlap; +Cc: auke-jan.h.kok, linux-kernel Several standards have been established on how to format tests and use NULL/false/true tests. Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com> --- Documentation/CodingStyle | 51 +++++++++++++++++++++++++++++++++++---------- 1 files changed, 40 insertions(+), 11 deletions(-) diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle index f518395..3635b38 100644 --- a/Documentation/CodingStyle +++ b/Documentation/CodingStyle @@ -393,7 +393,7 @@ int fun(int a) int result = 0; char *buffer = kmalloc(SIZE); - if (buffer == NULL) + if (!buffer) return -ENOMEM; if (condition1) { @@ -409,7 +409,36 @@ out: return result; } - Chapter 8: Commenting + Chapyer 8: Tests + +Testing return values from function calls can get complex when you need +to re-use the value later on. You should store the value before doing +any tests on it. Never assign values inside a condition to another +variable. + + err = test_something(); + if (err) { + printk(KERN_ERR "Error: test_something() failed\n"); + return err; + } + +If you give your variables and pointers good names, there is never a need +to compare the value stored in that variable to NULL or true/false, so +omit all that and keep it short. + + ptr = s->next; + if (!ptr) + return; + + v = (read_byte(register)); + if (v & mask) + return; + + if (is_prime(number)) + return 0; + + + Chapter 9: Commenting Comments are good, but there is also a danger of over-commenting. NEVER try to explain HOW your code works in a comment: it's much better to @@ -449,7 +478,7 @@ multiple data declarations). This leaves you room for a small comment on each item, explaining its use. - Chapter 9: You've made a mess of it + Chapter 10: You've made a mess of it That's OK, we all do. You've probably been told by your long-time Unix user helper that "GNU emacs" automatically formats the C sources for @@ -497,7 +526,7 @@ re-formatting you may want to take a look at the man page. But remember: "indent" is not a fix for bad programming. - Chapter 10: Configuration-files + Chapter 11: Configuration-files For configuration options (arch/xxx/Kconfig, and all the Kconfig files), somewhat different indentation is used. @@ -522,7 +551,7 @@ support for file-systems, for instance) should be denoted (DANGEROUS), other experimental options should be denoted (EXPERIMENTAL). - Chapter 11: Data structures + Chapter 12: Data structures Data structures that have visibility outside the single-threaded environment they are created and destroyed in should always have @@ -553,7 +582,7 @@ Remember: if another thread can find your data structure, and you don't have a reference count on it, you almost certainly have a bug. - Chapter 12: Macros, Enums and RTL + Chapter 13: Macros, Enums and RTL Names of macros defining constants and labels in enums are capitalized. @@ -608,7 +637,7 @@ The cpp manual deals with macros exhaustively. The gcc internals manual also covers RTL which is used frequently with assembly language in the kernel. - Chapter 13: Printing kernel messages + Chapter 14: Printing kernel messages Kernel developers like to be seen as literate. Do mind the spelling of kernel messages to make a good impression. Do not use crippled @@ -619,7 +648,7 @@ Kernel messages do not have to be terminated with a period. Printing numbers in parentheses (%d) adds no value and should be avoided. - Chapter 14: Allocating memory + Chapter 15: Allocating memory The kernel provides the following general purpose memory allocators: kmalloc(), kzalloc(), kcalloc(), and vmalloc(). Please refer to the API @@ -638,7 +667,7 @@ from void pointer to any other pointer type is guaranteed by the C programming language. - Chapter 15: The inline disease + Chapter 16: The inline disease There appears to be a common misperception that gcc has a magic "make me faster" speedup option called "inline". While the use of inlines can be @@ -665,7 +694,7 @@ appears outweighs the potential value of the hint that tells gcc to do something it would have done anyway. - Chapter 16: Function return values and names + Chapter 17: Function return values and names Functions can return values of many different kinds, and one of the most common is a value indicating whether the function succeeded or @@ -699,7 +728,7 @@ result. Typical examples would be functions that return pointers; they use NULL or the ERR_PTR mechanism to report failure. - Chapter 17: Don't re-invent the kernel macros + Chapter 18: Don't re-invent the kernel macros The header file include/linux/kernel.h contains a number of macros that you should use, rather than explicitly coding some variant of them yourself. ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] [condingstyle] Add chapter on tests 2007-05-25 17:25 ` [PATCH 2/2] [condingstyle] Add chapter on tests Auke Kok @ 2007-05-25 17:34 ` Stefan Richter 2007-05-25 17:40 ` Kok, Auke 2007-05-26 19:27 ` Jan Engelhardt 2007-05-27 16:08 ` Jesper Juhl 2 siblings, 1 reply; 19+ messages in thread From: Stefan Richter @ 2007-05-25 17:34 UTC (permalink / raw) To: Auke Kok; +Cc: randy.dunlap, linux-kernel Auke Kok wrote: > +If you give your variables and pointers good names, there is never a need > +to compare the value stored in that variable to NULL or true/false, so > +omit all that and keep it short. I agree with this in principle. But do we have to standardize it? -- Stefan Richter -=====-=-=== -=-= ==--= http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] [condingstyle] Add chapter on tests 2007-05-25 17:34 ` Stefan Richter @ 2007-05-25 17:40 ` Kok, Auke 0 siblings, 0 replies; 19+ messages in thread From: Kok, Auke @ 2007-05-25 17:40 UTC (permalink / raw) To: Stefan Richter; +Cc: randy.dunlap, linux-kernel Stefan Richter wrote: > Auke Kok wrote: >> +If you give your variables and pointers good names, there is never a need >> +to compare the value stored in that variable to NULL or true/false, so >> +omit all that and keep it short. > > I agree with this in principle. But do we have to standardize it? yes, I think so. Not only does it remove useless fluff but it forces you to pick your function and variable names decently. It really doesn't hurt to mention it especially when you see how many drivers have copied bad style over without knowing better. Now we can refer them to the CodingStyle doc right away. Auke ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] [condingstyle] Add chapter on tests 2007-05-25 17:25 ` [PATCH 2/2] [condingstyle] Add chapter on tests Auke Kok 2007-05-25 17:34 ` Stefan Richter @ 2007-05-26 19:27 ` Jan Engelhardt 2007-05-26 20:13 ` [PATCH] " Jan Engelhardt 2007-05-27 18:04 ` [PATCH 2/2] " Kok, Auke 2007-05-27 16:08 ` Jesper Juhl 2 siblings, 2 replies; 19+ messages in thread From: Jan Engelhardt @ 2007-05-26 19:27 UTC (permalink / raw) To: Auke Kok; +Cc: randy.dunlap, linux-kernel On May 25 2007 10:25, Auke Kok wrote: >diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle >index f518395..3635b38 100644 >--- a/Documentation/CodingStyle >+++ b/Documentation/CodingStyle >@@ -393,7 +393,7 @@ int fun(int a) > int result = 0; > char *buffer = kmalloc(SIZE); > >- if (buffer == NULL) >+ if (!buffer) > return -ENOMEM; Please don't do this. With ==NULL/!=NULL, it is clear what <randomvariable> could be (integer or pointer) without needing to look it up. It also reads quite strange: "if not buffer". For bools ('adjectives' / 'is a'), it works, not so much for ptrs. Hence: >+If you give your variables and pointers good names, there is never a need >+to compare the value stored in that variable to NULL or true/false, so >+omit all that and keep it short. >+ ptr = s->next; >+ if (!ptr) >+ return; Not agreed. >+ >+ v = (read_byte(register)); >+ if (v & mask) >+ return; well, yes. >+ if (is_prime(number)) Yes. And I'd also like to mention one rather special case where I'd rather like to see ==0 than ! for clarity (!strcmp looks like !streq, so one needs to look twice to get it): if (!strcmp(hay, needle)) At least don't force the '!' doctrine. Jan -- ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] [condingstyle] Add chapter on tests 2007-05-26 19:27 ` Jan Engelhardt @ 2007-05-26 20:13 ` Jan Engelhardt 2007-05-26 22:35 ` Scott Preece ` (2 more replies) 2007-05-27 18:04 ` [PATCH 2/2] " Kok, Auke 1 sibling, 3 replies; 19+ messages in thread From: Jan Engelhardt @ 2007-05-26 20:13 UTC (permalink / raw) To: Auke Kok; +Cc: randy.dunlap, Linux Kernel Mailing List Based in part on Auke's patch. Signed-off-by: Jan Engelhardt <jengelh@gmx.de> --- Documentation/CodingStyle | 74 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 10 deletions(-) Index: linux-2.6.22-rc3/Documentation/CodingStyle =================================================================== --- linux-2.6.22-rc3.orig/Documentation/CodingStyle +++ linux-2.6.22-rc3/Documentation/CodingStyle @@ -407,7 +407,61 @@ out: return result; } - Chapter 8: Commenting + Chapyer 8: Tests + +Testing return values from function calls can get complex when you need +to re-use the value later on. You should store the value before doing +any tests on it. Do not assign values inside a condition to another +variable. + + err = test_something(); + if (err) { + printk(KERN_ERR "Error: test_something() failed\n"); + return err; + } + +Testing for a flag, as done in the following example, is redundant and +can be shortened. + + if ((v & GFP_KERNEL) == GFP_KERNEL) + return; + +should become + + if (v & GFP_KERNEL) + return; + +The same goes for functions that return a bool: + + if (is_prime(number) == true) + return 0; + if (is_prime(number) == false) + return 1; + +should be: + + if (is_prime(number)) + return 0; + if (!is_prime(number)) + return 1; + +As far as pointers or functions returning an integer are concerned, +using long form tests helps to distinguish between pointers and bools +or functions returning boolean or integer, respectively. +Examples are: + + if (p == NULL) + return 1; + if (!p) + return 0; + + if (strcmp(haystack, needle) == 0) + return 1; + if (!strcmp(haystack, needle)) + return 0; + + + Chapter 9: Commenting Comments are good, but there is also a danger of over-commenting. NEVER try to explain HOW your code works in a comment: it's much better to @@ -447,7 +501,7 @@ multiple data declarations). This leave item, explaining its use. - Chapter 9: You've made a mess of it + Chapter 10: You've made a mess of it That's OK, we all do. You've probably been told by your long-time Unix user helper that "GNU emacs" automatically formats the C sources for @@ -495,7 +549,7 @@ re-formatting you may want to take a loo remember: "indent" is not a fix for bad programming. - Chapter 10: Kconfig configuration files + Chapter 11: Kconfig configuration files For all of the Kconfig* configuration files throughout the source tree, the indentation is somewhat different. Lines under a "config" definition @@ -531,7 +585,7 @@ For full documentation on the configurat Documentation/kbuild/kconfig-language.txt. - Chapter 11: Data structures + Chapter 12: Data structures Data structures that have visibility outside the single-threaded environment they are created and destroyed in should always have @@ -562,7 +616,7 @@ Remember: if another thread can find you have a reference count on it, you almost certainly have a bug. - Chapter 12: Macros, Enums and RTL + Chapter 13: Macros, Enums and RTL Names of macros defining constants and labels in enums are capitalized. @@ -617,7 +671,7 @@ The cpp manual deals with macros exhaust covers RTL which is used frequently with assembly language in the kernel. - Chapter 13: Printing kernel messages + Chapter 14: Printing kernel messages Kernel developers like to be seen as literate. Do mind the spelling of kernel messages to make a good impression. Do not use crippled @@ -628,7 +682,7 @@ Kernel messages do not have to be termin Printing numbers in parentheses (%d) adds no value and should be avoided. - Chapter 14: Allocating memory + Chapter 15: Allocating memory The kernel provides the following general purpose memory allocators: kmalloc(), kzalloc(), kcalloc(), and vmalloc(). Please refer to the API @@ -647,7 +701,7 @@ from void pointer to any other pointer t language. - Chapter 15: The inline disease + Chapter 16: The inline disease There appears to be a common misperception that gcc has a magic "make me faster" speedup option called "inline". While the use of inlines can be @@ -674,7 +728,7 @@ appears outweighs the potential value of something it would have done anyway. - Chapter 16: Function return values and names + Chapter 17: Function return values and names Functions can return values of many different kinds, and one of the most common is a value indicating whether the function succeeded or @@ -708,7 +762,7 @@ result. Typical examples would be funct NULL or the ERR_PTR mechanism to report failure. - Chapter 17: Don't re-invent the kernel macros + Chapter 18: Don't re-invent the kernel macros The header file include/linux/kernel.h contains a number of macros that you should use, rather than explicitly coding some variant of them yourself. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] [condingstyle] Add chapter on tests 2007-05-26 20:13 ` [PATCH] " Jan Engelhardt @ 2007-05-26 22:35 ` Scott Preece 2007-05-27 14:37 ` Stefan Richter 2007-05-27 21:01 ` Guennadi Liakhovetski 2 siblings, 0 replies; 19+ messages in thread From: Scott Preece @ 2007-05-26 22:35 UTC (permalink / raw) To: Jan Engelhardt; +Cc: Auke Kok, randy.dunlap, Linux Kernel Mailing List On 5/26/07, Jan Engelhardt <jengelh@linux01.gwdg.de> wrote: > > Based in part on Auke's patch. > > Signed-off-by: Jan Engelhardt <jengelh@gmx.de> > > --- > Documentation/CodingStyle | 74 +++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 64 insertions(+), 10 deletions(-) > > Index: linux-2.6.22-rc3/Documentation/CodingStyle > =================================================================== > --- linux-2.6.22-rc3.orig/Documentation/CodingStyle > +++ linux-2.6.22-rc3/Documentation/CodingStyle > @@ -407,7 +407,61 @@ out: > return result; > } > > - Chapter 8: Commenting > + Chapyer 8: Tests --- Fix the "Chapyer" spelling above. However, I think this set of rules is not universally agreed to and probably should not be added at this time. I'm not convinced that it makes the code more readable. scott ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] [condingstyle] Add chapter on tests 2007-05-26 20:13 ` [PATCH] " Jan Engelhardt 2007-05-26 22:35 ` Scott Preece @ 2007-05-27 14:37 ` Stefan Richter 2007-05-27 15:02 ` Jan Engelhardt 2007-05-27 21:01 ` Guennadi Liakhovetski 2 siblings, 1 reply; 19+ messages in thread From: Stefan Richter @ 2007-05-27 14:37 UTC (permalink / raw) To: Jan Engelhardt Cc: Auke Kok, randy.dunlap, Linux Kernel Mailing List, Scott Preece Jan Engelhardt wrote: > + if (is_prime(number) == true) > + return 0; > + if (is_prime(number) == false) > + return 1; > + > +should be: > + > + if (is_prime(number)) > + return 0; > + if (!is_prime(number)) > + return 1; > + > +As far as pointers or functions returning an integer are concerned, > +using long form tests helps to distinguish between pointers and bools > +or functions returning boolean or integer, respectively. > +Examples are: > + > + if (p == NULL) > + return 1; > + if (!p) > + return 0; > + > + if (strcmp(haystack, needle) == 0) > + return 1; > + if (!strcmp(haystack, needle)) > + return 0; The latter two examples seem odd. Didn't you mean the following? if (p == NULL) return 1; if (p) return 0; if (strcmp(haystack, needle) == 0) return 1; if (strcmp(haystack, needle)) return 0; Perhaps better: if (p == NULL) return NO_MEMORY; if (p) return MEMORY; if (strcmp(haystack, needle) == 0) return IS_SAME; if (strcmp(haystack, needle)) return IS_DIFFERENT; However, to follow your argument about non-boolean expressions, the following would be more consequently going into your direction: if (p == NULL) return NO_MEMORY; if (p != NULL) return MEMORY; if (strcmp(haystack, needle) == 0) return IS_SAME; if (strcmp(haystack, needle) != 0) return IS_DIFFERENT; I.e., why do the explicit comparison with 0 or NULL only when it is tested for equality, but not when testing for inequality? However, I agree with Scott Preece that these rules should be left out of CodingStyle because they are contentious. (Disclosure: I am personally used to "if (p)" and "if (!p)" tests of pointers and many integer expressions, but I tend to the longer form in less obvious cases like "if (strcmp(a, b) != 0)".) -- Stefan Richter -=====-=-=== -=-= ==-== http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] [condingstyle] Add chapter on tests 2007-05-27 14:37 ` Stefan Richter @ 2007-05-27 15:02 ` Jan Engelhardt 2007-05-27 15:23 ` Stefan Richter 0 siblings, 1 reply; 19+ messages in thread From: Jan Engelhardt @ 2007-05-27 15:02 UTC (permalink / raw) To: Stefan Richter Cc: Auke Kok, randy.dunlap, Linux Kernel Mailing List, Scott Preece On May 27 2007 16:37, Stefan Richter wrote: >Jan Engelhardt wrote: >> + if (is_prime(number) == true) >> + return 0; >> + if (is_prime(number) == false) >> + return 1; >> + >> +should be: >> + >> + if (is_prime(number)) >> + return 0; >> + if (!is_prime(number)) >> + return 1; >> + >> +As far as pointers or functions returning an integer are concerned, >> +using long form tests helps to distinguish between pointers and bools >> +or functions returning boolean or integer, respectively. >> +Examples are: >> + >> + if (p == NULL) >> + return 1; >> + if (!p) >> + return 0; >> + >> + if (strcmp(haystack, needle) == 0) >> + return 1; >> + if (!strcmp(haystack, needle)) >> + return 0; > >The latter two examples seem odd. Didn't you mean the following? See how much confusion it all makes! Right, it was intended -- first the long form is shown and then the shorter one (and "long form tests help to distinguish"): if (p == NULL) /* this way please */ return 1; if (!p) /* Everytime you shorten it, God kills a kitten */ return 0; /* so perhaps don't do it if you love animals or know someone who does. */ I seem to have forgotten more comments/explanation. > if (p == NULL) > return 1; > if (p) > return 0; > > if (strcmp(haystack, needle) == 0) > return 1; > if (strcmp(haystack, needle)) > return 0; > >Perhaps better: > > if (p == NULL) > return NO_MEMORY; > if (p) > return MEMORY; > > if (strcmp(haystack, needle) == 0) > return IS_SAME; > if (strcmp(haystack, needle)) > return IS_DIFFERENT; > >However, to follow your argument about non-boolean expressions, the >following would be more consequently going into your direction: > >I.e., why do the explicit comparison with 0 or NULL only when it is >tested for equality, but not when testing for inequality? > >However, I agree with Scott Preece that these rules should be left out >of CodingStyle because they are contentious. > >(Disclosure: I am personally used to "if (p)" and "if (!p)" tests of >pointers and many integer expressions, but I tend to the longer form in >less obvious cases like "if (strcmp(a, b) != 0)".) Jan -- ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] [condingstyle] Add chapter on tests 2007-05-27 15:02 ` Jan Engelhardt @ 2007-05-27 15:23 ` Stefan Richter 2007-05-27 15:52 ` Jan Engelhardt 0 siblings, 1 reply; 19+ messages in thread From: Stefan Richter @ 2007-05-27 15:23 UTC (permalink / raw) To: Jan Engelhardt Cc: Auke Kok, randy.dunlap, Linux Kernel Mailing List, Scott Preece Jan Engelhardt wrote: > if (!p) /* Everytime you shorten it, God kills a kitten */ Very bad news for felines, judging from current coding practice. -- Stefan Richter -=====-=-=== -=-= ==-== http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] [condingstyle] Add chapter on tests 2007-05-27 15:23 ` Stefan Richter @ 2007-05-27 15:52 ` Jan Engelhardt 0 siblings, 0 replies; 19+ messages in thread From: Jan Engelhardt @ 2007-05-27 15:52 UTC (permalink / raw) To: Stefan Richter Cc: Auke Kok, randy.dunlap, Linux Kernel Mailing List, Scott Preece On May 27 2007 17:23, Stefan Richter wrote: >Jan Engelhardt wrote: >> if (!p) /* Everytime you shorten it, God kills a kitten */ > >Very bad news for felines, judging from current coding practice. Don't worry. There are far worse codingstyles around, like the GNU 'standard'. (Indent is the premier element of any style.) Jan -- ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] [condingstyle] Add chapter on tests 2007-05-26 20:13 ` [PATCH] " Jan Engelhardt 2007-05-26 22:35 ` Scott Preece 2007-05-27 14:37 ` Stefan Richter @ 2007-05-27 21:01 ` Guennadi Liakhovetski 2 siblings, 0 replies; 19+ messages in thread From: Guennadi Liakhovetski @ 2007-05-27 21:01 UTC (permalink / raw) To: Jan Engelhardt; +Cc: Auke Kok, randy.dunlap, Linux Kernel Mailing List On Sat, 26 May 2007, Jan Engelhardt wrote: > +Testing for a flag, as done in the following example, is redundant and > +can be shortened. > + > + if ((v & GFP_KERNEL) == GFP_KERNEL) > + return; > + > +should become > + > + if (v & GFP_KERNEL) > + return; This looks wrong to me. These two are only equivalent if the "flag" only has 1 bit. And already here you fall into this trap: #define GFP_KERNEL (__GFP_WAIT | __GFP_IO | __GFP_FS) Thanks Guennadi --- Guennadi Liakhovetski ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] [condingstyle] Add chapter on tests 2007-05-26 19:27 ` Jan Engelhardt 2007-05-26 20:13 ` [PATCH] " Jan Engelhardt @ 2007-05-27 18:04 ` Kok, Auke 2007-05-27 20:17 ` Pekka Enberg 1 sibling, 1 reply; 19+ messages in thread From: Kok, Auke @ 2007-05-27 18:04 UTC (permalink / raw) To: Jan Engelhardt; +Cc: randy.dunlap, linux-kernel Jan Engelhardt wrote: > On May 25 2007 10:25, Auke Kok wrote: >> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle >> index f518395..3635b38 100644 >> --- a/Documentation/CodingStyle >> +++ b/Documentation/CodingStyle >> @@ -393,7 +393,7 @@ int fun(int a) >> int result = 0; >> char *buffer = kmalloc(SIZE); >> >> - if (buffer == NULL) >> + if (!buffer) >> return -ENOMEM; > > Please don't do this. With ==NULL/!=NULL, it is clear what > <randomvariable> could be (integer or pointer) without needing > to look it up. It also reads quite strange: "if not buffer". > For bools ('adjectives' / 'is a'), it works, not so much for ptrs. > Hence: > >> +If you give your variables and pointers good names, there is never a need >> +to compare the value stored in that variable to NULL or true/false, so >> +omit all that and keep it short. > >> + ptr = s->next; >> + if (!ptr) >> + return; > > Not agreed. that piece is a copy of mm/slab.c, and all over the core components of the kernel (even fs/inode.c written by Linus). I strongly think that "== NULL" doesn't add anything and that well-written functions and well-named variables really do not need the extra fluff. Auke ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] [condingstyle] Add chapter on tests 2007-05-27 18:04 ` [PATCH 2/2] " Kok, Auke @ 2007-05-27 20:17 ` Pekka Enberg 0 siblings, 0 replies; 19+ messages in thread From: Pekka Enberg @ 2007-05-27 20:17 UTC (permalink / raw) To: Kok, Auke; +Cc: Jan Engelhardt, randy.dunlap, linux-kernel On 5/27/07, Kok, Auke <auke-jan.h.kok@intel.com> wrote: > that piece is a copy of mm/slab.c, and all over the core components of the > kernel (even fs/inode.c written by Linus). I strongly think that "== NULL" > doesn't add anything and that well-written functions and well-named variables > really do not need the extra fluff. Yup, I we really don't use "== NULL" in core code that much. But I am not convinced this should be in CodingStyle either. It's more of a maintainer preference thing above all. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] [condingstyle] Add chapter on tests 2007-05-25 17:25 ` [PATCH 2/2] [condingstyle] Add chapter on tests Auke Kok 2007-05-25 17:34 ` Stefan Richter 2007-05-26 19:27 ` Jan Engelhardt @ 2007-05-27 16:08 ` Jesper Juhl 2 siblings, 0 replies; 19+ messages in thread From: Jesper Juhl @ 2007-05-27 16:08 UTC (permalink / raw) To: Auke Kok; +Cc: randy.dunlap, linux-kernel On 25/05/07, Auke Kok <auke-jan.h.kok@intel.com> wrote: > Several standards have been established on how to format tests and use > NULL/false/true tests. > Hmm, this may or may not be a good idea for CodingStyle, I'll leave that up to others. But, if you are going to renumber chapters then please also fix up the references to those chapter numbers elsewhere in the file - like this one "... (for example as a means of replacing macros, see Chapter 12) ..." After your patch that now points to the wrong chapter. -- Jesper Juhl <jesper.juhl@gmail.com> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] [CodingStyle] Add comment to not put spaces before tabs. 2007-05-25 17:25 [PATCH 1/2] [CodingStyle] Add comment to not put spaces before tabs Auke Kok 2007-05-25 17:25 ` [PATCH 2/2] [condingstyle] Add chapter on tests Auke Kok @ 2007-05-25 17:32 ` Stefan Richter 2007-05-25 19:43 ` H. Peter Anvin 1 sibling, 1 reply; 19+ messages in thread From: Stefan Richter @ 2007-05-25 17:32 UTC (permalink / raw) To: Auke Kok; +Cc: randy.dunlap, linux-kernel Auke Kok wrote: > +Don't put spaces before tabs or mix them. Make it "Don't put spaces before tabs." We do mix them if we combine tabs for indentation with spaces for alignment. -- Stefan Richter -=====-=-=== -=-= ==--= http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] [CodingStyle] Add comment to not put spaces before tabs. 2007-05-25 17:32 ` [PATCH 1/2] [CodingStyle] Add comment to not put spaces before tabs Stefan Richter @ 2007-05-25 19:43 ` H. Peter Anvin 2007-05-25 20:29 ` Kok, Auke 0 siblings, 1 reply; 19+ messages in thread From: H. Peter Anvin @ 2007-05-25 19:43 UTC (permalink / raw) To: Stefan Richter; +Cc: Auke Kok, randy.dunlap, linux-kernel Stefan Richter wrote: > Auke Kok wrote: >> +Don't put spaces before tabs or mix them. > > Make it "Don't put spaces before tabs." > > We do mix them if we combine tabs for indentation with spaces for alignment. It would probably be good to add a pointer to the cleanfile/cleanpatch scripts here, too. -hpa ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] [CodingStyle] Add comment to not put spaces before tabs. 2007-05-25 19:43 ` H. Peter Anvin @ 2007-05-25 20:29 ` Kok, Auke 2007-05-25 21:26 ` H. Peter Anvin 0 siblings, 1 reply; 19+ messages in thread From: Kok, Auke @ 2007-05-25 20:29 UTC (permalink / raw) To: H. Peter Anvin; +Cc: Stefan Richter, randy.dunlap, linux-kernel H. Peter Anvin wrote: > Stefan Richter wrote: >> Auke Kok wrote: >>> +Don't put spaces before tabs or mix them. >> Make it "Don't put spaces before tabs." >> >> We do mix them if we combine tabs for indentation with spaces for alignment. > > It would probably be good to add a pointer to the cleanfile/cleanpatch > scripts here, too. ahh yes, I'll incorporate that. BTW, would it be possible for cleanfile/cleanpatch to dump warnings to stderr for lines exceeding 80 characters? I think that would really be useful... Auke ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] [CodingStyle] Add comment to not put spaces before tabs. 2007-05-25 20:29 ` Kok, Auke @ 2007-05-25 21:26 ` H. Peter Anvin 0 siblings, 0 replies; 19+ messages in thread From: H. Peter Anvin @ 2007-05-25 21:26 UTC (permalink / raw) To: Kok, Auke; +Cc: Stefan Richter, randy.dunlap, linux-kernel Kok, Auke wrote: > > ahh yes, I'll incorporate that. > > BTW, would it be possible for cleanfile/cleanpatch to dump warnings to > stderr for lines exceeding 80 characters? I think that would really be > useful... > Should be trivial enough to do, although it probably should be an option. -hpa ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2007-05-27 21:01 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-05-25 17:25 [PATCH 1/2] [CodingStyle] Add comment to not put spaces before tabs Auke Kok 2007-05-25 17:25 ` [PATCH 2/2] [condingstyle] Add chapter on tests Auke Kok 2007-05-25 17:34 ` Stefan Richter 2007-05-25 17:40 ` Kok, Auke 2007-05-26 19:27 ` Jan Engelhardt 2007-05-26 20:13 ` [PATCH] " Jan Engelhardt 2007-05-26 22:35 ` Scott Preece 2007-05-27 14:37 ` Stefan Richter 2007-05-27 15:02 ` Jan Engelhardt 2007-05-27 15:23 ` Stefan Richter 2007-05-27 15:52 ` Jan Engelhardt 2007-05-27 21:01 ` Guennadi Liakhovetski 2007-05-27 18:04 ` [PATCH 2/2] " Kok, Auke 2007-05-27 20:17 ` Pekka Enberg 2007-05-27 16:08 ` Jesper Juhl 2007-05-25 17:32 ` [PATCH 1/2] [CodingStyle] Add comment to not put spaces before tabs Stefan Richter 2007-05-25 19:43 ` H. Peter Anvin 2007-05-25 20:29 ` Kok, Auke 2007-05-25 21:26 ` H. Peter Anvin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox