All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org, avarab@gmail.com
Subject: Re: [PATCH] multi-pack-index: fix potential segfault without sub-command
Date: Wed, 21 Jul 2021 04:10:59 -0400	[thread overview]
Message-ID: <YPfWkzRtQKthOgZx@coredump.intra.peff.net> (raw)
In-Reply-To: <8c0bb3e0dc121bd68f7014000fbb60b28750a0fe.1626715096.git.me@ttaylorr.com>

On Mon, Jul 19, 2021 at 01:18:49PM -0400, Taylor Blau wrote:

> 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

  reply	other threads:[~2021-07-21  8:13 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-19 17:18 [PATCH] multi-pack-index: fix potential segfault without sub-command Taylor Blau
2021-07-21  8:10 ` Jeff King [this message]
2021-07-21 16:52   ` [PATCH v2] " Taylor Blau
2021-07-23  7:34     ` 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=YPfWkzRtQKthOgZx@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.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.