Re: [PATCH] multi-pack-index: fix potential segfault without sub-command
On Mon, Jul 19, 2021 at 01:18:49PM -0400, Taylor Blau wrote:
Show 16 quoted lines
> Since cd57bc41bb (builtin/multi-pack-index.c: display usage on
> unrecognized command, 2021-03-30) we have used a "usage" label to avoid
> having two separate callers of usage_with_options (one when no arguments
> are given, and another for unrecognized sub-commands).
>
> But the first caller has been broken since cd57bc41bb, since it will
> happily jump to usage without arguments, and then pass argv[0] to the
> "unrecognized subcommand" error.
>
> Many compilers will save us from a segfault here, but the end result is
> ugly, since it mentions an unrecognized subcommand when we didn't even
> pass one, and (on GCC) includes "(null)" in its output.
>
> Move the "usage" label down past the error about unrecognized
> subcommands so that it is only triggered when it should be. While we're
> at it, bulk up our test coverage in this area, too.
Good find. The code change seems obviously correct.
> +test_expect_success 'usage shown without sub-command' '
> + test_expect_code 129 git multi-pack-index 2>err &&
> + ! test_i18ngrep "unrecognized subcommand" err
> +'
I think we're avoiding test_i18ngrep in new code these days.
-Peff