[whoops, resending as non-HTML mail] On Sun, Jan 21, 2018 at 11:40 AM, Russell King - ARM Linux <linux@xxxxxxxxxxxxxxx> wrote: > On Sat, Jan 20, 2018 at 01:06:09PM -0800, Dan Williams wrote: >> +/* >> + * If idx is negative or if idx > size then bit 63 is set in the mask, >> + * and the value of ~(-1L) is zero. When the mask is zero, bounds check >> + * failed, array_ptr will return NULL. > > The more times I see this the more times I'm unhappy with this comment: > > 1. does this really mean "idx > size" or "idx >= size" ? The code > implements the latter not the former. Copying the code here for context: return ~(long)(idx | (sz - 1 - idx)) >> (BITS_PER_LONG - 1); That part of the condition (ignoring the overflow edgecases) is equivalent to "!(idx > sz - 1)", which is equivalent to "idx <= sz - 1", which is (ignoring overflow edgecases) equivalent to "idx < sz". Handling of edgecases: idx>=2^(BITS_PER_LONG-1) will cause a NULL return through the first part of the condition. Hmm... a problematic case might be "sz==0 && idx==2^(BITS_PER_LONG-1)-1". The first part of the expression wouldn't trigger, the second part would be "2^(BITS_PER_LONG)-1-(2^(BITS_PER_LONG-1)-1) == 2^(BITS_PER_LONG)-2^(BITS_PER_LONG-1) == 2^(BITS_PER_LONG-1)", which also wouldn't trigger, I think?
