Hi! On 16/10/12 00:05, Ryan Mallon wrote: >>> This looks broken. If gbc was re-alloced above (index < 0) then >>> gbc->remap == NULL and this will oops? >> >> No, because I took care that even though index can be < 0, the resulting >> pointer is never dereferenced for -1. > > Ah, I see. I think its a bit non-obvious and flaky though, since it > looks like you are both dereferencing a potentially NULL pointer, and > indexing an array with -1. > > Even changing it to this I think makes it a bit more clear: > > if (gbc->remap == 0 || > bit - i != gbc->remap[gbc->nremap - 1].offset) > gbc->nremap++; > gbc->remap = krealloc(...); > ... > > If you want to keep your way, at the very least I think it deserves a > comment, since it is easy to misread.
Yes, commenting it now. Note that it's rather out-of-bounds than NULL pointer. (But with similar results, though. ;-) ) >> For this, the current API is working fine, even enabling userland access >> via sysfs. > > Fair enough. I didn't see the first round of patches. You probably can > still use for_each_set_bit though (maybe convert the mask to unsigned > long first to match the bitops API): > > for_each_set_bit(j, &gbc->mask, BITS_PER_LONG) > ... Thanks for the hint! Useful. Roland -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
