On Sun, Jul 8, 2012 at 7:32 PM, Peter Maydell <peter.mayd...@linaro.org> wrote: > 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.
64 - INT_MAX = 0x80000040 (maybe off by one), which should still trigger assert(INT_MAX <= 0x80000040) > > -- PMM