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. 2. is "bit 63" relevant here - what if longs are 32-bit? "the top bit" or "the sign bit" would be better. 3. "and the value of ~(-1L) is zero." So does this mean that when 0 <= idx < size, somehow the rules of logic change and ~(-1L) magically becomes no longer zero! I'd suggest changing the description to something like: * If 0 <= idx < size, return a mask of ~0UL, otherwise return zero. or: * When idx is out of bounds (iow, is negative or idx >= sz), the sign * bit will be set. Extend the sign bit to all bits and invert, giving * a result of zero for an out of bounds idx, or ~0UL if within bounds. depending on how deeply you want to describe what's going on here. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up
