[PATCH] CodingStyle: add typedefs chapter

[PATCH] CodingStyle: add typedefs chapter

[Randy.Dunlap]

From: Randy Dunlap <rdunlap@xenotime.net>

Add a chapter on typedefs, copied from an email from Linus
to lkml on Feb. 3, 2006.
(Subject: Re: [RFC][PATCH 1/5] Virtualization/containers: startup)

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 Documentation/CodingStyle |   77 ++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 65 insertions(+), 12 deletions(-)

--- linux-2617-rc3.orig/Documentation/CodingStyle
+++ linux-2617-rc3/Documentation/CodingStyle
@@ -155,7 +155,60 @@ problem, which is called the function-gr
 See next chapter.
 
 
-		Chapter 5: Functions
+		Chapter 5: Typedefs
+
+Please don't use things like "vps_t".
+
+It's a _mistake_ to use typedef for structures and pointers. When you see a
+
+	vps_t a;
+
+in the source, what does it mean?
+
+In contrast, if it says
+
+	struct virtual_container *a;
+
+you can actually tell what "a" is.
+
+Lots of people think that typedefs "help readability". Not so. They are
+useful only for:
+
+ (a) totally opaque objects (where the typedef is actively used to _hide_
+     what the object is).
+
+     Example: "pte_t" etc. opaque objects that you can only access using
+     the proper accessor functions.
+
+     NOTE! Opaqueness and "accessor functions" are not good in themselves.
+     The reason we have them for things like pte_t etc. is that there
+     really is absolutely _zero_ portably accessible information there.
+
+ (b) Clear integer types, where the abstraction _helps_ avoid confusion
+     whether it is "int" or "long".
+
+     u8/u16/u32 are perfectly fine typedefs.
+
+     NOTE! Again - there needs to be a _reason_ for this. If something is
+     "unsigned long", then there's no reason to do
+
+	typedef long myflags_t;
+
+     but if there is a clear reason for why it under certain circumstances
+     might be an "unsigned int" and under other configurations might be
+     "unsigned long", then by all means go ahead and use a typedef.
+
+ (c) when you use sparse to literally create a _new_ type for
+     type-checking.
+
+Maybe there are other cases too, but the rule should basically be to NEVER
+EVER use a typedef unless you can clearly match one of those rules.
+
+In general, a pointer, or a struct that has elements that can reasonably
+be directly accessed should _never_ be a typedef.
+
+
+		Chapter 6: Functions
 
 Functions should be short and sweet, and do just one thing.  They should
 fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
@@ -183,7 +236,7 @@ and it gets confused.  You know you're b
 to understand what you did 2 weeks from now.
 
 
-		Chapter 6: Centralized exiting of functions
+		Chapter 7: Centralized exiting of functions
 
 Albeit deprecated by some people, the equivalent of the goto statement is
 used frequently by compilers in form of the unconditional jump instruction.
@@ -220,7 +273,7 @@ out:
 	return result;
 }
 
-		Chapter 7: Commenting
+		Chapter 8: 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
@@ -240,7 +293,7 @@ When commenting the kernel API functions
 See the files Documentation/kernel-doc-nano-HOWTO.txt and scripts/kernel-doc
 for details.
 
-		Chapter 8: You've made a mess of it
+		Chapter 9: 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
@@ -288,7 +341,7 @@ re-formatting you may want to take a loo
 remember: "indent" is not a fix for bad programming.
 
 
-		Chapter 9: Configuration-files
+		Chapter 10: Configuration-files
 
 For configuration options (arch/xxx/Kconfig, and all the Kconfig files),
 somewhat different indentation is used.
@@ -313,7 +366,7 @@ support for file-systems, for instance) 
 experimental options should be denoted (EXPERIMENTAL).
 
 
-		Chapter 10: Data structures
+		Chapter 11: Data structures
 
 Data structures that have visibility outside the single-threaded
 environment they are created and destroyed in should always have
@@ -344,7 +397,7 @@ Remember: if another thread can find you
 have a reference count on it, you almost certainly have a bug.
 
 
-		Chapter 11: Macros, Enums and RTL
+		Chapter 12: Macros, Enums and RTL
 
 Names of macros defining constants and labels in enums are capitalized.
 
@@ -399,7 +452,7 @@ The cpp manual deals with macros exhaust
 covers RTL which is used frequently with assembly language in the kernel.
 
 
-		Chapter 12: Printing kernel messages
+		Chapter 13: 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
@@ -410,7 +463,7 @@ Kernel messages do not have to be termin
 Printing numbers in parentheses (%d) adds no value and should be avoided.
 
 
-		Chapter 13: Allocating memory
+		Chapter 14: Allocating memory
 
 The kernel provides the following general purpose memory allocators:
 kmalloc(), kzalloc(), kcalloc(), and vmalloc().  Please refer to the API
@@ -429,7 +482,7 @@ from void pointer to any other pointer t
 language.
 
 
-		Chapter 14: The inline disease
+		Chapter 15: 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
@@ -457,7 +510,7 @@ something it would have done anyway.
 
 
 
-		Chapter 15: References
+		Appendix I: References
 
 The C Programming Language, Second Edition
 by Brian W. Kernighan and Dennis M. Ritchie.
@@ -481,4 +534,4 @@ Kernel CodingStyle, by greg@kroah.com at
 http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_talk/html/
 
 --
-Last updated on 30 December 2005 by a community effort on LKML.
+Last updated on 30 April 2006.


---

Re: [PATCH] CodingStyle: add typedefs chapter

[Michael Buesch]

