On Sat, Dec 3, 2016 at 12:24 PM, Jeff King <peff@xxxxxxxx> wrote: > On Fri, Dec 02, 2016 at 09:31:04PM +0100, Rasmus Villemoes wrote: > >> It seems to be odd to do x=y if x==y. Maybe there's a bug somewhere near >> this, but as is this is somewhat confusing. > > Yeah, this code is definitely wrong, but I'm not sure what it's trying > to do. This is the first time I've looked at it. I'm sorry I don't know why it's there either :( The first version that has this was v3 [1] which still uses "util" pointdf instead of the commit slab but the logic does not differ much. This is the place when we "paint" the parent commit with the same bitmap as the child I mentioned earlier (though I think I mentioned it backward). You see similar code in the same loop just a bit earlier: if the commit has not been painted, it gets a new bitmap, otherwise new refs are OR'd to its bitmap. It's the OR part (when the bitmaps pointed by *p_refs and *refs differ, it's not just about pointer comparison) that's probably missing here. But it looks like we can safely delete the " || *p_refs == *refs" part because the commit in question is inserted back to the commit list "head" and revisited in the next iteration. If its bitmap is different from the child's, then in the next iteration it should hit the "if (memcmp(tmp, *refs, bitmap_size))" line above, in the same loop, then the new bit will be added. If it's marked UNINTERESTING though, that won't happen. I'll need more time to stare at this code... [1] http://public-inbox.org/git/1385351754-9954-9-git-send-email-pclouds@xxxxxxxxx/ -- Duy