2006-05-01 17:06:03

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter

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


2006-05-01 20:46:01

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter

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 <[email protected]>

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 <[email protected]>
---
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 [email protected] 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.

2006-05-02 00:38:05

by Johannes Stezenbach

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter

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

2006-05-02 13:28:23

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter

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.

2006-05-02 14:20:58

by Johannes Stezenbach

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter

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

2006-05-02 14:31:56

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter

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

2006-05-02 17:08:51

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter

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 <[email protected]>

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 <[email protected]>
---
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 [email protected] 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.

2006-05-02 18:41:30

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter



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

2006-05-02 18:51:16

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter

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

2006-05-02 19:07:51

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter



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

2006-05-02 19:21:47

by Marcel Siegert

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter

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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2006-05-02 19:44:57

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter



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

2006-05-02 23:22:26

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter

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

2006-05-03 19:38:38

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter

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 <[email protected]>

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 <[email protected]>
---
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 [email protected] 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.

2006-05-03 21:52:55

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter

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

2006-05-03 22:06:39

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter

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

2006-05-03 22:10:44

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] CodingStyle: add typedefs chapter

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