[-- Attachment #1: Type: text/plain, Size: 271 bytes --]

On Monday 01 May 2006 02:44, you wrote:
> +     NOTE! Again - there needs to be a _reason_ for this. If something is
> +     "unsigned long", then there's no reason to do
> +
> +	typedef long myflags_t;

typedef unsigned long myflags_t;

-- 
Greetings Michael.

[-- Attachment #2: Type: application/pgp-signature, Size: 191 bytes --]

Re: [PATCH] CodingStyle: add typedefs chapter

[Jan Engelhardt]

>-		Chapter 5: Functions
>+		Chapter 5: Typedefs
>+
>+Please don't use things like "vps_t".
>+It's a _mistake_ to use typedef for structures and pointers. When you see a
>+	vps_t a;
>+in the source, what does it mean?
>+In contrast, if it says
>+	struct virtual_container *a;
>+you can actually tell what "a" is.
>+
>+Lots of people think that typedefs "help readability". Not so. They are
>+useful only for:
[...]

What about task_t vs struct task_struct? Both are used in the kernel.



Jan Engelhardt
-- 

Re: [PATCH] CodingStyle: add typedefs chapter

[Alexey Dobriyan]

On Mon, May 01, 2006 at 04:00:09PM +0200, Jan Engelhardt wrote:
> >+Please don't use things like "vps_t".
> >+It's a _mistake_ to use typedef for structures and pointers. When you see a
> >+	vps_t a;
> >+in the source, what does it mean?
> >+In contrast, if it says
> >+	struct virtual_container *a;
> >+you can actually tell what "a" is.
> >+
> >+Lots of people think that typedefs "help readability". Not so. They are
> >+useful only for:
> [...]
>
> What about task_t vs struct task_struct? Both are used in the kernel.

task_t			=> struct task
struct task_struct	=> struct task

Roughly 2765 hits :-\

Re: [PATCH] CodingStyle: add typedefs chapter

[linux-os (Dick Johnson)]

[-- Attachment #1: Type: text/plain, Size: 3045 bytes --]


On Mon, 1 May 2006, Alexey Dobriyan wrote:

> On Mon, May 01, 2006 at 04:00:09PM +0200, Jan Engelhardt wrote:
>>> +Please don't use things like "vps_t".
>>> +It's a _mistake_ to use typedef for structures and pointers. When you see a
>>> +	vps_t a;
>>> +in the source, what does it mean?
>>> +In contrast, if it says
>>> +	struct virtual_container *a;
>>> +you can actually tell what "a" is.
>>> +
>>> +Lots of people think that typedefs "help readability". Not so. They are
>>> +useful only for:
>> [...]
>>
>> What about task_t vs struct task_struct? Both are used in the kernel.
>
> task_t			=> struct task
> struct task_struct	=> struct task
>
> Roughly 2765 hits :-\

Yes, also 'current' is probably the most used. Any, since this
has become a FAQ, maybe it's about time to put something in the
Documentation?

--- /usr/src/linux-2.6.16.4/Documentation/CodingStyle.orig	2006-05-01 10:17:03.000000000 -0400
+++ /usr/src/linux-2.6.16.4/Documentation/CodingStyle	2006-05-01 10:37:09.000000000 -0400
@@ -343,6 +343,33 @@
  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.

+	typedefs and and structs
+
+Typedefs should never be used for information hiding. In other words,
+if a typedef defines an aggregate type, and the individual components
+are accessed anywhere in the code, a typedef should not be used.
+
+An example of proper usage:
+typedef struct opaque_type FILE;	// In a header
+
+	FILE *fp;			// In a program block
+
+The type 'FILE' in this example is something that was defined in
+a 'C' runtime library. The code uses pointers to this opaque type,
+but never even knows, and doesn't care, what's inside that structure.
+Therefore, FILE can have been defined using a typedef.
+
+An example of incorrect usage:
+
+typedef struct file_operations FO;	// In a header
+
+	FO	fo;			// In a program block
+	memset(&foo, 0x00, sizeof(foo));
+
+In this case, object FO contains structure members that will be
+accessed by the code. It should not have been defined. Instead,
+the structure name should have been used directly.
+

  		Chapter 11: Macros, Enums and RTL



In case the M$ server mangles the patch, it's included as an attachment.


Cheers,
Dick Johnson
Penguin : Linux version 2.6.16.4 on an i686 machine (5592.89 BogoMips).
New book: http://www.lymanschool.com
_
\x1a\x04


****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

[-- Attachment #2: style.patch --]
[-- Type: TEXT/PLAIN, Size: 1383 bytes --]

--- /usr/src/linux-2.6.16.4/Documentation/CodingStyle.orig	2006-05-01 10:17:03.000000000 -0400
+++ /usr/src/linux-2.6.16.4/Documentation/CodingStyle	2006-05-01 10:37:09.000000000 -0400
@@ -343,6 +343,33 @@
 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.
 
+	typedefs and and structs
+
+Typedefs should never be used for information hiding. In other words,
+if a typedef defines an aggregate type, and the individual components
+are accessed anywhere in the code, a typedef should not be used.
+
+An example of proper usage:
+typedef struct opaque_type FILE;	// In a header
+
+	FILE *fp;			// In a program block
+
+The type 'FILE' in this example is something that was defined in
+a 'C' runtime library. The code uses pointers to this opaque type,
+but never even knows, and doesn't care, what's inside that structure.
+Therefore, FILE can have been defined using a typedef.
+
+An example of incorrect usage:
+
+typedef struct file_operations FO;	// In a header
+
+	FO	fo;			// In a program block
+	memset(&foo, 0x00, sizeof(foo));
+
+In this case, object FO contains structure members that will be
+accessed by the code. It should not have been defined. Instead,
+the structure name should have been used directly.
+
 
 		Chapter 11: Macros, Enums and RTL
 

Re: [PATCH] CodingStyle: add typedefs chapter

[Jiri Slaby]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

linux-os (Dick Johnson) napsal(a):
> On Mon, 1 May 2006, Alexey Dobriyan wrote:
> 
>> On Mon, May 01, 2006 at 04:00:09PM +0200, Jan Engelhardt wrote:
>>>> +Please don't use things like "vps_t".
>>>> +It's a _mistake_ to use typedef for structures and pointers. When you see a
>>>> +	vps_t a;
>>>> +in the source, what does it mean?
>>>> +In contrast, if it says
>>>> +	struct virtual_container *a;
>>>> +you can actually tell what "a" is.
>>>> +
>>>> +Lots of people think that typedefs "help readability". Not so. They are
>>>> +useful only for:
>>> [...]
>>>
>>> What about task_t vs struct task_struct? Both are used in the kernel.
>> task_t			=> struct task
>> struct task_struct	=> struct task
>>
>> Roughly 2765 hits :-\
> 
> Yes, also 'current' is probably the most used. Any, since this
> has become a FAQ, maybe it's about time to put something in the
> Documentation?
> 
> --- /usr/src/linux-2.6.16.4/Documentation/CodingStyle.orig	2006-05-01 10:17:03.000000000 -0400
> +++ /usr/src/linux-2.6.16.4/Documentation/CodingStyle	2006-05-01 10:37:09.000000000 -0400
> @@ -343,6 +343,33 @@
>   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.
> 
> +	typedefs and and structs
> +
> +Typedefs should never be used for information hiding. In other words,
> +if a typedef defines an aggregate type, and the individual components
> +are accessed anywhere in the code, a typedef should not be used.
> +
> +An example of proper usage:
> +typedef struct opaque_type FILE;	// In a header
> +
> +	FILE *fp;			// In a program block
> +
> +The type 'FILE' in this example is something that was defined in
> +a 'C' runtime library. The code uses pointers to this opaque type,
> +but never even knows, and doesn't care, what's inside that structure.
> +Therefore, FILE can have been defined using a typedef.
> +
> +An example of incorrect usage:
> +
> +typedef struct file_operations FO;	// In a header
> +
> +	FO	fo;			// In a program block
> +	memset(&foo, 0x00, sizeof(foo));
> +
> +In this case, object FO contains structure members that will be
> +accessed by the code. It should not have been defined. Instead,
> +the structure name should have been used directly.
> +
> 
>   		Chapter 11: Macros, Enums and RTL
> 
> 
> 
> In case the M$ server mangles the patch, it's included as an attachment.
> 
> 
> Cheers,
> Dick Johnson
> Penguin : Linux version 2.6.16.4 on an i686 machine (5592.89 BogoMips).
> New book: http://www.lymanschool.com
> _
> 
> 
> 
> ****************************************************************
> The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.
> 
> Thank you.
> 
> 
> ------------------------------------------------------------------------
> 
> --- /usr/src/linux-2.6.16.4/Documentation/CodingStyle.orig	2006-05-01 10:17:03.000000000 -0400
> +++ /usr/src/linux-2.6.16.4/Documentation/CodingStyle	2006-05-01 10:37:09.000000000 -0400
> @@ -343,6 +343,33 @@
>  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.
>  
> +	typedefs and and structs
> +
> +Typedefs should never be used for information hiding. In other words,
> +if a typedef defines an aggregate type, and the individual components
> +are accessed anywhere in the code, a typedef should not be used.
> +
> +An example of proper usage:
> +typedef struct opaque_type FILE;	// In a header
> +
> +	FILE *fp;			// In a program block
> +
> +The type 'FILE' in this example is something that was defined in
> +a 'C' runtime library. The code uses pointers to this opaque type,
> +but never even knows, and doesn't care, what's inside that structure.
> +Therefore, FILE can have been defined using a typedef.
> +
> +An example of incorrect usage:
> +
> +typedef struct file_operations FO;	// In a header
> +
> +	FO	fo;			// In a program block
You meant 'FO foo', didn't you?
> +	memset(&foo, 0x00, sizeof(foo));
> +
> +In this case, object FO contains structure members that will be
> +accessed by the code. It should not have been defined. Instead,
> +the structure name should have been used directly.
> +
>  
>  		Chapter 11: Macros, Enums and RTL
>  

regards,
- --
Jiri Slaby         www.fi.muni.cz/~xslaby
\_.-^-._   jirislaby@gmail.com   _.-^-._/
B67499670407CE62ACC8 22A032CC55C339D47A7E
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFEViK1MsxVwznUen4RAi8wAJ9km1HybAI8qRUR5IY9y52+LDHdogCfSy4O
ENhfqjTKF/+zdyjmuV7wXws=
=yZTA
-----END PGP SIGNATURE-----

Re: [PATCH] CodingStyle: add typedefs chapter

[Artem B. Bityutskiy]

Jan Engelhardt wrote:
> What about task_t vs struct task_struct? Both are used in the kernel.

If you use task_struct's fields, pick struct task_struct, otherwise pick 
task_t.

-- 
Best regards, Artem B. Bityutskiy
Oktet Labs (St. Petersburg), Software Engineer.
+7 812 4286709 (office) +7 911 2449030 (mobile)
E-mail: dedekind@oktetlabs.ru, Web: www.oktetlabs.ru

Re: [PATCH] CodingStyle: add typedefs chapter

[Randy.Dunlap]

On Mon, 1 May 2006 09:28:33 +0200 Michael Buesch wrote:

> On Monday 01 May 2006 02:44, you wrote:
> > +     NOTE! Again - there needs to be a _reason_ for this. If something is
> > +     "unsigned long", then there's no reason to do
> > +
> > +	typedef long myflags_t;
> 
> typedef unsigned long myflags_t;

You mean that Linus's original email contained a thinko?

Fixed, thanks.

---
From: Randy Dunlap <rdunlap@xenotime.net>

Add a chapter on typedefs, copied from an email from Linus
to lkml on Feb. 3, 2006.
(Subject: Re: [RFC][PATCH 1/5] Virtualization/containers: startup)

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 Documentation/CodingStyle |   77 ++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 65 insertions(+), 12 deletions(-)

--- linux-2617-rc3.orig/Documentation/CodingStyle
+++ linux-2617-rc3/Documentation/CodingStyle
@@ -155,7 +155,60 @@ problem, which is called the function-gr
 See next chapter.
 
 
-		Chapter 5: Functions
+		Chapter 5: Typedefs
+
+Please don't use things like "vps_t".
+
+It's a _mistake_ to use typedef for structures and pointers. When you see a
+
+	vps_t a;
+
+in the source, what does it mean?
+
+In contrast, if it says
+
+	struct virtual_container *a;
+
+you can actually tell what "a" is.
+
+Lots of people think that typedefs "help readability". Not so. They are
+useful only for:
+
+ (a) totally opaque objects (where the typedef is actively used to _hide_
+     what the object is).
+
+     Example: "pte_t" etc. opaque objects that you can only access using
+     the proper accessor functions.
+
+     NOTE! Opaqueness and "accessor functions" are not good in themselves.
+     The reason we have them for things like pte_t etc. is that there
+     really is absolutely _zero_ portably accessible information there.
+
+ (b) Clear integer types, where the abstraction _helps_ avoid confusion
+     whether it is "int" or "long".
+
+     u8/u16/u32 are perfectly fine typedefs.
+
+     NOTE! Again - there needs to be a _reason_ for this. If something is
+     "unsigned long", then there's no reason to do
+
+	typedef unsigned long myflags_t;
+
+     but if there is a clear reason for why it under certain circumstances
+     might be an "unsigned int" and under other configurations might be
+     "unsigned long", then by all means go ahead and use a typedef.
+
+ (c) when you use sparse to literally create a _new_ type for
+     type-checking.
+
+Maybe there are other cases too, but the rule should basically be to NEVER
+EVER use a typedef unless you can clearly match one of those rules.
+
+In general, a pointer, or a struct that has elements that can reasonably
+be directly accessed should _never_ be a typedef.
+
+
+		Chapter 6: Functions
 
 Functions should be short and sweet, and do just one thing.  They should
 fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
@@ -183,7 +236,7 @@ and it gets confused.  You know you're b
 to understand what you did 2 weeks from now.
 
 
-		Chapter 6: Centralized exiting of functions
+		Chapter 7: Centralized exiting of functions
 
 Albeit deprecated by some people, the equivalent of the goto statement is
 used frequently by compilers in form of the unconditional jump instruction.
@@ -220,7 +273,7 @@ out:
 	return result;
 }
 
-		Chapter 7: Commenting
+		Chapter 8: 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
@@ -240,7 +293,7 @@ When commenting the kernel API functions
 See the files Documentation/kernel-doc-nano-HOWTO.txt and scripts/kernel-doc
 for details.
 
-		Chapter 8: You've made a mess of it
+		Chapter 9: 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
@@ -288,7 +341,7 @@ re-formatting you may want to take a loo
 remember: "indent" is not a fix for bad programming.
 
 
-		Chapter 9: Configuration-files
+		Chapter 10: Configuration-files
 
 For configuration options (arch/xxx/Kconfig, and all the Kconfig files),
 somewhat different indentation is used.
@@ -313,7 +366,7 @@ support for file-systems, for instance) 
 experimental options should be denoted (EXPERIMENTAL).
 
 
-		Chapter 10: Data structures
+		Chapter 11: Data structures
 
 Data structures that have visibility outside the single-threaded
 environment they are created and destroyed in should always have
@@ -344,7 +397,7 @@ Remember: if another thread can find you
 have a reference count on it, you almost certainly have a bug.
 
 
-		Chapter 11: Macros, Enums and RTL
+		Chapter 12: Macros, Enums and RTL
 
 Names of macros defining constants and labels in enums are capitalized.
 
@@ -399,7 +452,7 @@ The cpp manual deals with macros exhaust
 covers RTL which is used frequently with assembly language in the kernel.
 
 
-		Chapter 12: Printing kernel messages
+		Chapter 13: 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
@@ -410,7 +463,7 @@ Kernel messages do not have to be termin
 Printing numbers in parentheses (%d) adds no value and should be avoided.
 
 
-		Chapter 13: Allocating memory
+		Chapter 14: Allocating memory
 
 The kernel provides the following general purpose memory allocators:
 kmalloc(), kzalloc(), kcalloc(), and vmalloc().  Please refer to the API
@@ -429,7 +482,7 @@ from void pointer to any other pointer t
 language.
 
 
-		Chapter 14: The inline disease
+		Chapter 15: 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
@@ -457,7 +510,7 @@ something it would have done anyway.
 
 
 
-		Chapter 15: References
+		Appendix I: References
 
 The C Programming Language, Second Edition
 by Brian W. Kernighan and Dennis M. Ritchie.
@@ -481,4 +534,4 @@ Kernel CodingStyle, by greg@kroah.com at
 http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_talk/html/
 
 --
-Last updated on 30 December 2005 by a community effort on LKML.
+Last updated on 30 April 2006.

Re: [PATCH] CodingStyle: add typedefs chapter

[David Woodhouse]

On Mon, 2006-05-01 at 16:00 +0200, Jan Engelhardt wrote:
> What about task_t vs struct task_struct? Both are used in the kernel. 

task_t should probably die.

-- 
dwmw2

Re: [PATCH] CodingStyle: add typedefs chapter

[David Woodhouse]

On Sun, 2006-04-30 at 17:44 -0700, Randy.Dunlap wrote:
> + (b) Clear integer types, where the abstraction _helps_ avoid confusion
> +     whether it is "int" or "long".
> +
> +     u8/u16/u32 are perfectly fine typedefs. 

No, u8/u16/u32 are fall into category (d):

 (d) New types which are identical to standard C99 types, in certain
     exceptional circumstances.

     Although it would only take a short amount of time for the eyes and
     brain to become accustomed to the standard types like 'uint32_t',
     some people object to their use anyway.

     Therefore, the gratuitous 'u8/u16/u32/u64' types and their signed
     equivalents which are identical to standard types are permitted --
     although they are not mandatory.

 (e) Types safe for use in userspace. 

     In certain structures which are visible to userspace, we cannot
     require C99 types and cannot use the 'u32' form above. Thus, we
     use __u32 and similar types in all structures which are shared
     with userspace.

-- 
dwmw2

Re: [PATCH] CodingStyle: add typedefs chapter

[Jan Engelhardt]

>> What about task_t vs struct task_struct? Both are used in the kernel. 
>
>task_t should probably die.

Acked.



Jan Engelhardt
-- 

Re: [PATCH] CodingStyle: add typedefs chapter

[Jiri Slaby]

Jan Engelhardt napsal(a):
>>> What about task_t vs struct task_struct? Both are used in the kernel. 
>> task_t should probably die.
> 
> Acked.
Wouldn't this be a janitors' task?

regards,
-- 
Jiri Slaby         www.fi.muni.cz/~xslaby
\_.-^-._   jirislaby@gmail.com   _.-^-._/
B67499670407CE62ACC8 22A032CC55C339D47A7E

Re: [PATCH] CodingStyle: add typedefs chapter

[Randy.Dunlap]

On Mon, 01 May 2006 18:06:06 +0100 David Woodhouse wrote:

> On Sun, 2006-04-30 at 17:44 -0700, Randy.Dunlap wrote:
> > + (b) Clear integer types, where the abstraction _helps_ avoid confusion
> > +     whether it is "int" or "long".
> > +
> > +     u8/u16/u32 are perfectly fine typedefs. 
> 
> No, u8/u16/u32 are fall into category (d):

Thanks.  Here's an updated version.

---
From: Randy Dunlap <rdunlap@xenotime.net>

Add a chapter on typedefs, copied from an email from Linus
to lkml on Feb. 3, 2006:
(Subject: Re: [RFC][PATCH 1/5] Virtualization/containers: startup)
with added lkml feedback, esp. David Woodhouse.

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 Documentation/CodingStyle |   96 ++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 84 insertions(+), 12 deletions(-)

--- linux-2617-rc3.orig/Documentation/CodingStyle
+++ linux-2617-rc3/Documentation/CodingStyle
@@ -155,7 +155,79 @@ problem, which is called the function-gr
 See next chapter.
 
 
-		Chapter 5: Functions
+		Chapter 5: Typedefs
+
+Please don't use things like "vps_t".
+
+It's a _mistake_ to use typedef for structures and pointers. When you see a
+
+	vps_t a;
+
+in the source, what does it mean?
+
+In contrast, if it says
+
+	struct virtual_container *a;
+
+you can actually tell what "a" is.
+
+Lots of people think that typedefs "help readability". Not so. They are
+useful only for:
+
+ (a) totally opaque objects (where the typedef is actively used to _hide_
+     what the object is).
+
+     Example: "pte_t" etc. opaque objects that you can only access using
+     the proper accessor functions.
+
+     NOTE! Opaqueness and "accessor functions" are not good in themselves.
+     The reason we have them for things like pte_t etc. is that there
+     really is absolutely _zero_ portably accessible information there.
+
+ (b) Clear integer types, where the abstraction _helps_ avoid confusion
+     whether it is "int" or "long".
+
+     u8/u16/u32 are perfectly fine typedefs, although they fit into
+     category (d) better than here.
+
+     NOTE! Again - there needs to be a _reason_ for this. If something is
+     "unsigned long", then there's no reason to do
+
+	typedef unsigned long myflags_t;
+
+     but if there is a clear reason for why it under certain circumstances
+     might be an "unsigned int" and under other configurations might be
+     "unsigned long", then by all means go ahead and use a typedef.
+
+ (c) when you use sparse to literally create a _new_ type for
+     type-checking.
+
+ (d) New types which are identical to standard C99 types, in certain
+     exceptional circumstances.
+
+     Although it would only take a short amount of time for the eyes and
+     brain to become accustomed to the standard types like 'uint32_t',
+     some people object to their use anyway.
+
+     Therefore, the gratuitous 'u8/u16/u32/u64' types and their signed
+     equivalents which are identical to standard types are permitted --
+     although they are not mandatory.
+
+ (e) Types safe for use in userspace.
+
+     In certain structures which are visible to userspace, we cannot
+     require C99 types and cannot use the 'u32' form above. Thus, we
+     use __u32 and similar types in all structures which are shared
+     with userspace.
+
+Maybe there are other cases too, but the rule should basically be to NEVER
+EVER use a typedef unless you can clearly match one of those rules.
+
+In general, a pointer, or a struct that has elements that can reasonably
+be directly accessed should _never_ be a typedef.
+
+
+		Chapter 6: Functions
 
 Functions should be short and sweet, and do just one thing.  They should
 fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
@@ -183,7 +255,7 @@ and it gets confused.  You know you're b
 to understand what you did 2 weeks from now.
 
 
-		Chapter 6: Centralized exiting of functions
+		Chapter 7: Centralized exiting of functions
 
 Albeit deprecated by some people, the equivalent of the goto statement is
 used frequently by compilers in form of the unconditional jump instruction.
@@ -220,7 +292,7 @@ out:
 	return result;
 }
 
-		Chapter 7: Commenting
+		Chapter 8: 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
@@ -240,7 +312,7 @@ When commenting the kernel API functions
 See the files Documentation/kernel-doc-nano-HOWTO.txt and scripts/kernel-doc
 for details.
 
-		Chapter 8: You've made a mess of it
+		Chapter 9: 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
@@ -288,7 +360,7 @@ re-formatting you may want to take a loo
 remember: "indent" is not a fix for bad programming.
 
 
-		Chapter 9: Configuration-files
+		Chapter 10: Configuration-files
 
 For configuration options (arch/xxx/Kconfig, and all the Kconfig files),
 somewhat different indentation is used.
@@ -313,7 +385,7 @@ support for file-systems, for instance) 
 experimental options should be denoted (EXPERIMENTAL).
 
 
