From: Stefan Beller <sbeller@google.com>
Date: Fri, 19 Feb 2016 19:10:37 GMT
Subject: Re: [PATCHv13 5/7] git submodule update: have a dedicated helper for cloning
Message-ID: <CAGZ79kY_qYdCgJ=8oQKdj4TF2gSaYwhE1c8KjnLwOiUFa9YjPg@mail.gmail.com>
In-Reply-To: <20160219190821.GA777@sigill.intra.peff.net>

On Fri, Feb 19, 2016 at 11:08 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Feb 19, 2016 at 12:20:48PM -0500, Eric Sunshine wrote:
>
>> >>> +             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).
>
> Exactly. :) Sorry for not being clear the first time around.

Sorry for being confused to easily. I should have just read the code of
argv_array_pushl before answering. In the resend I removed the strdup.

Thanks,
Stefan

>
> -Peff