Skip to content

Commit b2b5ad5

Browse files
peffgitster
authored andcommitted
diff: replace diff_options.dry_run flag with NULL file
We introduced a dry_run flag to diff_options in b55e6d3 (diff: ensure consistent diff behavior with ignore options, 2025-08-08), with the idea that the lower-level diff code could skip output when it is set. As we saw with the bugs fixed by 3ed5d8b (diff: stop output garbled message in dry run mode, 2025-10-20), it is easy to miss spots. In the end, we located all of them by checking where diff_options.file is used. That suggests another possible approach: we can replace the dry_run boolean with a NULL pointer for "file", as we know that using "file" in dry_run mode would always be an error. This turns any missed spots from producing extra output[1] into a segfault. Which is less forgiving, but that is the point: this is indicative of a programming error, and complaining loudly and immediately is good. [1] We protect ourselves against garbled output as a separate step, courtesy of 623f7af (diff: restore redirection to /dev/null for diff_from_contents, 2025-10-17). So in that sense this patch can only introduce user-visible errors (since any "bugs" were going to /dev/null before), but the idea is to catch them rather than quietly send garbage to /dev/null. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 0152831 commit b2b5ad5

File tree

2 files changed

+8
-10
lines changed

2 files changed

+8
-10
lines changed

diff.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1351,7 +1351,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
13511351
int len = eds->len;
13521352
unsigned flags = eds->flags;
13531353

1354-
if (o->dry_run)
1354+
if (!o->file)
13551355
return;
13561356

13571357
switch (s) {
@@ -3765,9 +3765,9 @@ static void builtin_diff(const char *name_a,
37653765

37663766
if (o->word_diff)
37673767
init_diff_words_data(&ecbdata, o, one, two);
3768-
if (o->dry_run) {
3768+
if (!o->file) {
37693769
/*
3770-
* Unlike the !dry_run case, we need to ignore the
3770+
* Unlike the normal output case, we need to ignore the
37713771
* return value from xdi_diff_outf() here, because
37723772
* xdi_diff_outf() takes non-zero return from its
37733773
* callback function as a sign of error and returns
@@ -4423,7 +4423,7 @@ static void run_external_diff(const struct external_diff *pgm,
44234423
{
44244424
struct child_process cmd = CHILD_PROCESS_INIT;
44254425
struct diff_queue_struct *q = &diff_queued_diff;
4426-
int quiet = !(o->output_format & DIFF_FORMAT_PATCH) || o->dry_run;
4426+
int quiet = !(o->output_format & DIFF_FORMAT_PATCH) || !o->file;
44274427
int rc;
44284428

44294429
/*
@@ -4621,7 +4621,7 @@ static void run_diff_cmd(const struct external_diff *pgm,
46214621
p->status == DIFF_STATUS_RENAMED)
46224622
o->found_changes = 1;
46234623
} else {
4624-
if (!o->dry_run)
4624+
if (o->file)
46254625
fprintf(o->file, "* Unmerged path %s\n", name);
46264626
o->found_changes = 1;
46274627
}
@@ -6199,15 +6199,15 @@ static void diff_flush_patch(struct diff_filepair *p, struct diff_options *o)
61996199
/* return 1 if any change is found; otherwise, return 0 */
62006200
static int diff_flush_patch_quietly(struct diff_filepair *p, struct diff_options *o)
62016201
{
6202-
int saved_dry_run = o->dry_run;
6202+
FILE *saved_file = o->file;
62036203
int saved_found_changes = o->found_changes;
62046204
int ret;
62056205

6206-
o->dry_run = 1;
6206+
o->file = NULL;
62076207
o->found_changes = 0;
62086208
diff_flush_patch(p, o);
62096209
ret = o->found_changes;
6210-
o->dry_run = saved_dry_run;
6210+
o->file = saved_file;
62116211
o->found_changes |= saved_found_changes;
62126212
return ret;
62136213
}

diff.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,8 +408,6 @@ struct diff_options {
408408
#define COLOR_MOVED_WS_ERROR (1<<0)
409409
unsigned color_moved_ws_handling;
410410

411-
bool dry_run;
412-
413411
struct repository *repo;
414412
struct strmap *additional_path_headers;
415413

0 commit comments

Comments
 (0)