-		Chapter 10: Data structures
+		Chapter 11: Data structures
 
 Data structures that have visibility outside the single-threaded
 environment they are created and destroyed in should always have
@@ -344,7 +416,7 @@ Remember: if another thread can find you
 have a reference count on it, you almost certainly have a bug.
 
 
-		Chapter 11: Macros, Enums and RTL
+		Chapter 12: Macros, Enums and RTL
 
 Names of macros defining constants and labels in enums are capitalized.
 
@@ -399,7 +471,7 @@ The cpp manual deals with macros exhaust
 covers RTL which is used frequently with assembly language in the kernel.
 
 
-		Chapter 12: Printing kernel messages
+		Chapter 13: 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
@@ -410,7 +482,7 @@ Kernel messages do not have to be termin
 Printing numbers in parentheses (%d) adds no value and should be avoided.
 
 
-		Chapter 13: Allocating memory
+		Chapter 14: Allocating memory
 
 The kernel provides the following general purpose memory allocators:
 kmalloc(), kzalloc(), kcalloc(), and vmalloc().  Please refer to the API
@@ -429,7 +501,7 @@ from void pointer to any other pointer t
 language.
 
 
-		Chapter 14: The inline disease
+		Chapter 15: 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
@@ -457,7 +529,7 @@ something it would have done anyway.
 
 
 
