From: Jonathan Nieder <jrnieder@gmail.com>
Date: Tue, 01 Jun 2010 04:01:15 GMT
Subject: Re: [PATCH 2/8] revert: use run_command_v_opt() instead of execv_git_cmd()
Message-ID: <20100601040115.GA22401@progeny.tock>
In-Reply-To: <20100531194240.28729.9964.chriscool@tuxfamily.org>

Christian Couder wrote:

> diff --git a/builtin/revert.c b/builtin/revert.c
> index 5df0d69..9085894 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -530,7 +530,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
>  			args[i++] = defmsg;
>  		}
>  		args[i] = NULL;
> -		return execv_git_cmd(args);
> +		return run_command_v_opt(args, RUN_GIT_CMD);
>  	}
>  	free_message(&msg);
>  	free(defmsg);

Doesn’t this leak msg and defmsg?  Maybe it would make sense to free
the in-core copy of the commit message before the if (!no_commit)
block.

	int ret;

	...
	ret = 0;
	free_message(&msg);
	if (!no_commit)
		ret = commit_for_cherry_pick(signoff, edit, defmsg);
	free(defmsg);
	return ret;

Jonathan