Re: [PATCH] get_ref_states: strdup entries and free util in stale list

[Junio C Hamano <gitster@pobox.com>]

Bert Wesarg <bert.wesarg@googlemail.com> writes:

> On Tue, Dec 1, 2009 at 01:21, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> Hmm, the Subject: matches what the code does, but nobody mentions why it
>> is necessary (iow, what breaks in the current code and what becomes better
>> if the patch is applied). The blank space before your "S-o-b:" line is
>> for you to describe these things.
> Sure. unfortunately the code where the string list is filled is not in
> the patch. But if you look at the code it should be self-explanatory.

That is _exactly_ why I want the description in the commit log message.  I
don't want commit messages (or lack thereof) to force people to look at
the code outside the patch.

Otherwise I'll have to ask _you_ to personally give the 7-line explanation
you just gave us to anybody who runs "git log -p" with the default context
size after this patch is applied.  I do not think you have the bandwidth
to handle that ;-).

> There is actually also an other solution: we could first strdup the
> ref name to the .util member and take this as the input for the
> abbrev_ref()/string list entry and safe there the strdup.

I too thought something like that may make sense, but it doesn't look like
so, for two reasons:

 - string-list API is a bit cumbersome to use if you allocate the string
   yourself.  You will be made responsible for freeing them, and often you
   do so by setting strdup_strings to true immediately before calling
   string_list_clear(), which is kind of ugly;

 - The ref abbrev_branch() is called and the address of whose substring is
   taken to be used as "name" in handle_one_branch() is refspec.src, but
   what goes to .util is refname that is refspec.dst---they are different
   strings and one is not a substring of the other.

IOW, your patch text is good; it just would have been better with a bit
more explanation.

Thanks, will queue in 'maint'.