From: Jeff King <peff@peff.net>
Date: Fri, 19 Feb 2016 19:08:21 GMT
Subject: Re: [PATCHv13 5/7] git submodule update: have a dedicated helper for cloning
Message-ID: <20160219190821.GA777@sigill.intra.peff.net>
In-Reply-To: <CAPig+cR6SKwM7x=n6fdZDscm2GLTNQq-ZNvB-ELqAk6dk1jGFw@mail.gmail.com>

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.

-Peff