Re: [patch 143/166] preadv/pwritev: Add preadv and pwritev system calls.

Linus Torvalds
2009-04-04T17:58:26Z

On Sat, 4 Apr 2009, Andrew Morton wrote:
>
> Does C promote the `unsigned long high' beforehand, or will the
> intermediate expression overflow?

Thanks, good catch. Will fix. Needs the cast - I'm sure I had it at one
point, but it got lost..

[ looks around ]

Heh. I had the cast in my original email that wasn't a patch:

#define HALF_LONG (BITS_IN_LONG / 2)

offset = (((loff_t)high << HALF_LONG) << HALF_LONG) | low;

but then when I actually turned it into a patch, I had lost it:

+#define HALF_LONG_BITS (BITS_PER_LONG / 2)
+ return ((high << HALF_LONG_BITS) << HALF_LONG_BITS) | low;

(but at least I had fixed the "BITS_IN_LONG" to "BITS_PER_LONG").

> I think it's wrong on 32-bit?
>
> Also, HALF_LONG_BITS is 32 on 64-bit, so "high" gets shifted to zero.
> It's unclear whether this was deliberate, but either way, it's a sneaky
> trick and deserves a code comment!

It was very much deliberate - the high bits disappear, since we don't have
128-bit loff_t.

And it's not a sneaky trick, it's totally standard programming. We have
the same thing in a few other places:

drivers/net/starfire.c: writel( (np->queue_mem_dma >> 16) >> 16, ioaddr + RxDescQHiAddr);

Notice the ">> 16) >> 16" thing? That's a way to write ">> 32" without
having the undefined overflow in 32 bits - if we have just a 32-bit dma
address type, it just turns into "high bits are 0".

Linus