Re: [PATCH v3 0/2] patch-id for merges

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 12, 2016, 20:44 UTC
Message-ID: <xmqqpoo8zw3a.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160912175641.nj4h6oupbwaedwbx@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 19 quoted lines

> On Mon, Sep 12, 2016 at 10:18:33AM -0700, Junio C Hamano wrote:
>
>> > +static int patch_id_defined(struct commit *commit)
>> > +{
>> > +	/* must be 0 or 1 parents */
>> > +	return !commit->parents || !commit->parents->next;
>> > +}
>> 
>> If we make the first hunk begin like so:
>> 
>> > +	if (commit->parents) {
>> > +		if (!patch_id_defined(commit))
>> > +			return -1;
>> 
>> I wonder if the compiler gives us the same code.
>
> Good idea. I actually put the "patch_id_defined" check outside the "if"
> block you've quoted (otherwise we're making assumptions about the
> contents of patch_id_defined).

Facepalm. I was mis-reading the condition in the helper function. Of course, guarding up-front makes more sense.