Patrick Steinhardt <ps@xxxxxx> 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.
