Jeff King <peff@xxxxxxxx> writes:
> 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.