git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] diff: don't crash with empty argument to -G or -S

From
Elijah Newren <newren@gmail.com>
Date
Feb 17, 2025, 22:18 UTC
Message-ID
<CABPp-BF01SYAr8TNxMBNvxPKseSqLH4mVC=MrrVp8w9ejEzK1g@mail.gmail.com>
In-Reply-To
<20250217175759.1576684-1-sandals@crustytoothpaste.net>

On Mon, Feb 17, 2025 at 9:58 AM brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 12 quoted lines
>
> The pickaxe options, -G and -S, need either a regex or a string to look
> through the history for.  An empty value isn't very useful since it
> would either match everything or nothing, and what's worse, we presently
> crash with a BUG like so when the user provides one:
>
>     BUG: diffcore-pickaxe.c:241: should have needle under -G or -S
>
> Since it's not very nice of us to crash and this wouldn't do anything
> useful anyway, let's simply inform the user that they must provide a
> non-empty argument and exit with an error if they provide an empty one
> instead.
Makes sense.
Show 28 quoted lines
> Reported-by: Jared Van Bortel <cebtenzzre@gmail.com>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  diff.c                 |  4 ++++
>  t/t4209-log-pickaxe.sh | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/diff.c b/diff.c
> index 019fb893a7..c89c15d98e 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -5493,6 +5493,8 @@ static int diff_opt_pickaxe_regex(const struct option *opt,
>         BUG_ON_OPT_NEG(unset);
>         options->pickaxe = arg;
>         options->pickaxe_opts |= DIFF_PICKAXE_KIND_G;
> +       if (arg && !*arg)
> +               return error(_("-G requires a non-empty argument"));
>         return 0;
>  }
>
> @@ -5504,6 +5506,8 @@ static int diff_opt_pickaxe_string(const struct option *opt,
>         BUG_ON_OPT_NEG(unset);
>         options->pickaxe = arg;
>         options->pickaxe_opts |= DIFF_PICKAXE_KIND_S;
> +       if (arg && !*arg)
> +               return error(_("-S requires a non-empty argument"));
>         return 0;
>  }
Simple fix, as expected.
Show 23 quoted lines
> diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
> index a675ace081..0e2f80a268 100755
> --- a/t/t4209-log-pickaxe.sh
> +++ b/t/t4209-log-pickaxe.sh
> @@ -93,6 +93,22 @@ test_expect_success 'usage: --no-pickaxe-regex' '
>         test_cmp expect actual
>  '
>
> +test_expect_success 'usage: -G and -S with empty argument' '
> +       cat >expect <<-\EOF &&
> +       error: -S requires a non-empty argument
> +       EOF
> +
> +       test_expect_code 129 git log -S "" 2>actual &&
> +       test_cmp expect actual &&
> +
> +       cat >expect <<-\EOF &&
> +       error: -G requires a non-empty argument
> +       EOF
> +
> +       test_expect_code 129 git log -G "" 2>actual &&
> +       test_cmp expect actual
> +'
Looks good to me.
Previous: brian m. carlsonNext: Junio C Hamano
Message 4 of 6 in “Crash on empty pickaxe argument”
  1. Jared Van BortelFeb 17, 2025
  2. brian m. carlsonFeb 17, 2025
  3. diff: don't crash with empty argument to -G or -Sbrian m. carlson, Feb 17, 2025
  4. Elijah NewrenFeb 17, 2025
  5. Junio C HamanoFeb 18, 2025
  6. brian m. carlsonFeb 18, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.