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

From: Jeff King <peff@peff.net>
Date: Dec 9, 2016, 14:03 UTC
Message-ID: <20161209140345.76ybodldmg2lxgbn@sigill.intra.peff.net>
In-Reply-To: <20161207153627.1468-19-Karthik.188@gmail.com>

On Wed, Dec 07, 2016 at 09:06:26PM +0530, Karthik Nayak wrote:

Show 18 quoted lines

> +const char *quote_literal_for_format(const char *s)
>  {
> +	struct strbuf buf = STRBUF_INIT;
>  
> +	strbuf_reset(&buf);
> +	while (*s) {
> +		const char *ep = strchrnul(s, '%');
> +		if (s < ep)
> +			strbuf_add(&buf, s, ep - s);
> +		if (*ep == '%') {
> +			strbuf_addstr(&buf, "%%");
> +			s = ep + 1;
> +		} else {
> +			s = ep;
> +		}
>  	}
> +	return buf.buf;
>  }

You should use strbuf_detach() to return the buffer from a strbuf. Otherwise it is undefined whether the pointer is allocated or not (and whether it needs to be freed).

In this case, if "s" is empty, buf.buf would point to a string literal, but otherwise to allocated memory. strbuf_detach() normalizes that.

But...

> +			    branch_get_color(BRANCH_COLOR_REMOTE), maxwidth, quote_literal_for_format(remote_prefix),

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).

-Peff