Jeff King <peff@xxxxxxxx> writes:
> On Thu, Aug 23, 2012 at 10:10:29AM +0200, mhagger@xxxxxxxxxxxx wrote:
>
>> From: Michael Haggerty <mhagger@xxxxxxxxxxxx>
>>
>> Use the names (nr_heads, heads) consistently across functions, instead
>> of sometimes naming the same values (nr_match, match).
>
> I think this is fine, although:
>
>> --- a/builtin/fetch-pack.c
>> +++ b/builtin/fetch-pack.c
>> @@ -521,7 +521,7 @@ static void mark_recent_complete_commits(unsigned long cutoff)
>> }
>> }
>>
>> -static void filter_refs(struct ref **refs, int nr_match, char **match)
>> +static void filter_refs(struct ref **refs, int nr_heads, char **heads)
>> {
>> struct ref **return_refs;
>> struct ref *newlist = NULL;
>> @@ -530,12 +530,12 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
>> struct ref *fastarray[32];
>> int match_pos;
>
> This match_pos is an index into the "match" array, which becomes "head".
> Should it become head_pos?
>
> And then bits like this:
>
>> - while (match_pos < nr_match) {
>> - cmp = strcmp(ref->name, match[match_pos]);
>> + while (match_pos < nr_heads) {
>> + cmp = strcmp(ref->name, heads[match_pos]);
>
> Would be:
>
> while (head_pos < nr_heads)
>
> which makes more sense to me.
Using one name is better, but I wonder "heads" is the better one
between the two.
After all, this codepath is not limited to branches these days as
the word "head" implies. Rather, <nr_thing, thing> has what we
asked for, and <refs> has what the other sides have, and we are
trying to make sure we haven't asked what they do not have in this
function.
And the way we do so is to match the "thing"s with what are in
"refs" to find unmatched ones.
So between the two, I would have chosen "match" instead of "heads"
to call the "thing".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html