Re: [PATCH 01/11] diffcore-break: guard against NULLed queue entries in merge loop

[Junio C Hamano <gitster@pobox.com>]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The outer loop in `diffcore_merge_broken()` sets `q->queue[j]` to NULL
> when it merges a broken pair back together, and has a NULL check to skip
> such entries on subsequent iterations. The inner loop, however, lacks
> this guard: when it scans forward looking for a matching peer, it can
> encounter a slot that was NULLed by a previous outer-loop iteration and
> dereference it unconditionally.
>
> In practice this requires at least two broken pairs whose peers
> both survive rename/copy detection and appear later in the queue,
> which is rare but not impossible.

Interesting find.  This is an ancient part of the codebase that
nobody has touched in the past 21 years since eeaa460314 ([PATCH]
diff: Update -B heuristics., 2005-06-03) introduced it ;-).

Well spotted.

> Add the same `if (!pp) continue` guard to the inner loop.
>
> Pointed out by Coverity.
>
> Assisted-by: Claude Opus 4.6
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  diffcore-break.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/diffcore-break.c b/diffcore-break.c
> index 17b5ad1fed..b5bcc956cc 100644
> --- a/diffcore-break.c
> +++ b/diffcore-break.c
> @@ -289,6 +289,8 @@ void diffcore_merge_broken(void)
>  			 */
>  			for (j = i + 1; j < q->nr; j++) {
>  				struct diff_filepair *pp = q->queue[j];
> +				if (!pp)
> +					continue;
>  				if (pp->broken_pair &&
>  				    !strcmp(pp->one->path, pp->two->path) &&
>  				    !strcmp(p->one->path, pp->two->path)) {