Subject: Re: [PATCH v5] fetch-pack: always allow fetching of literal SHA1s

From: Junio C Hamano <[email-masked]>
Date: Fri, 12 May 2017 15:01:35 +0900

Jonathan Nieder <[email-masked]> 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?