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

[PATCH] multi-pack-index: fix potential segfault without sub-command

From
Taylor Blau <me@ttaylorr.com>
Date
Jul 19, 2021, 17:18 UTC
Message-ID
<8c0bb3e0dc121bd68f7014000fbb60b28750a0fe.1626715096.git.me@ttaylorr.com>

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.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Noticed this while I was reading code in a similar area while reviewing
one of Ævar's series.
 builtin/multi-pack-index.c  | 2 +-
 t/t5319-multi-pack-index.sh | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index 5d3ea445fd..8ff0dee2ec 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -176,8 +176,8 @@ int cmd_multi_pack_index(int argc, const char **argv,
 	else if (!strcmp(argv[0], "expire"))
 		return cmd_multi_pack_index_expire(argc, argv);
 	else {
-usage:
 		error(_("unrecognized subcommand: %s"), argv[0]);
+usage:
 		usage_with_options(builtin_multi_pack_index_usage,
 				   builtin_multi_pack_index_options);
 	}
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index 5641d158df..dab7123b3a 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -824,4 +824,9 @@ test_expect_success 'load reverse index when missing .idx, .pack' '
 	)
 '

+test_expect_success 'usage shown without sub-command' '
+	test_expect_code 129 git multi-pack-index 2>err &&
+	! test_i18ngrep "unrecognized subcommand" err
+'
+
 test_done
--
2.31.1.163.ga65ce7f831
Next: Jeff King
Message 1 of 4 in “multi-pack-index: fix potential segfault without sub-command”
  1. multi-pack-index: fix potential segfault without sub-commandTaylor Blau, Jul 19, 2021
  2. Jeff KingJul 21, 2021
  3. multi-pack-index: fix potential segfault without sub-commandTaylor Blau, Jul 21, 2021
  4. Jeff KingJul 23, 2021

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.