On 27 June 2012 12:39, Eric Blake <[email protected]> wrote: > On 06/27/2012 04:29 AM, Peter Maydell wrote: >> +static inline uint64_t field64(uint64_t value, int start, int length) >> +{ >> + assert(start >= 0 && start <= 63 && length > 0 && start + length <= 64); > > You're failing to account for wraparound: > > field64(value, 63, MAX_INT) > > gives undefined behavior in the addition, and even on (most) platforms > where it silently wraps around to a negative number, you have then > missed triggering the assert and proceed to do more unefined behavior > with a negative shift. You can solve that, and use one less conjunct, > by using: > > assert(start >= 0 && length > 0 && (unsigned) start + length <= 64);
Yes, that works (took me a minute to figure out that it relies on two positive ints not being able to overflow an unsigned int). -- PMM
