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

On Fri, Dec 9, 2016 at 7:33 PM, Jeff King [off-list ref] wrote:

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

quoted

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

quoted

+                         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

Ah! Yes this should be 'static struct strbuf' indeed, I blindly copied Junio's
suggestion.

strbuf_detach() is also a better way to go.

Thanks.

-- 
Regards,
Karthik Nayak