diff: replace diff_options.dry_run flag with NULL file

We introduced a dry_run flag to diff_options in b55e6d36eb (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 3ed5d8bd73 (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 623f7af284 (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>

diff --git a/diff.c b/diff.c
index d83d8987028cbd2105d9bb51eba5b69bd25cd380..a8d50fb1fcd640033c2426ee067aea6437d327c0 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -1351,7 +1351,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
        int len = eds->len;
        unsigned flags = eds->flags;
 
-       if (o->dry_run)
+       if (!o->file)
                return;
 
        switch (s) {
@@ -3765,9 +3765,9 @@ static void builtin_diff(const char *name_a,
 
                if (o->word_diff)
                        init_diff_words_data(&ecbdata, o, one, two);
-               if (o->dry_run) {
+               if (!o->file) {
                        /*
-                        * Unlike the !dry_run case, we need to ignore the
+                        * Unlike the normal output case, we need to ignore the
                         * return value from xdi_diff_outf() here, because
                         * xdi_diff_outf() takes non-zero return from its
                         * callback function as a sign of error and returns
@@ -4423,7 +4423,7 @@ static void run_external_diff(const struct external_diff *pgm,
 {
        struct child_process cmd = CHILD_PROCESS_INIT;
        struct diff_queue_struct *q = &diff_queued_diff;
-       int quiet = !(o->output_format & DIFF_FORMAT_PATCH) || o->dry_run;
+       int quiet = !(o->output_format & DIFF_FORMAT_PATCH) || !o->file;
        int rc;
 
        /*
@@ -4621,7 +4621,7 @@ static void run_diff_cmd(const struct external_diff *pgm,
                    p->status == DIFF_STATUS_RENAMED)
                        o->found_changes = 1;
        } else {
-               if (!o->dry_run)
+               if (o->file)
                        fprintf(o->file, "* Unmerged path %s\n", name);
                o->found_changes = 1;
        }
@@ -6199,15 +6199,15 @@ static void diff_flush_patch(struct diff_filepair *p, struct diff_options *o)
 /* return 1 if any change is found; otherwise, return 0 */
 static int diff_flush_patch_quietly(struct diff_filepair *p, struct diff_options *o)
 {
-       int saved_dry_run = o->dry_run;
+       FILE *saved_file = o->file;
        int saved_found_changes = o->found_changes;
        int ret;
 
-       o->dry_run = 1;
+       o->file = NULL;
        o->found_changes = 0;
        diff_flush_patch(p, o);
        ret = o->found_changes;
-       o->dry_run = saved_dry_run;
+       o->file = saved_file;
        o->found_changes |= saved_found_changes;
        return ret;
 }

diff --git a/diff.h b/diff.h
index 2fa256c3ef00798b7d15c3b0af5d0d87e6a794d8..31eedd5c0c39d336205b05ed0125c5ba316169d5 100644 (file)
--- a/diff.h
+++ b/diff.h
@@ -408,8 +408,6 @@ struct diff_options {
        #define COLOR_MOVED_WS_ERROR (1<<0)
        unsigned color_moved_ws_handling;
 
-       bool dry_run;
-
        struct repository *repo;
        struct strmap *additional_path_headers;