Junio C Hamano <gits...@pobox.com> writes:

> Jeff King <p...@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.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to