From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 03 May 2012 17:13:09 GMT
Subject: Re: [PATCH v3] git cherry-pick: Add NULL check to sequencer parsing of HEAD
Message-ID: <7vobq5fasa.fsf@alter.siamese.dyndns.org>
In-Reply-To: <vpqtxzx5hxz.fsf@bauges.imag.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Neil Horman <nhorman@tuxdriver.com> writes:
>
>> -	if (!head_commit || parse_commit(head_commit))
>> -		return error(_("could not parse commit %s\n"),
>> -			     sha1_to_hex(head_commit->object.sha1));
>> +
>> +	if (parse_commit(head_commit))
>> +		return -1;
>
> Why did you replace the error("...") with only a -1? error() also
> returns -1, but displays a message before, which I think was fine. If
> you want to remove the message, then explain why in the commit message.
>
> If you do not test for head_commit to be null, you can't use it in the
> error message. But from the context, it seems you can use head_sha1. If
> not, a message like "Could not parse HEAD commit" seems better than
> nothing.

Yeah, I think v2 in this series is the appropriate fix.