Re: [PATCH 2/2] [condingstyle] Add chapter on tests
On 5/27/07, Kok, Auke <[EMAIL PROTECTED]> 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. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] [condingstyle] Add chapter on tests
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
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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] [condingstyle] Add chapter on tests
On 25/05/07, Auke Kok <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] [condingstyle] Add chapter on tests
On 25/05/07, Auke Kok [EMAIL PROTECTED] 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 [EMAIL PROTECTED] Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html Plain text mails only, please http://www.expita.com/nomime.html - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] [condingstyle] Add chapter on tests
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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] [condingstyle] Add chapter on tests
On 5/27/07, Kok, Auke [EMAIL PROTECTED] 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. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] [condingstyle] Add chapter on tests
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
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
--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] [condingstyle] Add chapter on tests
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
--
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] [condingstyle] Add chapter on tests
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] [condingstyle] Add chapter on tests
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/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] [condingstyle] Add chapter on tests
Several standards have been established on how to format tests and use
NULL/false/true tests.
Signed-off-by: Auke Kok <[EMAIL PROTECTED]>
---
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
+
[PATCH 2/2] [condingstyle] Add chapter on tests
Several standards have been established on how to format tests and use
NULL/false/true tests.
Signed-off-by: Auke Kok [EMAIL PROTECTED]
---
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: [PATCH 2/2] [condingstyle] Add chapter on tests
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] [condingstyle] Add chapter on tests
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/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