-		Chapter 15: References
+		Appendix I: References
 
 The C Programming Language, Second Edition
 by Brian W. Kernighan and Dennis M. Ritchie.
@@ -481,4 +553,4 @@ Kernel CodingStyle, by greg@kroah.com at
 http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_talk/html/
 
 --
-Last updated on 30 December 2005 by a community effort on LKML.
+Last updated on 30 April 2006.

Re: [PATCH] CodingStyle: add typedefs chapter

[Jan Engelhardt]

>>>> What about task_t vs struct task_struct? Both are used in the kernel. 
>>> task_t should probably die.
>> 
>> Acked.
>Wouldn't this be a janitors' task?

Too simple :)

  find rc3 -type f -print0 | xargs -0 perl -i -pe
    's/\btask_t\b/struct task_struct'

+ a compile test afterwards. Something I missed? (Besides that lines may 
get longer and violate the 80-column CodingStyle rule.)


Jan Engelhardt
-- 

Re: [PATCH] CodingStyle: add typedefs chapter

[David Woodhouse]

On Mon, 2006-05-01 at 23:01 +0200, Jan Engelhardt wrote:
>   find rc3 -type f -print0 | xargs -0 perl -i -pe
>     's/\btask_t\b/struct task_struct'
> 
> + a compile test afterwards. Something I missed? (Besides that lines
> may get longer and violate the 80-column CodingStyle rule.) 

