Messages in this thread | | | Date | Thu, 18 May 2006 15:41:57 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: [PATCH] sector_t overflow in block layer |
| |
On Thu, 18 May 2006, Anton Altaparmakov wrote: > > I think you missed that Andrewas said he is worried about 64-bit overflows > as well.
Ahh. Ok. However, then the test _really_ should be something like
sector_t maxsector, sector; int sector_shift = get_sector_shift(bh->b_size);
maxsector = (~(sector_t)0) >> sector_shift; if (unshifted_value > maxsector) return -EIO; sector = (sector_t) unshifted_value << sector_shift;
which is a lot clearer, and likely faster too, with a proper get_sector_shift. Something like this:
/* * What it the shift required to turn a bh of size * "size" into a 512-byte sector count? */ static inline int get_sector_shift(unsigned int size) { int shift = -1; unsigned int n = 256;
do { shift++; } while ((n += n) < size); return shift; }
which should generate good code on just about any architecture (it avoids actually using shifts on purpose), and I think the end result will end up being more readable (I'm not claiming that the "get_sector_shift()" implementation is readable, I'm claiming that the _users_ will be more readable).
Of course, even better would be to not have "b_size" at all, but use "b_shift", but we don't have that. And the sector shift calculation might be fast enough that it's even a win (turning a 64-bit multiply into a shift _tends_ to be better)
Linus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |