Re: [PATCH v4] diffcore-break: use a goto instead of a redundant if statement
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 30, 2019, 01:36 UTC
- Message-ID
- <xmqqa7amimzc.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20190929204322.1244907-1-alexhenrie24@gmail.com>
Alex Henrie <alexhenrie24@gmail.com> writes:
Show 6 quoted lines
> 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.
Show 33 quoted lines
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> > --- > diffcore-break.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/diffcore-break.c b/diffcore-break.c > index 875aefd3fe..ee7519d959 100644 > --- a/diffcore-break.c > +++ b/diffcore-break.c > @@ -286,17 +286,15 @@ void diffcore_merge_broken(void) > /* Peer survived. Merge them */ > merge_broken(p, pp, &outq); > q->queue[j] = NULL; > - break; > + goto next; > } > } > - if (q->nr <= j) > - /* The peer did not survive, so we keep > - * it in the output. > - */ > - diff_q(&outq, p); > + /* The peer did not survive, so we keep > + * it in the output. > + */ > } > - else > - diff_q(&outq, p); > + diff_q(&outq, p); > +next:; > } > free(q->queue); > *q = outq;