torvalds@transmeta.com (Linus Torvalds) wrote on 25.07.00 in <Pine.LNX.4.10.10007251817060.893-100000@penguin.transmeta.com>:
> On Wed, 26 Jul 2000, Jamie Lokier wrote:
> >
> > 1. Make the name explicit: inode->i_union->bdev.
> >
> > 2. Change all the code that refers to the newly unioned fields.
> > It should be clear from the name i_union that no code should
> > read a field until it knows the type of the inode.
> >
> > 3. Do something similar for struct page. At least that would
> > remove the ugly overloading of page->next_hash fields on the Sparc.
>
> I hate unions that have to do it this way.
>
> Unions are imho only acceptable when they implicitly know their own type.
> The in-kernel example of this is the inode per-filesystem thing. An inode
> has a filesystem-specific part, and there is no way any other filesystem
> can access it except by a major bug somewhere.
Think of it as object oriented design. The filesystem-specific part is
exactly equivalent to deriving a filesystem specific class from a generic
class which may add a few instance variables.
That's *why* it works well. If other unions can be made to work the same
way, they, too, will work well -- if not, not.
Basically, unions only really work well for stuff that an OO language does
with subclasses. They *can* be made to work for other stuff, but that can
get ugly fast - there's some situations where you just have no good
choice, but avoid it where you can.
> Any union that needs code like
>
> if (xxx->type == yyy)..
> ....
>
> is a design mistake.
Not necessarily. That is, if you need that if statement *anyway*, then
it's typically ok (for the same reason it's ok in the class-like context -
basically, you only ever want to access unionized fields from code that's
naturally specific to this union variant), but if you need it only to
access the union, then you lose.
The problem with the if variant is that it's quite a bit harder to make
certain you are in the good situation.
A case that works would be if you get commands from the network, and the
command determines the union variant (which holds the parameters). You
branch on the command (need to do that anyway) and then take the
parameters for that command.
If there's something you need to do for all (or even many) commands,
though, you pray that it's not inside the union.
> I don't think you'll find all that many unions in Linux. And I don't think
> we should add new ones..
... except in well-understood situations. That means no "these probably
won't be needed at the same time, but I didn't bother to look at all the
source".
MfG Kai
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Mon Jul 31 2000 - 21:00:21 EST