From: Jeff King <peff@peff.net>
Date: Fri, 10 May 2013 17:00:17 GMT
Subject: Re: [PATCH] Support running an arbitrary git action through checkout
Message-ID: <20130510170016.GA29848@sigill.intra.peff.net>
In-Reply-To: <2d5cfb3be9487f607051cad3d5230434660307ba.1368198269.git.trast@inf.ethz.ch>

On Fri, May 10, 2013 at 05:06:07PM +0200, Thomas Rast wrote:

> +	if (argc > 1 && !prefixcmp(argv[1], "--")) {
> +		const char *subcommand = argv[1] + 2;
> +		struct cmdnames main_cmds, other_cmds;
> +
> +		memset(&main_cmds, 0, sizeof(main_cmds));
> +		memset(&other_cmds, 0, sizeof(other_cmds));
> +
> +		load_command_list("git-", &main_cmds, &other_cmds);
> +
> +		if (is_in_cmdlist(&main_cmds, subcommand) ||
> +		    is_in_cmdlist(&other_cmds, subcommand)) {
> +			const char **args = xmalloc((argc) * sizeof(char*));
> +			args[0] = subcommand;
> +			memcpy(args+1, argv+2, argc*sizeof(char*));
> +			args[argc] = NULL;

Doesn't this memcpy overflow args, since it is only argc slots long? I
think you want to copy only (argc-1) slots, since you are omitting both
argv[0] as well as the "--subcommand" argument in argv[1]. You can also
drop the setting of NULL, as you will copy the original NULL as the last
item in your memcpy.

Other than that, the patch looks brilliant. :)

-Peff