If we're going to do that, we might as well make it 'struct task'. The
additional '_struct' is redundant.

-- 
dwmw2

Re: [PATCH] CodingStyle: add typedefs chapter

[Daniel Barkalow]

On Sun, 30 Apr 2006, Randy.Dunlap wrote:

> + (b) Clear integer types, where the abstraction _helps_ avoid confusion
> +     whether it is "int" or "long".
> +
> +     u8/u16/u32 are perfectly fine typedefs.
> +
> +     NOTE! Again - there needs to be a _reason_ for this. If something is
> +     "unsigned long", then there's no reason to do
> +
> +	typedef long myflags_t;
> +
> +     but if there is a clear reason for why it under certain circumstances
> +     might be an "unsigned int" and under other configurations might be
> +     "unsigned long", then by all means go ahead and use a typedef.
> +
> + (c) when you use sparse to literally create a _new_ type for
> +     type-checking.

Is there an official position on "typedef unsigned __bitwise__ myflags_t"? 
That is, getting into case (c) with something not particularly useful for 
typechecking, just because you want to use a typedef. 

I don't remember if the gfp_t thing was only done because it would catch a 
very common type of bug, or if it's generally suggested for new code.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH] CodingStyle: add typedefs chapter

[Alexey Dobriyan]

On Mon, May 01, 2006 at 10:07:29PM +0100, David Woodhouse wrote:
> On Mon, 2006-05-01 at 23:01 +0200, Jan Engelhardt wrote:
> >   find rc3 -type f -print0 | xargs -0 perl -i -pe
> >     's/\btask_t\b/struct task_struct'
> >
> > + a compile test afterwards. Something I missed? (Besides that lines
> > may get longer and violate the 80-column CodingStyle rule.)
>
> If we're going to do that, we might as well make it 'struct task'. The
> additional '_struct' is redundant.

struct sighand_struct
	signal_struct
	files_struct
	fs_struct
	sel_arg_struct
	mmap_arg_struct
	vm_area_struct
	tty_struct
	fasync_struct
	rose_facilities_struct
	poll_table_struct
		...

What is the best day for Grand Renaming?

Re: [PATCH] CodingStyle: add typedefs chapter

[Johannes Stezenbach]

On Mon, May 01, 2006, David Woodhouse wrote:
> On Sun, 2006-04-30 at 17:44 -0700, Randy.Dunlap wrote:
> > + (b) Clear integer types, where the abstraction _helps_ avoid confusion
> > +     whether it is "int" or "long".
> > +
> > +     u8/u16/u32 are perfectly fine typedefs. 
> 
> No, u8/u16/u32 are fall into category (d):
> 
>  (d) New types which are identical to standard C99 types, in certain
>      exceptional circumstances.
> 
>      Although it would only take a short amount of time for the eyes and
>      brain to become accustomed to the standard types like 'uint32_t',
>      some people object to their use anyway.
> 
>      Therefore, the gratuitous 'u8/u16/u32/u64' types and their signed
>      equivalents which are identical to standard types are permitted --
>      although they are not mandatory.

IMHO u32 etc. are the well established data types used
everywhere in kernel source. Your wording suggests that
the use of C99 types would be better, and while I respect
your personal opinion, I think it is wrong to put that in the
kernel CodingStyle document.

c.f. http://lkml.org/lkml/2004/12/14/127


Johannes

Re: [PATCH] CodingStyle: add typedefs chapter

[Jörn Engel]

On Tue, 2 May 2006 01:38:59 +0400, Alexey Dobriyan wrote:
> 
> What is the best day for Grand Renaming?

Usually when you're in the area changing something anyway.

Jörn

-- 
Good warriors cause others to come to them and do not go to others.
-- Sun Tzu

Re: [PATCH] CodingStyle: add typedefs chapter

[Jes Sorensen]

>>>>> "David" == David Woodhouse <dwmw2@infradead.org> writes:

David> On Mon, 2006-05-01 at 23:01 +0200, Jan Engelhardt wrote:
>> find rc3 -type f -print0 | xargs -0 perl -i -pe
>> 's/\btask_t\b/struct task_struct'
>> 
>> + a compile test afterwards. Something I missed? (Besides that
>> lines may get longer and violate the 80-column CodingStyle rule.)

David> If we're going to do that, we might as well make it 'struct
David> task'. The additional '_struct' is redundant.

There were some long discussions about that a couple of years ago. I
believe Linus stated that he preferred _struct for those things.

It is also less likely to hit name clashes.

Jes

Re: [PATCH] CodingStyle: add typedefs chapter

[David Woodhouse]

On Tue, 2006-05-02 at 02:37 +0200, Johannes Stezenbach wrote:
> IMHO u32 etc. are the well established data types used
> everywhere in kernel source. Your wording suggests that
> the use of C99 types would be better, and while I respect
> your personal opinion, I think it is wrong to put that in the
> kernel CodingStyle document.

Perhaps the word 'gratuitous' should be removed, then, if you object to
being able to infer my opinion.

The point remains that the peculiarity should definitely be documented,
along with an explanation of the reasoning (or lack of such) behind it.

> c.f. http://lkml.org/lkml/2004/12/14/127 

That's about types used for _export_. It's accepted that __uXX types are
necessary in stuff which is visible by userspace. That was point (e).¹

The only bit in that mail which is relevant to my point (d) is the
penultimate (and final) paragraphs. And those are a complete non
sequitur and make just as much sense if you swap over 'u32' and
'uint32_t' and also 'kernel' and 'C language'...

"In other words, uint8_t/uint16_t/uint32_t/uint64_t (and the signed
versions: int8_t and friends) _are_ the standard names in the C
language, and the __u8/__u16/etc versions have always existed alongside
them for things like header files that have namespace issues.

"So forget about that stupid abortion called "u32" already. It buys
you absolutely nothing."

-- 
dwmw2

¹ Actually I had a conversation with Uli the other day in which he
seemed to suggest that proper C99 types _would_ be better, but let's not
go there for now.

Re: [PATCH] CodingStyle: add typedefs chapter

[Johannes Stezenbach]

On Tue, May 02, 2006, David Woodhouse wrote:
> On Tue, 2006-05-02 at 02:37 +0200, Johannes Stezenbach wrote:
> > IMHO u32 etc. are the well established data types used
> > everywhere in kernel source. Your wording suggests that
> > the use of C99 types would be better, and while I respect
> > your personal opinion, I think it is wrong to put that in the
> > kernel CodingStyle document.
> 
> Perhaps the word 'gratuitous' should be removed, then, if you object to
> being able to infer my opinion.
> 
> The point remains that the peculiarity should definitely be documented,
> along with an explanation of the reasoning (or lack of such) behind it.
> 
> > c.f. http://lkml.org/lkml/2004/12/14/127 
> 
> That's about types used for _export_. It's accepted that __uXX types are
> necessary in stuff which is visible by userspace. That was point (e).¹
> 
> The only bit in that mail which is relevant to my point (d) is the
> penultimate (and final) paragraphs. And those are a complete non
> sequitur and make just as much sense if you swap over 'u32' and
> 'uint32_t' and also 'kernel' and 'C language'...
> 
> "In other words, uint8_t/uint16_t/uint32_t/uint64_t (and the signed
> versions: int8_t and friends) _are_ the standard names in the C
> language, and the __u8/__u16/etc versions have always existed alongside
> them for things like header files that have namespace issues.
> 
> "So forget about that stupid abortion called "u32" already. It buys
> you absolutely nothing."

