On 8 July 2012 20:12, Blue Swirl <blauwir...@gmail.com> wrote: > On Sun, Jul 8, 2012 at 6:54 PM, Peter Maydell <peter.mayd...@linaro.org> > wrote: >> On 8 July 2012 19:32, Blue Swirl <blauwir...@gmail.com> wrote: >>> On Sun, Jul 8, 2012 at 2:04 PM, Peter Maydell <peter.mayd...@linaro.org> >>> wrote: >>>> On 8 July 2012 13:12, <blauwir...@gmail.com> wrote: >>>>> -static inline uint64_t deposit64(uint64_t value, int start, int length, >>>>> - uint64_t fieldval) >>>>> +static inline uint64_t deposit64(uint64_t value, unsigned int start, >>>>> + unsigned int length, uint64_t fieldval) >>>>> { >>>>> uint64_t mask; >>>>> - assert(start >= 0 && length > 0 && length <= 64 - start); >>>>> + assert(length > 0 && length <= 64 - start); >>>> >>>> This breaks the assertion (consider the case of start == UINT_MAX >>>> and length == 64). >>> >>> The original is equally buggy in other cases since there is no bound >>> check for the upper limit. >> >> For what upper limit? Overlong length or start should both be caught >> by the third condition in the signed case. > > Nice. Why is it written like that, I'd use > start + length <= 64?
That would fail to handle the case of start == length == INT_MAX. -- PMM