On Mon, Feb 15 2021, Taylor Blau wrote:
> trace2_cmd_mode(argv[0]);
Not a new issue, but curious that in the commit-graph.c code we'll first
validate, but here write garbage into the trace2_cmd_mode() before
potentially dying.
>
> if (!strcmp(argv[0], "repack"))
> - return midx_repack(the_repository, opts.object_dir,
> - (size_t)opts.batch_size, opts.flags);
> - if (opts.batch_size)
> - die(_("--batch-size option is only for 'repack' subcommand"));
> -
> - if (!strcmp(argv[0], "write"))
> - return write_midx_file(opts.object_dir, opts.flags);
> - if (!strcmp(argv[0], "verify"))
> - return verify_midx_file(the_repository, opts.object_dir, opts.flags);
> - if (!strcmp(argv[0], "expire"))
> - return expire_midx_packs(the_repository, opts.object_dir, opts.flags);
> -
> - die(_("unrecognized subcommand: %s"), argv[0]);
> + return cmd_multi_pack_index_repack(argc, argv);
> + else if (!strcmp(argv[0], "write"))
> + return cmd_multi_pack_index_write(argc, argv);
> + else if (!strcmp(argv[0], "verify"))
> + return cmd_multi_pack_index_verify(argc, argv);
> + else if (!strcmp(argv[0], "expire"))
> + return cmd_multi_pack_index_expire(argc, argv);
> + else
> + die(_("unrecognized subcommand: %s"), argv[0]);
I realize this is the existing behavior, but let's just make this die()
be the usage_with_options() we emit above in this case?
So maybe this on top?
diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index caf0248a98..6f9223d538 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -65,6 +65,8 @@ static int cmd_multi_pack_index_write(int argc, const char **argv)
{
struct option *options = common_opts;
+ trace2_cmd_mode(argv[0]);
+
argc = parse_options(argc, argv, NULL,
options, builtin_multi_pack_index_write_usage,
PARSE_OPT_KEEP_UNKNOWN);
@@ -79,6 +81,8 @@ static int cmd_multi_pack_index_verify(int argc, const char **argv)
{
struct option *options = common_opts;
+ trace2_cmd_mode(argv[0]);
+
argc = parse_options(argc, argv, NULL,
options, builtin_multi_pack_index_verify_usage,
PARSE_OPT_KEEP_UNKNOWN);
@@ -93,6 +97,8 @@ static int cmd_multi_pack_index_expire(int argc, const char **argv)
{
struct option *options = common_opts;
+ trace2_cmd_mode(argv[0]);
+
argc = parse_options(argc, argv, NULL,
options, builtin_multi_pack_index_expire_usage,
PARSE_OPT_KEEP_UNKNOWN);
@@ -112,6 +118,8 @@ static int cmd_multi_pack_index_repack(int argc, const char **argv)
OPT_END(),
};
+ trace2_cmd_mode(argv[0]);
+
options = parse_options_dup(builtin_multi_pack_index_repack_options);
options = add_common_options(options);
@@ -144,20 +152,15 @@ int cmd_multi_pack_index(int argc, const char **argv,
if (!opts.object_dir)
opts.object_dir = get_object_directory();
- if (argc == 0)
- usage_with_options(builtin_multi_pack_index_usage,
- builtin_multi_pack_index_options);
-
- trace2_cmd_mode(argv[0]);
-
- if (!strcmp(argv[0], "repack"))
+ if (argc && !strcmp(argv[0], "repack"))
return cmd_multi_pack_index_repack(argc, argv);
- else if (!strcmp(argv[0], "write"))
+ else if (argc && !strcmp(argv[0], "write"))
return cmd_multi_pack_index_write(argc, argv);
- else if (!strcmp(argv[0], "verify"))
+ else if (argc && !strcmp(argv[0], "verify"))
return cmd_multi_pack_index_verify(argc, argv);
- else if (!strcmp(argv[0], "expire"))
+ else if (argc && !strcmp(argv[0], "expire"))
return cmd_multi_pack_index_expire(argc, argv);
else
- die(_("unrecognized subcommand: %s"), argv[0]);
+ usage_with_options(builtin_multi_pack_index_usage,
+ builtin_multi_pack_index_options);
}
