Re: drivers/block/ub.c

From: Andries Brouwer
Date: Sun Jun 27 2004 - 10:26:01 EST


On Sun, Jun 27, 2004 at 04:24:18PM +0200, Oliver Neukum wrote:

> > > > The above writes clearly and simply what one wants.
> > > > I expect that you propose writing
> > > >
> > > >         *((u32 *)(cmd->cdb + 2)) = cpu_to_be32(block);
> > > >
> > > > or some similar unspeakable ugliness.
> > > > If you had something else in mind, please reveal what.
> > >
> > > That "ugliness" has the unspeakable advantage of producing sane code
> > > on big endian architectures.
> >
> > I am not so sure. It tells the compiler to do a 4-byte access
> > on an address that possibly is not 4-byte aligned.
>
> We also have the unaligned family of macro. Probably the cleanest
> solution would be a union to do away with the ugly casts that would
> be needed.

You see what is happening. Pete writes simple straightforward correct
code. You want to replace it by ugly code that is perhaps not 100%
correct. Then the ugliness spreads - not only a local cast, but
globally the data structures must be adapted. And would it help? What
padding do you get in that union? Do the details depend on the gcc
version? On the compilation flags?

Always write simple direct obvious code. Avoid all casts.
Uglifying code burdens maintenance. It endangers correctness.
It is reasonable only when efficiency is really important,
when every nanosecond counts (and the ugly code is really faster).
That is not the case here.


Andries

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