;-)

Maybe I got it wrong, but my impression so far was that
u8 etc. are preferred for kernel code, and C99 types
are merely tolerated. (Mostly for consistency reasons,
I guess, since most old code uses u8 etc.)

However, personally I don't care either way, I just
want to make sure that code written acording to
Documentation/CodingStyle also meets Linus' expectations.


Johannes

Re: [PATCH] CodingStyle: add typedefs chapter

[David Woodhouse]

On Tue, 2006-05-02 at 16:20 +0200, Johannes Stezenbach wrote:
> Maybe I got it wrong, but my impression so far was that
> u8 etc. are preferred for kernel code, and C99 types
> are merely tolerated. (Mostly for consistency reasons,
> I guess, since most old code uses u8 etc.)

It depends. In existing code, you should follow the precedent which is
set already. In new code of your own, you do as you see fit. Perhaps
that should be made clearer...

 (d) New types which are identical to standard C99 types, in certain
     exceptional circumstances.
 
     Although it would only take a short amount of time for the eyes and
     brain to become accustomed to the standard types like 'uint32_t',
     some people object to their use anyway.
 
     Therefore, the Linux-specific 'u8/u16/u32/u64' types and their
     signed equivalents which are identical to standard types are
     permitted -- although they are not mandatory in new code of your
     own.

     When editing existing code which already uses one or the other set
     of types, you should conform to the existing choices in that code.

-- 
dwmw2

Re: [PATCH] CodingStyle: add typedefs chapter

[Randy.Dunlap]

On Tue, 02 May 2006 15:31:48 +0100 David Woodhouse wrote:

> On Tue, 2006-05-02 at 16:20 +0200, Johannes Stezenbach wrote:
> > Maybe I got it wrong, but my impression so far was that
> > u8 etc. are preferred for kernel code, and C99 types
> > are merely tolerated. (Mostly for consistency reasons,
> > I guess, since most old code uses u8 etc.)
> 
> It depends. In existing code, you should follow the precedent which is
> set already. In new code of your own, you do as you see fit. Perhaps
> that should be made clearer...

patch updated, thanks.
---

From: Randy Dunlap <rdunlap@xenotime.net>

Add a chapter on typedefs, based on an email from Linus
to lkml on Feb. 3, 2006:
(Subject: Re: [RFC][PATCH 1/5] Virtualization/containers: startup)
with added lkml feedback, esp. David Woodhouse.

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 Documentation/CodingStyle |  100 ++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 88 insertions(+), 12 deletions(-)

--- linux-2617-rc3.orig/Documentation/CodingStyle
+++ linux-2617-rc3/Documentation/CodingStyle
@@ -155,7 +155,83 @@ problem, which is called the function-gr
 See next chapter.
 
 
-		Chapter 5: Functions
+		Chapter 5: Typedefs
+
+Please don't use things like "vps_t".
+
+It's a _mistake_ to use typedef for structures and pointers. When you see a
+
+	vps_t a;
+
+in the source, what does it mean?
+
+In contrast, if it says
+
+	struct virtual_container *a;
+
+you can actually tell what "a" is.
+
+Lots of people think that typedefs "help readability". Not so. They are
+useful only for:
+
+ (a) totally opaque objects (where the typedef is actively used to _hide_
+     what the object is).
+
+     Example: "pte_t" etc. opaque objects that you can only access using
+     the proper accessor functions.
+
+     NOTE! Opaqueness and "accessor functions" are not good in themselves.
+     The reason we have them for things like pte_t etc. is that there
+     really is absolutely _zero_ portably accessible information there.
+
+ (b) Clear integer types, where the abstraction _helps_ avoid confusion
+     whether it is "int" or "long".
+
+     u8/u16/u32 are perfectly fine typedefs, although they fit into
+     category (d) better than here.
+
+     NOTE! Again - there needs to be a _reason_ for this. If something is
+     "unsigned long", then there's no reason to do
+
+	typedef unsigned long myflags_t;
+
+     but if there is a clear reason for why it under certain circumstances
+     might be an "unsigned int" and under other configurations might be
+     "unsigned long", then by all means go ahead and use a typedef.
+
+ (c) when you use sparse to literally create a _new_ type for
+     type-checking.
+
+ (d) New types which are identical to standard C99 types, in certain
+     exceptional circumstances.
+
+     Although it would only take a short amount of time for the eyes and
+     brain to become accustomed to the standard types like 'uint32_t',
+     some people object to their use anyway.
+
+     Therefore, the Linux-specific 'u8/u16/u32/u64' types and their
+     signed equivalents which are identical to standard types are
+     permitted -- although they are not mandatory in new code of your
+     own.
+
+     When editing existing code which already uses one or the other set
+     of types, you should conform to the existing choices in that code.
+
+ (e) Types safe for use in userspace.
+
+     In certain structures which are visible to userspace, we cannot
+     require C99 types and cannot use the 'u32' form above. Thus, we
+     use __u32 and similar types in all structures which are shared
+     with userspace.
+
+Maybe there are other cases too, but the rule should basically be to NEVER
+EVER use a typedef unless you can clearly match one of those rules.
+
+In general, a pointer, or a struct that has elements that can reasonably
+be directly accessed should _never_ be a typedef.
+
+
+		Chapter 6: Functions
 
 Functions should be short and sweet, and do just one thing.  They should
 fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
@@ -183,7 +259,7 @@ and it gets confused.  You know you're b
 to understand what you did 2 weeks from now.
 
 
-		Chapter 6: Centralized exiting of functions
+		Chapter 7: Centralized exiting of functions
 
 Albeit deprecated by some people, the equivalent of the goto statement is
 used frequently by compilers in form of the unconditional jump instruction.
@@ -220,7 +296,7 @@ out:
 	return result;
 }
 
-		Chapter 7: Commenting
+		Chapter 8: 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
@@ -240,7 +316,7 @@ When commenting the kernel API functions
 See the files Documentation/kernel-doc-nano-HOWTO.txt and scripts/kernel-doc
 for details.
 
-		Chapter 8: You've made a mess of it
+		Chapter 9: 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
@@ -288,7 +364,7 @@ re-formatting you may want to take a loo
 remember: "indent" is not a fix for bad programming.
 
 
-		Chapter 9: Configuration-files
+		Chapter 10: Configuration-files
 
 For configuration options (arch/xxx/Kconfig, and all the Kconfig files),
 somewhat different indentation is used.
@@ -313,7 +389,7 @@ support for file-systems, for instance) 
 experimental options should be denoted (EXPERIMENTAL).
 
 
-		Chapter 10: Data structures
+		Chapter 11: Data structures
 
 Data structures that have visibility outside the single-threaded
 environment they are created and destroyed in should always have
@@ -344,7 +420,7 @@ Remember: if another thread can find you
 have a reference count on it, you almost certainly have a bug.
 
 
-		Chapter 11: Macros, Enums and RTL
+		Chapter 12: Macros, Enums and RTL
 
 Names of macros defining constants and labels in enums are capitalized.
 
@@ -399,7 +475,7 @@ The cpp manual deals with macros exhaust
 covers RTL which is used frequently with assembly language in the kernel.
 
 
-		Chapter 12: Printing kernel messages
+		Chapter 13: 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
@@ -410,7 +486,7 @@ Kernel messages do not have to be termin
 Printing numbers in parentheses (%d) adds no value and should be avoided.
 
 
