Subject: Re: [RFC/PATCH 09/11] branch.c: use 'ref-filter' data structures

From: Matthieu Moy <[email-masked]>
Date: Wed, 29 Jul 2015 12:08:14 +0200

Karthik Nayak <[email-masked]> writes:

> On Tue, Jul 28, 2015 at 7:18 PM, Matthieu Moy
> <[email-masked]> wrote:
>> Christian Couder <[email-masked]> writes:
>>
>>> On Tue, Jul 28, 2015 at 8:56 AM, Karthik Nayak <[email-masked]> wrote:
>>>
>>>> +static void ref_array_append(struct ref_array *array, const char *refname)
>>>> +{
>>>> +       size_t len = strlen(refname);
>>>> +       struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1);
>>>> +       memcpy(ref->refname, refname, len);
>>>> +       ref->refname[len] = '\0';
>>
>> This looks very much like new_ref_array_item, except that the later also
>> takes an objectname parameter. I find it suspicious that you leave the
>> objectname field uninitialized.
>>
>
> Well the objectname is not required here, the idea is to retain the way branch.c
> works.
>
>> Why is this code not calling new_ref_array_item?
>>
>
> Because no objectname is there.

You do have the object_id in the scope of the call-site, so why not use
it?

(Well, in any case, do as you think is best, it's temporary throw-away
code, we shouldn't loose too much time polishing it)

>> The function disapears in the next commit, but I also think that this
>> function deserves to exist in ref-filter.{c,h} and remain after the end
>> of the series.
>>
>
> Why though? I don't see the need when new_ref_array_item() is present.

OK, if the function is specificly for "append an item but leave the
objectname uninitialized", it's too specific to be useful somewhere
else. But then, make it more explicit in the name of the function and/or
in a comment.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/