Re: [PATCH v8 18/19] branch: use ref-filter printing APIs

[Jeff King <peff@peff.net>]

On Mon, Dec 12, 2016 at 09:59:49PM +0530, Karthik Nayak wrote:

> >> > This caller never stores the return value, and it ends up leaking. So I
> >> > wonder if you wanted "static struct strbuf" in the first place (and that
> >> > would explain the strbuf_reset() in your function).
> >>
> >> Ah! Yes this should be 'static struct strbuf' indeed, I blindly copied Junio's
> >> suggestion.
> >>
> >> strbuf_detach() is also a better way to go.
> >
> > One of the other, though. If it's static, then you _don't_ want to
> > detach.
> >
> 
> Wait. Why not? since it gets added to the strbuf's within
> build_format() and that
> value is not needed again, why do we need to re-allocate? we can use the same
> variable again (i.e by keeping it as static).
> 
> I'm sorry, but I didn't get why these two should be mutually exclusive?

What is the memory ownership convention for the return value from the
function?

If the caller should own the memory, then you want to do this:

  char *foo(...)
  {
	struct strbuf buf = STRBUF_INIT;
	... fill up buf ...
	return strbuf_detach(&buf, NULL);
  }

The "detach" disconnects the memory from the strbuf (which is going out
of scope anyway), and the only pointer left to it is in the caller. It's
important to use "detach" here and not just return the pointer, because
that ensures that the returned value is always allocated memory (and
never strbuf_slopbuf).

If the caller should not own the memory, then the function retains
ownership. And you want something like this:

  char *foo(...)
  {
	static struct strbuf buf = STRBUF_INIT;
	strbuf_reset(&buf);
	... fill up buf ...
	return buf.buf;
  }

The same buffer is reused over and over. The "reset" call clears any
leftover bits from the last invocation, and you must _not_ call
strbuf_detach() in the return, as it disconnects the memory from the
strbuf (and so next time you'd end up allocating again, and each return
value becomes a memory leak).

Does that make sense?

-Peff