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

Re: [PATCH 3/4] describe: use argv-array

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 9, 2013, 16:00 UTC
Message-ID
<7vehb7wwob.fsf@alter.siamese.dyndns.org>
In-Reply-To
<7vip0jwzou.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
Show 27 quoted lines
> Jeff King <peff@peff.net> writes:
>
>> On Sun, Jul 07, 2013 at 03:33:43PM -0700, Junio C Hamano wrote:
>>
>>> +		argv_array_init(&args);
>>> +		argv_array_push(&args, "name-rev");
>>> +		argv_array_push(&args, "--name-only");
>>> +		argv_array_push(&args, "--no-undefined");
>>> [...]
>>> -		memcpy(args + i, argv, argc * sizeof(char *));
>>> -		args[i + argc] = NULL;
>>> -		return cmd_name_rev(i + argc, args, prefix);
>>> +		return cmd_name_rev(args.argc, args.argv, prefix);
>>
>> This leaks the memory allocated by "args". The original did, too, and it
>> is probably not that big a deal (we exit right after anyway). The fix
>> would be something like:
>>
>>   rc = cmd_name_rev(args.argc, args.argv, prefix);
>>   argv_array_clear(&args);
>>   return rc;
>
> Yes; this was meant as a straight rewrite and I did not bother, but
> I should have cleaned it up as I meant to build on top.
>
> Will amend, even though I do not think we need to build anything on
> top.

Heh, you fooled me. cmd_name_rev() uses the usual parse-options machinery that updates args.argv[]. Dashed options that were consumed will not remain in args.argv[] and argv_array_clear() will not have a chance to free them, and besides, args.argc and args.argv will be out of sync and wreaks havoc in argv_array_clear().

We could expose argv_array_push_nodup() and use it in this caller and then free the args.argv[] but not its contents, but I do not think it is worth it.

Previous: Junio C HamanoNext: Jeff King
Message 11 of 20 in “Make "git name-rev $(git rev-parse v1.8.3)" work”
  1. 0/4 Make "git name-rev $(git rev-parse v1.8.3)" workJunio C Hamano, Jul 7, 2013
  2. 1/4 name-ref: factor out name shortening logic from name_ref()Junio C Hamano, Jul 7, 2013
  3. Michael HaggertyJul 8, 2013
  4. Junio C HamanoJul 8, 2013
  5. 2/4 name-rev: allow converting the exact object name at the tip of a refJunio C Hamano, Jul 7, 2013
  6. Ramkumar RamachandraJul 8, 2013
  7. Junio C HamanoJul 8, 2013
  8. 3/4 describe: use argv-arrayJunio C Hamano, Jul 7, 2013
  9. Jeff KingJul 9, 2013
  10. Junio C HamanoJul 9, 2013
  11. Junio C HamanoJul 9, 2013
  12. Jeff KingJul 9, 2013
  13. 4/4 describe/name-rev: tell name-rev to peel the incoming object to commit firstJunio C Hamano, Jul 7, 2013
  14. Ramkumar RamachandraJul 8, 2013
  15. Jeff KingJul 9, 2013
  16. Jeff KingJul 9, 2013
  17. Junio C HamanoJul 9, 2013
  18. Jeff KingJul 9, 2013
  19. Junio C HamanoJul 9, 2013
  20. Ramkumar RamachandraJul 9, 2013

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.