From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: Thu, 03 May 2012 11:45:54 GMT
Subject: Re: [PATCH] git cherry-pick: Add NULL check to sequencer parsing of HEAD
Message-ID: <4FA26FF2.2050607@lsrfire.ath.cx>
In-Reply-To: <1336044026-16897-1-git-send-email-nhorman@tuxdriver.com>

Am 03.05.2012 13:20, schrieb Neil Horman:
> Michael Mueller noted that a feature I recently added failed to check the return
> of lookup_commit to ensure that it was not NULL.  I don't think a NULL can
> actually happen in the this particular use case, but regardless it seems a good
> idea to check.
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> ---
>   sequencer.c |   11 ++++++++++-
>   1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index f83cdfd..ad4d781 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -261,7 +261,16 @@ static int is_index_unchanged(void)
>   		return error(_("Could not resolve HEAD commit\n"));
>
>   	head_commit = lookup_commit(head_sha1);
> -	if (!head_commit || parse_commit(head_commit))
> +
> +	/*
> +	 * If head_commit is NULL, just return, as check_commit,
> +	 * called from lookup_commit, would have indicated that
> +	 * head_commit is not a commit object already.
> +	 */
> +	if (!head_commit)
> +		return;

A return value is missing.  Perhaps -1?

> +
> +	if (parse_commit(head_commit))
>   		return error(_("could not parse commit %s\n"),
>   			     sha1_to_hex(head_commit->object.sha1));

Note: parse_commit() can handle NULL, and it already reports error 
details itself.

René