Re: [PATCHv13 5/7] git submodule update: have a dedicated helper for cloning

From: Stefan Beller <sbeller@google.com>
Date: Feb 19, 2016, 16:47 UTC
Message-ID: <CAGZ79kbp+A1J4isLGftMiA2UZM16NO6nUycraj4U==2Zv6kHVQ@mail.gmail.com>
In-Reply-To: <20160219120310.GB10204@sigill.intra.peff.net>

On Fri, Feb 19, 2016 at 4:03 AM, Jeff King <peff@peff.net> wrote:

Show 22 quoted lines

> On Thu, Feb 18, 2016 at 03:33:16PM -0800, Stefan Beller wrote:
>
>> +     if (needs_cloning) {
>> +             cp->git_cmd = 1;
>> +             cp->no_stdin = 1;
>> +             cp->stdout_to_stderr = 1;
>> +             cp->err = -1;
>> +             argv_array_push(&cp->args, "submodule--helper");
>> +             argv_array_push(&cp->args, "clone");
>> +             if (pp->quiet)
>> +                     argv_array_push(&cp->args, "--quiet");
>> +
>> +             if (pp->prefix)
>> +                     argv_array_pushl(&cp->args, "--prefix", pp->prefix, NULL);
>> +
>> +             argv_array_pushl(&cp->args, "--path", sub->path, NULL);
>> +             argv_array_pushl(&cp->args, "--name", sub->name, NULL);
>> +             argv_array_pushl(&cp->args, "--url", strdup(url), NULL);
>
> No need to strdup() here; argv_array handles its own memory, so this
> just leaks (and if we were keeping it, it should be xstrdup(), of
> course).

We cannot remove the strdup as the url is a local variable we read in from git_config_get_string and the local variable is going out of scope before the child process ends?

I'll change it to xstrdup then.

>
> -Peff