Re: [PATCH v2 4/4] builtin/multi-pack-index.c: split sub-commands
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Feb 15, 2021, 21:54 UTC
- Message-ID
- <877dn9atew.fsf@evledraar.gmail.com>
- In-Reply-To
- <16f33e41388ed431f70e09ef68717bd30fbee67f.1613422804.git.me@ttaylorr.com>
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.
Show 24 quoted lines
>
> 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); }