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
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
