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
