git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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).

Previous: Stefan BellerNext: Jeff King
Message 9 of 13 in “[PATCHv13 0/7] Expose submodule parallelism to the user”
  1. Stefan BellerFeb 18, 2016
  2. 1/7 submodule-config: keep update strategy aroundStefan Beller, Feb 18, 2016
  3. 2/7 submodule-config: drop check against NULLStefan Beller, Feb 18, 2016
  4. 3/7 fetching submodules: respect `submodule.fetchJobs` config optionStefan Beller, Feb 18, 2016
  5. 4/7 submodule update: direct error message to stderrStefan Beller, Feb 18, 2016
  6. 5/7 git submodule update: have a dedicated helper for cloningStefan Beller, Feb 18, 2016
  7. Jeff KingFeb 19, 2016
  8. Stefan BellerFeb 19, 2016
  9. Eric SunshineFeb 19, 2016
  10. Jeff KingFeb 19, 2016
  11. Stefan BellerFeb 19, 2016
  12. 6/7 submodule update: expose parallelism to the userStefan Beller, Feb 18, 2016
  13. 7/7 clone: allow an explicit argument for parallel submodule clonesStefan Beller, Feb 18, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.