Jonathan Nieder <jrnieder@xxxxxxxxx> writes: >> +static void add_refs_to_oidset(struct oidset *oids, struct ref *refs) >> +{ >> + for (; refs; refs = refs->next) >> + oidset_insert(oids, &refs->old_oid); >> +} >> + >> +static int tip_oids_contain(struct oidset *tip_oids, >> + struct ref *unmatched, struct ref *newlist, >> + const struct object_id *id) >> +{ >> + if (!tip_oids->map.cmpfn) { > > This feels like a layering violation. Could it be e.g. a static inline > function oidset_is_initialized in oidset.h? Surely it does. Also, tip_oids_contain() uses unmatched and newlist only on the first call, but the internal API this patch establishes gives an illusion (confusion) that updating unmatched and newlist while making repeated to calls to this function may affect the outcome of tip_oids_contain() function. In fact, doesn't the loop that calls this function extend "newlist" by extending the list at its tail?