Re: [PATCHv13 5/7] git submodule update: have a dedicated helper for cloning
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Feb 19, 2016, 17:20 UTC
- Message-ID
- <CAPig+cR6SKwM7x=n6fdZDscm2GLTNQq-ZNvB-ELqAk6dk1jGFw@mail.gmail.com>
- In-Reply-To
- <CAGZ79kbp+A1J4isLGftMiA2UZM16NO6nUycraj4U==2Zv6kHVQ@mail.gmail.com>
On Fri, Feb 19, 2016 at 11:47 AM, Stefan Beller <sbeller@google.com> wrote:
Show 29 quoted lines
> On Fri, Feb 19, 2016 at 4:03 AM, Jeff King <peff@peff.net> wrote:
>> 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.When Peff said "argv_array handles its own memory", he meant that it does xstrdup() itself, so there's no need for you to do so a second time manually (leaking a string as a consequence).