On Mon, Aug 01, 2016 at 09:49:37PM +0000, Eric Wong wrote:

> +static void setup_pager_env(struct argv_array *env)
> +{
> +     const char *pager_env = PAGER_ENV;
> +
> +     while (*pager_env) {
> +             struct strbuf buf = STRBUF_INIT;
> +             const char *cp = strchrnul(pager_env, '=');
> +
> +             if (!*cp)
> +                     die("malformed build-time PAGER_ENV");
> +             strbuf_add(&buf, pager_env, cp - pager_env);
> +             cp = strchrnul(pager_env, ' ');
> +             if (!getenv(buf.buf)) {
> +                     strbuf_reset(&buf);
> +                     strbuf_add(&buf, pager_env, cp - pager_env);
> +                     argv_array_push(env, strbuf_detach(&buf, NULL));
> +             }

argv_array handles its own allocation, so this leaks the detached
strbuf.

You'd want:

  argv_array_push(env, buf.buf);
  strbuf_release(&buf);

or just:

  argv_array_pushf(env, "%.*s", (int)(cp - pager_env), pager_env);

Also:

> +             strbuf_reset(&buf);

should this be strbuf_release()? If we didn't follow the conditional
above (because getenv() told us the variable was already set), then we
would not do do the detach/release there, and would finish the loop with
memory still allocated by "buf".

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to