On Thu, May 03, 2012 at 01:45:54PM +0200, René Scharfe wrote:
> 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@xxxxxxxxxxxxx>
> >---
> > 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?
>
Yeah, sorry, not sure how I missed the compiler warning.
> >+
> >+ 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.
No, it doesn't. parse_commit checks NULL already, true, but it just returns -1.
No error message is provided to the user
Neil
>
> René
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html