-		Chapter 13: Allocating memory
+		Chapter 14: Allocating memory
 
 The kernel provides the following general purpose memory allocators:
 kmalloc(), kzalloc(), kcalloc(), and vmalloc().  Please refer to the API
@@ -429,7 +505,7 @@ from void pointer to any other pointer t
 language.
 
 
-		Chapter 14: The inline disease
+		Chapter 15: 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
@@ -457,7 +533,7 @@ something it would have done anyway.
 
 
 
-		Chapter 15: References
+		Appendix I: References
 
 The C Programming Language, Second Edition
 by Brian W. Kernighan and Dennis M. Ritchie.
@@ -481,4 +557,4 @@ Kernel CodingStyle, by greg@kroah.com at
 http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_talk/html/
 
 --
-Last updated on 30 December 2005 by a community effort on LKML.
+Last updated on 30 April 2006.

Re: [PATCH] CodingStyle: add typedefs chapter

[Linus Torvalds]

On Tue, 2 May 2006, Randy.Dunlap wrote:
> +
> +     Although it would only take a short amount of time for the eyes and
> +     brain to become accustomed to the standard types like 'uint32_t',
> +     some people object to their use anyway.

The problem with uint32_t is that it's ugly, it used to be unportable, and 
you can't use it in header files _anyway_.

In other words, there's no _point_ to the "standard type". 

I really object to this whole thing. The fact is, "u8" and friends _are_ 
the standard types as far as the kernel is concerned.  Claiming that they 
aren't is just silly.

It's the "uint32_t" kind of thing that isn't standard within the kernel. 
You can't use that thing in public header files anyway due to name scoping 
rules, so they have basically no redeeming features.

			Linus

Re: [PATCH] CodingStyle: add typedefs chapter

[David Woodhouse]

On Tue, 2006-05-02 at 11:41 -0700, Linus Torvalds wrote:
> The problem with uint32_t is that it's ugly, it used to be unportable, and 
> you can't use it in header files _anyway_.

Unportable? It's at least as portable as u32 is, surely? We probably
wouldn't have used <stdint.h> in the kernel anyway -- we define them
ourselves. 

The header files are completely irrelevant too -- we're talking about
'u32' not '__u32'.

The important thing is your belief that it's ugly, which is what was
documented.

> I really object to this whole thing. The fact is, "u8" and friends _are_ 
> the standard types as far as the kernel is concerned.  Claiming that they 
> aren't is just silly.

When describing the CodingStyle rules "thou shalt not use typedefs" we
do need to list the exceptions, and that includes 'u32' et al.

Yes, those _are_ acceptable in the kernel. That's what the document
_says_. It _doesn't_ say that they're not.

-- 
dwmw2

Re: [PATCH] CodingStyle: add typedefs chapter

[Linus Torvalds]

On Tue, 2 May 2006, David Woodhouse wrote:
>
> On Tue, 2006-05-02 at 11:41 -0700, Linus Torvalds wrote:
> > The problem with uint32_t is that it's ugly, it used to be unportable, and 
> > you can't use it in header files _anyway_.
> 
> Unportable? It's at least as portable as u32 is, surely? We probably
> wouldn't have used <stdint.h> in the kernel anyway -- we define them
> ourselves. 

When the u<n> things were done, uint<n>_t wasn't at all common. 

> The header files are completely irrelevant too -- we're talking about
> 'u32' not '__u32'.

That's not irrelevant. Usually you want to have stuff in header files that 
you use in source code. You want the two to visually look similar. It's a 
hell of a lot less confusing to use "u32" (in source) and "__u32" (in the 
header file), than it is to mix "uint32_t" (in source) and some random 
other thing (in header file).

> The important thing is your belief that it's ugly, which is what was
> documented.

And that wasn't what I objected to. 

What I objected to was that other part, which said that "uint32_t" was 
somehow more standard.

IN THE KERNEL IT IS _LESS_ STANDARD.

And outside the kernel, that documentation is not exactly relevant.

		Linus

Re: [PATCH] CodingStyle: add typedefs chapter

[Marcel Siegert]

Linus Torvalds wrote:
> 
> On Tue, 2 May 2006, David Woodhouse wrote:
> 
>>On Tue, 2006-05-02 at 11:41 -0700, Linus Torvalds wrote:
>>
>>>The problem with uint32_t is that it's ugly, it used to be unportable, and 
>>>you can't use it in header files _anyway_.
>>
>>Unportable? It's at least as portable as u32 is, surely? We probably
>>wouldn't have used <stdint.h> in the kernel anyway -- we define them
>>ourselves. 
> 
> 
> When the u<n> things were done, uint<n>_t wasn't at all common. 
> 
> 
>>The header files are completely irrelevant too -- we're talking about
>>'u32' not '__u32'.
> 
> 
> That's not irrelevant. Usually you want to have stuff in header files that 
> you use in source code. You want the two to visually look similar. It's a 
> hell of a lot less confusing to use "u32" (in source) and "__u32" (in the 
> header file), than it is to mix "uint32_t" (in source) and some random 
> other thing (in header file).

isn't it possible to mix up u32 and some random other thing?
> 
>>The important thing is your belief that it's ugly, which is what was
>>documented.
> 
> 
> And that wasn't what I objected to. 
> 
> What I objected to was that other part, which said that "uint32_t" was 
> somehow more standard.
> 
> IN THE KERNEL IT IS _LESS_ STANDARD.
> 
> And outside the kernel, that documentation is not exactly relevant.

nack. something is a standard or something is not. black or white. grey isn't there.
of course, you are free to create your own kernel standard or whatever.
what about __uint32_t? *scnr*

marcel


> 		Linus
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] CodingStyle: add typedefs chapter

[Linus Torvalds]

On Tue, 2 May 2006, Marcel Siegert wrote:
>
> nack. something is a standard or something is not. black or white. grey isn't
> there.

Here's a saying for you:

	"Only stupid people are black or white. The real world isn't".

Anyway, look at the particular document we're talking about. We're talking 
about the kernel standard coding-style.

Within that one, 8-char tabs are standard. Within that one, "u8" is 
standard. Within that one, typedefs are frowned upon.

In other contexts, other things are standard. Live with it.

		Linus

Re: [PATCH] CodingStyle: add typedefs chapter

[David Woodhouse]

On Tue, 2006-05-02 at 12:07 -0700, Linus Torvalds wrote:
> And that wasn't what I objected to. 
> 
> What I objected to was that other part, which said that "uint32_t" was 
> somehow more standard.

It didn't say "more standard". It referred to "the standard C99 types".

It's heading off the question "why object to ifdefs but permit _these_
gratuitous ones?" which would otherwise be asked.

It's a document which is _describing_ the Linux coding style. To refer
to u32 et al as 'standard' would be self-referential. Describe them as
'the Linux standard types' in other documents by all means, but it
doesn't make much sense to do so in Documentation/CodingStyle.

-- 
dwmw2

Re: [PATCH] CodingStyle: add typedefs chapter

[Randy.Dunlap]

On Wed, 03 May 2006 00:22:16 +0100 David Woodhouse wrote:

> On Tue, 2006-05-02 at 12:07 -0700, Linus Torvalds wrote:
> > And that wasn't what I objected to. 
> > 
> > What I objected to was that other part, which said that "uint32_t" was 
> > somehow more standard.
> 
> It didn't say "more standard". It referred to "the standard C99 types".
> 
> It's heading off the question "why object to ifdefs but permit _these_
> gratuitous ones?" which would otherwise be asked.
> 
> It's a document which is _describing_ the Linux coding style. To refer
> to u32 et al as 'standard' would be self-referential. Describe them as
> 'the Linux standard types' in other documents by all means, but it
> doesn't make much sense to do so in Documentation/CodingStyle.

