Re: [PATCH v4 1/4] rebase: start implementing it as a builtin

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 9, 2018, 08:36 UTC
Message-ID: <CAPig+cQopjftfSoPHPZQAzECTAUUwZ-pXYMeWEV=VJBFm63t9g@mail.gmail.com>
In-Reply-To: <857b04d8-37b4-596f-6f3e-45b6e2a59442@gmail.com>

On Mon, Jul 9, 2018 at 3:59 AM Andrei Rybak <rybak.a.v@gmail.com> wrote:

Show 14 quoted lines

> On 2018-07-08 20:01, Pratik Karki wrote:
> > +static int use_builtin_rebase(void)
> > +{
> > +     struct child_process cp = CHILD_PROCESS_INIT;
> > +     struct strbuf out = STRBUF_INIT;
> > +     int ret;
> > +
> > +     argv_array_pushl(&cp.args,
> > +                      "config", "--bool", "rebase.usebuiltin", NULL);
> > +     cp.git_cmd = 1;
> > +     if (capture_command(&cp, &out, 6))
> > +             return 0;
>
> Does strbuf out leak on return here?

Good catch. This _is_ a potential leak. Here is an excerpt from the documentation of pipe_command(), which is called by capture_command():

    Any output collected in the buffers is kept even if the
    command returns a non-zero exit.

So, yes, this needs a strbuf_release() before returning.

Show 5 quoted lines

> > +     strbuf_trim(&out);
> > +     ret = !strcmp("true", out.buf);
> > +     strbuf_release(&out);
> > +     return ret;
> > +}