From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Lidong Yan <yldhome2d2@gmail.com>
Subject: Re: [PATCH 3/4] diff: replace diff_options.dry_run flag with NULL file
Date: Fri, 24 Oct 2025 10:22:52 -0700 [thread overview]
Message-ID: <xmqqcy6cp637.fsf@gitster.g> (raw)
In-Reply-To: <20251024170853.GC2345184@coredump.intra.peff.net> (Jeff King's message of "Fri, 24 Oct 2025 13:08:53 -0400")
Jeff King <peff@peff.net> writes:
> 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.
Makes sense.
next prev parent reply other threads:[~2025-10-24 17:22 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-24 17:05 [PATCH 0/4] diff dry-run cleanups Jeff King
2025-10-24 17:06 ` [PATCH 1/4] diff: send external diff output to diff_options.file Jeff King
2025-10-24 17:07 ` [PATCH 2/4] diff: drop save/restore of color_moved in dry-run mode Jeff King
2025-10-24 17:08 ` [PATCH 3/4] diff: replace diff_options.dry_run flag with NULL file Jeff King
2025-10-24 17:22 ` Junio C Hamano [this message]
2025-10-24 17:09 ` [PATCH 4/4] diff: drop dry-run redirection to /dev/null Jeff King
2025-10-24 17:25 ` [PATCH 5/4] diff: simplify run_external_diff() quiet logic Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqcy6cp637.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=yldhome2d2@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.