On Sun, Sep 29, 2019 at 7:36 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Alex Henrie <alexhenrie24@xxxxxxxxx> writes: > > > The condition "if (q->nr <= j)" checks whether the loop exited normally > > or via a break statement. This check can be avoided by replacing the > > jump out of the inner loop with a jump to the end of the outer loop. > > > > With the break replaced by a goto, the two diff_q calls then can be > > replaced with a single diff_q call outside of the outer if statement. > > I doubt that it is a good idea to do these two things. Especially I > do not see why the latter makes the resulting code better. Well, I admit that code clarity is somewhat subjective. To me it's not obvious that "if (q->nr <= j)" means "if the loop exited normally", but a goto does make it obvious. (And it's definitely more clear to scan-build, which complains about a possible memory leak when an if statement is used but does not complain when the if statement is replaced with a goto.) As far as the diff_q calls, I think that having one call instead of two is slightly more readable, but I don't care very much about it. I'd be happy to drop that change from the next version of the patch. -Alex
