Re: [PATCH 01/11] diffcore-break: guard against NULLed queue entries in merge loop
From: Junio C Hamano <hidden>
Date: 2026-07-10 03:11:05
"Johannes Schindelin via GitGitGadget" [off-list ref] writes:
From: Johannes Schindelin <redacted> 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.
quoted hunk ↗ jump to hunk
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 <redacted> --- 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)) {