Re: [PATCH] git cherry-pick: Add NULL check to sequencer parsing of HEAD

> 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.