Re: [PATCH v3] diffcore-break: use a goto instead of a redundant if statement

From: Alex Henrie <alexhenrie24@gmail.com>
Date: Sep 29, 2019, 20:10 UTC
Message-ID: <CAMMLpeQnVb1M_Wy_GDeH44YVweXvNQ2jpyJqtvJ4zS_fGfxKuw@mail.gmail.com>
In-Reply-To: <20190929093706.ylm5dsftwl2y2nnz@hashpling.org>

On Sun, Sep 29, 2019 at 3:37 AM CB Bailey <cb@hashpling.org> wrote:

Show 11 quoted lines

>
> Previously, if the condition matched in the inner loop, the function
> would null out the entry in the queue that that inner loop had reached
> (q->queue[j] = NULL) and then break out of the inner loop. This meant
> that the outer loop would skip over this entry (if (!p)).
>
> The change introduced seems to break out of both loops as soon as we
> reach one match, whereas before other subsequent matches would be
> considered and merged. Not only this, but the outer 'else' case for all
> subsequent entries is skipped so the rest of the entries the original
> queue are missing from 'outq'.

> I spent a bit of time trying to see if this change was user visible
> which turned out to be unneeded as t4008-diff-break-rewrite.sh already
> fails with this change for me in my environment, initially with this
> test but also 3 other tests in this file.

Thank you for reviewing this. I should have run `make test` myself before sending the patch; I do indeed see the same test failure that you saw. I will send a v4 of this patch with the label in the right place.

-Alex