Re: [PATCH v2 1/3] refs: keep track of unresolved reference value in iterator

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

Patrick Steinhardt <ps@pks.im> writes:

>>  const char *refs_resolve_ref_unsafe(struct ref_store *refs,
>>  				    const char *refname,
>> +				    char **referent,
>>  				    int resolve_flags,
>>  				    struct object_id *oid,
>>  				    int *flags)
>
> I wonder whether this really should be a `char **`. You don't seem to be
> free'ing returned pointers anywhere, so this should probably at least be
> `const char **` to indicate that memory is owned by the ref store. But
> that would require the ref store to inevitably release it, which may
> easily lead to bugs when the caller expects a different lifetime.
>
> So how about we instead make this a `struct strbuf *referent`? This
> makes it quite clear who owns the memory. Furthermore, if the caller
> wants to resolve multiple refs, it would allow them to reuse the buffer
> for better-optimized allocation patterns.

Or return an allocated piece of memory via "char **".  I think an
interface that _requires_ the caller to use strbuf is not nice, if
the caller is not expected to further _edit_ the returned contents
using the strbuf API.  If it is likely that the caller would want to
perform further post-processing on the result, an interface based on
strbuf is nice, but I do not think it applies to this case.