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

From: Junio C Hamano <[email-masked]>
Date: Mon, 30 Sep 2019 10:36:39 +0900

Alex Henrie <[email-masked]> 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.

> Signed-off-by: Alex Henrie <[email-masked]>
> ---
>  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;