I appreciate the info about the typedefs, but I didn't really mean to
signal them out. They just happened to be an example I picked. The
fundamental issue is that we have a large number of files written using
a fairly common corporate coding standard (4 space indents, no tabs,
javadoc comments) that is used by a number of OSes. We generate our API
documentation from it using doxygen, so the comments at least attempt to
have useful info in them. In order to use this code in the kernel, we're
having to basically run it through the evil indent program and strip
quite a bit of stuff out. Obviously indent will make a mess of some
stuff, so lots of hand editing is needed. The end result is code that
has diverged from the original in such a way that diffs are basically
impossible.
We're already having issues with bugs between the different versions and
the kernel submit stuff has just started. Maybe the only solution is two
divergent code bases, but it just feels like the gun is pointed at my
foot and cocked. I really don't want to pull the trigger knowing what is
about to happen.
Obviously I don't target these comments at true Linux drivers. The CF
libata driver is obviously a Linux specific driver and should follow all
Linux standards. Code for bootloader communication and networks setup is
completely OS agnostic, so sharing it between systems makes sense.
Chad
Ralf Baechle wrote:
> On Sat, Nov 22, 2008 at 04:44:52AM -0600, Kevin D. Kissell wrote:
>
>>>
>>>> [This should be good for some useless weekend flaming.]
>>>>
>>> Yeah! ;-)
>
> With the oil price down so far we gotta make some good use of that ;-)
>
>> That's a better argument than the one in the HTML version of
>> Documentation/CodingStyle.txt that I had bookmarked (which was what I
>> cited). Interestingly, if I look at the *current* Linux
>> Documentation/CodingStyle.txt for 2.6.28-rc6, the blanket interdiction
>> of typedefs is no longer there! Things *have* evolved, as I said they'd
>> have to, to recognize 5 (a good Illuminati number) cases where typedefs
>> are permitted. Superficially, based on Chad's description (I admit that
>> I haven't been reviewing his patches) the Cavium case would seem to fall
>> into the first category. Is the MIPS Linux community now some kind of
>> ultra-orthodox sub-sect of the Linux cult? ;o)
>
> There was never a blanket interdiction of typedefs though it may seem to
> and unfortunately scripts/checkpatch.pl also bitches about every typedef,
> no matter what. It was rather a restriction to only using typedefs for
> simple types such as char, int or pointers but not on structs - otherwise
> the definitions for u32 etc. would not have made it in. In a 2002 OLS
> paper giving a bit of a rationale for the Linux coding standard Greg KH
> writes:
>
> [...]
> typedef is evil
>
> typedef should not be used in naming any of your structures. Almost all main
> kernel structures do not have a typedef to shorten their usage. This
> includes the following:
>
> struct inode
> struct dentry
> struct file
> struct buffer_head
> struct user
> struct task_struct
>
> Using typedef tries to hide the real type of a variable. There have been
> records of some kernel code using typedefs nested up to 4 layers deep,
> preventing the programmer from telling what type of variable they are really
> using. This can easily cause very large structures to be accidentally
> declared on the stack, or to be returned from functions if the programmer
> does not realize the size of the structure. typedef can also be used as a
> crutch to keep from typing long structure denitions. If this is the case,
> the structure names should be made shorter, according to the above listed
> naming rules. Never define a typedef to just signify a pointer to a
> structure, as in the following example:
>
> typedef struct foo
> {
> int bar;
> int baz;
> } foo t, *pfoo_t;
>
> This again hides the true type of the variable, and is using the name of
> the variable type to define what is is (see the comment about Hungarian
> notation previously.
>
> Some examples of where typedef is badly used are in the include/raid/md*.h
> files where every structure has a typedef assigned to it, and in the
> drivers/acpi/include/*.h files, where a lot of the structures do not even
> have a name assigned to them, only a typedef.
>
> The only place that using typedef is acceptable, is in declaring function
> prototypes. These can be difcult to type out every time, so declaring a
> typedef for these is nice to do. An example of this is the bh_end_io_t
> typedef which is used as a parameter in the init buffer() call. This is
> defined in include/fs.h as:
>
> typedef void (bh_end_io_t)(struct buffer_head *bh, int uptodate);
>
> [...]
>
> The full paper is available at http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_paper/codingstyle.ps
>
> Ralf
--
Chad Reese <kreese@xxxxxxxxxxxxxxxxxx>
Cavium Networks
Phone: 650 - 623 - 7038
Cell: 321 - 438 - 7753