All references to "standard types" now say "standard C99 types".
However, Linus still objects to the C99 integer typedefs AFAICT.
Are we at an impasse?
It would be a really Good Idea to have something about typedefs
in Doc/CodingStyle.

---
From: Randy Dunlap <rdunlap@xenotime.net>

Add a chapter on typedefs, based on an email from Linus
to lkml on Feb. 3, 2006:
(Subject: Re: [RFC][PATCH 1/5] Virtualization/containers: startup)
with added lkml feedback, esp. David Woodhouse.

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 Documentation/CodingStyle |  100 ++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 88 insertions(+), 12 deletions(-)

--- linux-2617-rc3.orig/Documentation/CodingStyle
+++ linux-2617-rc3/Documentation/CodingStyle
@@ -155,7 +155,83 @@ problem, which is called the function-gr
 See next chapter.
 
 
-		Chapter 5: Functions
+		Chapter 5: Typedefs
+
+Please don't use things like "vps_t".
+
+It's a _mistake_ to use typedef for structures and pointers. When you see a
+
+	vps_t a;
+
+in the source, what does it mean?
+
+In contrast, if it says
+
+	struct virtual_container *a;
+
+you can actually tell what "a" is.
+
+Lots of people think that typedefs "help readability". Not so. They are
+useful only for:
+
+ (a) totally opaque objects (where the typedef is actively used to _hide_
+     what the object is).
+
+     Example: "pte_t" etc. opaque objects that you can only access using
+     the proper accessor functions.
+
+     NOTE! Opaqueness and "accessor functions" are not good in themselves.
+     The reason we have them for things like pte_t etc. is that there
+     really is absolutely _zero_ portably accessible information there.
+
+ (b) Clear integer types, where the abstraction _helps_ avoid confusion
+     whether it is "int" or "long".
+
+     u8/u16/u32 are perfectly fine typedefs, although they fit into
+     category (d) better than here.
+
+     NOTE! Again - there needs to be a _reason_ for this. If something is
+     "unsigned long", then there's no reason to do
+
+	typedef unsigned long myflags_t;
+
+     but if there is a clear reason for why it under certain circumstances
+     might be an "unsigned int" and under other configurations might be
+     "unsigned long", then by all means go ahead and use a typedef.
+
+ (c) when you use sparse to literally create a _new_ type for
+     type-checking.
+
+ (d) New types which are identical to standard C99 types, in certain
+     exceptional circumstances.
+
+     Although it would only take a short amount of time for the eyes and
+     brain to become accustomed to the standard C99 types like 'uint32_t',
+     some people object to their use anyway.
+
+     Therefore, the Linux-specific 'u8/u16/u32/u64' types and their
+     signed equivalents which are identical to standard C99 types are
+     permitted -- although they are not mandatory in new code of your
+     own.
+
+     When editing existing code which already uses one or the other set
+     of types, you should conform to the existing choices in that code.
+
+ (e) Types safe for use in userspace.
+
+     In certain structures which are visible to userspace, we cannot
+     require C99 types and cannot use the 'u32' form above. Thus, we
+     use __u32 and similar types in all structures which are shared
+     with userspace.
+
+Maybe there are other cases too, but the rule should basically be to NEVER
+EVER use a typedef unless you can clearly match one of those rules.
+
+In general, a pointer, or a struct that has elements that can reasonably
+be directly accessed should _never_ be a typedef.
+
+
+		Chapter 6: Functions
 
 Functions should be short and sweet, and do just one thing.  They should
 fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
@@ -183,7 +259,7 @@ and it gets confused.  You know you're b
 to understand what you did 2 weeks from now.
 
 
-		Chapter 6: Centralized exiting of functions
+		Chapter 7: Centralized exiting of functions
 
 Albeit deprecated by some people, the equivalent of the goto statement is
 used frequently by compilers in form of the unconditional jump instruction.
@@ -220,7 +296,7 @@ out:
 	return result;
 }
 
-		Chapter 7: Commenting
+		Chapter 8: 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
@@ -240,7 +316,7 @@ When commenting the kernel API functions
 See the files Documentation/kernel-doc-nano-HOWTO.txt and scripts/kernel-doc
 for details.
 
-		Chapter 8: You've made a mess of it
+		Chapter 9: 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
@@ -288,7 +364,7 @@ re-formatting you may want to take a loo
 remember: "indent" is not a fix for bad programming.
 
 
-		Chapter 9: Configuration-files
+		Chapter 10: Configuration-files
 
 For configuration options (arch/xxx/Kconfig, and all the Kconfig files),
 somewhat different indentation is used.
@@ -313,7 +389,7 @@ support for file-systems, for instance) 
 experimental options should be denoted (EXPERIMENTAL).
 
 
-		Chapter 10: Data structures
+		Chapter 11: Data structures
 
 Data structures that have visibility outside the single-threaded
 environment they are created and destroyed in should always have
@@ -344,7 +420,7 @@ Remember: if another thread can find you
 have a reference count on it, you almost certainly have a bug.
 
 
-		Chapter 11: Macros, Enums and RTL
+		Chapter 12: Macros, Enums and RTL
 
 Names of macros defining constants and labels in enums are capitalized.
 
@@ -399,7 +475,7 @@ The cpp manual deals with macros exhaust
 covers RTL which is used frequently with assembly language in the kernel.
 
 
-		Chapter 12: Printing kernel messages
+		Chapter 13: 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
@@ -410,7 +486,7 @@ Kernel messages do not have to be termin
 Printing numbers in parentheses (%d) adds no value and should be avoided.
 
 
-		Chapter 13: Allocating memory
+		Chapter 14: Allocating memory
 
 The kernel provides the following general purpose memory allocators:
 kmalloc(), kzalloc(), kcalloc(), and vmalloc().  Please refer to the API
@@ -429,7 +505,7 @@ from void pointer to any other pointer t
 language.
 
 
-		Chapter 14: The inline disease
+		Chapter 15: 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
@@ -457,7 +533,7 @@ something it would have done anyway.
 
 
 
-		Chapter 15: References
+		Appendix I: References
 
 The C Programming Language, Second Edition
 by Brian W. Kernighan and Dennis M. Ritchie.
@@ -481,4 +557,4 @@ Kernel CodingStyle, by greg@kroah.com at
 http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_talk/html/
 
 --
-Last updated on 30 December 2005 by a community effort on LKML.
+Last updated on 30 April 2006.

Re: [PATCH] CodingStyle: add typedefs chapter

[David Woodhouse]

On Wed, 2006-05-03 at 12:41 -0700, Randy.Dunlap wrote:
> All references to "standard types" now say "standard C99 types".
> However, Linus still objects to the C99 integer typedefs AFAICT.
> Are we at an impasse?
> It would be a really Good Idea to have something about typedefs
> in Doc/CodingStyle. 

Absolutely, and you need to document the exceptions. That includes
saying that 'u32' et al are OK.

-- 
dwmw2

Re: [PATCH] CodingStyle: add typedefs chapter

[Randy.Dunlap]

On Wed, 03 May 2006 22:52:44 +0100 David Woodhouse wrote:

> On Wed, 2006-05-03 at 12:41 -0700, Randy.Dunlap wrote:
> > All references to "standard types" now say "standard C99 types".
> > However, Linus still objects to the C99 integer typedefs AFAICT.
> > Are we at an impasse?
> > It would be a really Good Idea to have something about typedefs
> > in Doc/CodingStyle. 
> 
> Absolutely, and you need to document the exceptions. That includes
> saying that 'u32' et al are OK.

Uh, I think that it currently says that...

---
~Randy

Re: [PATCH] CodingStyle: add typedefs chapter

[David Woodhouse]

On Wed, 2006-05-03 at 15:09 -0700, Randy.Dunlap wrote:
> Uh, I think that it currently says that... 

Yes, it does. So that should be fine then...

-- 
dwmw2