Jeff King <peff@xxxxxxxx> writes: > If the bloom filter also computes against an empty tree for root > commits (I didn't check, but that would make sense), I think that AND > could be an OR: > > if (!origin->commit->parents || > !oidcmp(...)) > > though it probably doesn't matter that much in practice. Root commits > are rather rare. Correct. I just followed the code from bloom.c::get_bloom_filter() down, and for a root commit, diff_tree_oid() with NULL in the first parameter (i.e. old_oid) is called. This NULL pointer eventually reaches tree-walk.c::fill_tree_descriptor() and the function just gives an empty tree in that case, which is what we want. > > BTW, we could also be using oideq() here. I thought coccicheck would > note this, but it doesn't seem to. I suspect we could also get away with > a direct pointer comparison of "parent == origin->commit->parents->item" > due to the way we allocate "struct commit", but I'd rather err on the > safer and less subtle side. :) True. oideq() is probably an improvement; I agree that pointer equality is taking it too far.