Newsgroups: fa.linux.kernel
From: Linus Torvalds
Subject: Re: [PATCH] 2.5.21 - list.h cleanup
Original-Message-ID: <Pine.LNX.4.44.0206100954250.30535-100000@home.transmeta.com>
Date: Mon, 10 Jun 2002 17:03:50 GMT
Message-ID: <fa.l4d7opv.1mhk21p@ifi.uio.no>
On Mon, 10 Jun 2002, Thomas 'Dent' Mirlacher wrote:
>
> ok, the point that *_t is hiding implementation details (when used for
> structs is valid). but is there a general consens on not using typedefs
> for structs?
The linux coding style _tends_ to avoid using typedefs. It's not a hard
rule (and I did in fact apply this patch, since it otherwise looked fine),
but it's fairly common to use an explicit "struct xxxx" instead of
"xxxx_t".
I dislike type abstraction if it has no real reason. And saving on typing
is not a good reason - if your typing speed is the main issue when you're
coding, you're doing something seriously wrong.
(Similarly, if you are trying to compress lines to be shorter, you have
other problems, nothing to do with type names).
Does code look "prettier" with a "_t" rather than a "struct "? I don't
know. I personally don't think so, and I also hate the "_p" (or even more
the just plain "p") convention for "pointer".
Hiding the fact that it's a struct causes bad things if people don't
realize it: allocating structs on the stack is something you should be
aware of (and careful with), and passing them as parameters is is much
better done explicitly as a "pointer to struct".
There are _some_ exceptions. For example, "pte_t" etc might well be a
struct on most architectures, and that's ok: it's expressly designed to be
an opaque (and still fairly small) thing. This is an example of where
there are clear _reasons_ for the abstraction, not just abstraction for
its own sake.
But in the end, maintainership matters. I personally don't want the
typedef culture to get the upper hand, but I don't mind a few of them, and
people who maintain their own code usually get the last word.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds
Subject: Re: [PATCH] 2.5.21 - list.h cleanup
Original-Message-ID: <Pine.LNX.4.44.0206101011440.30535-100000@home.transmeta.com>
Date: Mon, 10 Jun 2002 17:22:34 GMT
Message-ID: <fa.l5sjnpv.1l1431s@ifi.uio.no>
On Mon, 10 Jun 2002, Thomas 'Dent' Mirlacher wrote:
>
> On Mon, 10 Jun 2002, Linus Torvalds wrote:
>
> --snip/snip
> > But in the end, maintainership matters. I personally don't want the
> > typedef culture to get the upper hand, but I don't mind a few of them, and
> > people who maintain their own code usually get the last word.
>
> to sum it up:
>
> using the "struct mystruct" is _recommended_, but not a must.
Well, it's more than just "struct xx". It's really typedefs in general.
For example, some people like to do things like
typedef unsigned int counter_t;
and then use "counter_t" all over the place. I think that's not just ugly,
but stupid and counter-productive. It makes it much harder to do things
like "printk()" portably, for example ("should I use %u, %l or just %d?"),
and generally adds no value. It only _hides_ information, like whether the
type is signed or not.
There is nothing wrong with just using something like "unsigned long"
directly, even if it is a few characters longer than you might like. And
if you care about the number of bits, use "u32" or something. Don't make
up useless types that have no added advantage.
We actually have real _problems_ due to this in the kernel, where people
use "off_t", and it's not easily printk'able across different
architectures (we used to have this same problem with size_t). We should
have just used "unsigned long" inside the kernel, and be done with it (and
"unsigned long long" for loff_t).
We should also have some format for printing out "u32/u64" etc, but that's
another issue and has the problem that gcc won't understand them, so
adding new formats is _hard_ from a maintenance standpoint.
Linus
PS. And never _ever_ make the "pointerness" part of the type. People who
write
typedef struct urb_struct * urbp_t;
(or whatever the name was) should just be shot. I was _soo_ happy to see
that crap get excised from the kernel USB drivers.
Newsgroups: fa.linux.kernel
From: Linus Torvalds
Subject: Re: [patch] Workqueue Abstraction, 2.5.40-H7
Original-Message-ID: <Pine.LNX.4.33.0210011210030.1878-100000@penguin.transmeta.com>
Date: Tue, 1 Oct 2002 19:15:17 GMT
Message-ID: <fa.o99pduv.hkq436@ifi.uio.no>
On Tue, 1 Oct 2002, Ingo Molnar wrote:
>
> the attached (compressed) patch is the next iteration of the workqueue
> abstraction. There are two major categories of changes:
Please don't introduce more typedefs. They only hide what the hell the
thing is, which is actively _bad_ for structures, since passing a
structure by value etc is something that should never be done, for
example.
The few saved characters of typing do not actually _buy_ you anything
else, and only obscures what the thing is.
Also, it's against the Linux coding standard, which does not like adding
magic single-letter suffixes to things - that also is the case for your
strange "_s" suffix for a structure (the real suffix is "_struct").
Remember: typing out something is not bad. It's _especially_ not bad if
the typing makes it more clear what the thing is.
I've done a global search-and-replace on the patch. The resulting patch is
actually _cleaner_, because it also matches more closely the old code
(which used "struct tq_struct"), so things like tabbed comment alignment
etc tend to be more correct (not always, but closer).
Linus
Newsgroups: fa.linux.kernel
From: torvalds@transmeta.com (Linus Torvalds)
Subject: Re: [patch] Workqueue Abstraction, 2.5.40-H7
Original-Message-ID: <ancug3$iq1$1@penguin.transmeta.com>
Date: Tue, 1 Oct 2002 19:52:30 GMT
Message-ID: <fa.k1e80bv.530685@ifi.uio.no>
In article ,
Linus Torvalds wrote:
>
>Pease don't introduce more typedefs. They only hide what the hell the
>thing is, which is actively _bad_ for structures, since passing a
>structure by value etc is something that should never be done, for
>example.
Btw, just to avoid counter-examples: Linux does use structures and
typedefs occasionally to hide and force compiler typechecking on small
structures on purpose. We have a few places where we do things like
typedef struct {
unsigned int value;
} atomic_t;
(and similar things for the page table entries etc).
This is done because the things are often really regular scalars, but we
use the structure as a strict type checking mechanism. In this case,
using a typedef is fine, because we don't actually ever want to _access_
it as a structure, and the typedef provices exactly the kind of
information hiding that we need.
But type hiding for a real structure just doesn't make sense, since we
use it as a true structure, and hiding information just makes it harder
to see.
Linus
Newsgroups: fa.linux.kernel
From: Linus Torvalds
Subject: Re: [patch] Workqueue Abstraction, 2.5.40-H7
Original-Message-ID: <Pine.LNX.4.33.0210011345260.1372-100000@penguin.transmeta.com>
Date: Tue, 1 Oct 2002 20:49:18 GMT
Message-ID: <fa.oa9nemv.gk45b9@ifi.uio.no>
On Tue, 1 Oct 2002, Ingo Molnar wrote:
>
> Despite all the previous fuss about the problems of typedefs, i've never
> had *any* problem with using typedefs in various code i wrote.
Big things should have big names. That's why "u8" is u8, because it's not
just physically small, it also has very little semantics associated with
it.
I want those variable declarations to stand out, and make people
understand that this is not just a variable, it's a structure, and it may
be taking up a noticeable amount of space on the stack, for example.
That's the main issue for me. I don't personally care so much about trying
to avoid dependencies in the header files that can also be problematic.
That's probably partly because I use fast enough machines that parsing
them a few extra times doesn't much bother me, and circular requirements
tend to be rare enough not to bother me unduly.
So the thing is a big red warning sign that you're now using a complex
data structure, and you should be aware of the semantics that go with it.
Linus
From: Linus Torvalds
Newsgroups: fa.linux.kernel
Subject: Re: [RFC][PATCH 1/5] Virtualization/containers: startup
Date: Fri, 03 Feb 2006 17:16:47 UTC
Message-ID: <fa.4bkWs6R4XPzyMAJare3mNZgSBgI@ifi.uio.no>
Original-Message-ID: <Pine.LNX.4.64.0602030905380.4630@g5.osdl.org>
On Fri, 3 Feb 2006, Kirill Korotaev wrote:
>
> This patch introduces some abstract container/VPS kernel structure and tiny
> amount of operations on it.
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.
Linus
From: Linus Torvalds
Newsgroups: fa.linux.kernel
Subject: Re: Slab: Remove kmem_cache_t
Date: Wed, 29 Nov 2006 16:12:48 UTC
Message-ID: <fa.QqBcP76GmvQ7hKoj8Em059Dwf50@ifi.uio.no>
On Wed, 29 Nov 2006, Nick Piggin wrote:
>
> You are saying that they should only be used to create new "primitive"
> types (ie. that you can use in arithmetic / logical ops) that can
> change depending on the config.
Well, it doesn't have to be something that is "arithmetic".
For an example of a primitive type that isn't arithmetic, the page table
entries are (pgt_t/pud_t/pmd_t/pte_t) are excellent - they don't do any
arithmetic or logical ops, but they do change depending on config, and no,
they aren't always opaque structures.
(Actually, these days they mostly are, but on many architectures it's much
slower to pass even a small struct around than it is to pass an integer
around - due simply to calling conventions - so for truly opaque things,
the typedef has the advantage that it _can_ be an opaque integer type, and
nobody will notice).
> That's fair enough. I'm sure you've also said in the past that they can
> be used (IIRC you even encouraged it) when the type is opaque in the
> context it is being used.
I'm sure I've been inconsistent, but in general, typedefs are bad. I think
you'll notice that I almost never use them myself. I much prefer passing
an opaque structure around, _unless_ I know the structure is so small that
it makes sense to do the above optimization (ie allow the case where the
opaque thing actually ends up being an integer).
Opaque integer types are generally useless in C, because they lose all the
type information _way_ too easily. There are no warnings for mis-use,
unless you use a sparse "bitwise" type and actually run sparse on the
thing. So even when there are performance reasons to use opaque integer
types (and on x86, the page table things were one such thing), usign a
struct is often preferable just for type-checking.
And as mentioned, there _are_ exceptions. Some types just get _sooo_
complex that it's inconvenient to type them out, even if they are
perfectly regular types, and don't depend on any config option. The
"filldir_t" typedef in fs.h is such an example - it's not really opaque,
_nor_ is it a config option, but it sure as hell would be inconvenient for
all low-level filesystems to do
int my_readdir(struct file *filp, void *dirent,
int (*filldir)(void *, const char *, int, loff_t,
u64, unsigned))
{
...
}
because let's face it, having to write out that "filldir" type just made
me use two lines (and potential for totally unnecessary tupos) because the
thing was so complex. So at that point, using a typedef is just common
sense, and we can do
int my_readdir(struct file *filp, void *dirent, filldir_t filldir)
{
...
}
instead.
But it's really quite hard to make that kind of complex type in C. It's
almost always a function pointer that takes complex arguments.
[ That said, I generally won't _complain_ if people use typedefs, but on
the other hand, some people definitely are too eager to do it, and I'll
happily remove them if people send me a patch. For example, we used to
have "task_t" for "struct task_struct", and that was just _unnecessary_,
and made it just harder to pick out what it was. Sometimes long names
and the explicit "struct" is a _good_ thing. ]
One final thing: for _small_ structures, typedefs are much better than for
large ones. Why? Because of stack usage. I want people to really _think_
about local variable sizes, and that's one thing that a typedef sometimes
causes - especially if it's opaque, so that users don't have any "handle"
on whether it is big or small, it's really nasty to use them for automatic
storage on the stack, because you may simply blow your stack usage on a
single (or a couple) of structures.
Making things be "struct something_or_other" makes at least _me_ think
more about it than if it's "file_t". Maybe it's just me, but I immediately
think "complex structure" when I see "struct", but "file_t" to me mentally
says "single word".
Linus