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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 19, 2016 at 11:47 AM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> On Fri, Feb 19, 2016 at 4:03 AM, Jeff King <peff@xxxxxxxx> 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).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]