On Tue, Jul 20, 2021 at 07:47:39PM +0200, SZEDER Gábor wrote: > On Tue, Jul 20, 2021 at 01:39:45PM +0200, Ævar Arnfjörð Bjarmason wrote: > > Change the parse_options() invocation in the commit-graph code to make > > sense. We're calling it twice, once for common options parsing, and > > then for the sub-commands. > > > > But we never checked if we had something leftover in argc in "write" > > or "verify", as a result we'd silently accept garbage in these > > subcommands. Let's not do that. > > > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> > > --- > > builtin/commit-graph.c | 10 ++++++++-- > > t/t5318-commit-graph.sh | 5 +++++ > > 2 files changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c > > index bf34aa43f22..88cbcb5a64f 100644 > > --- a/builtin/commit-graph.c > > +++ b/builtin/commit-graph.c > > @@ -104,7 +104,10 @@ static int graph_verify(int argc, const char **argv) > > opts.progress = isatty(2); > > argc = parse_options(argc, argv, NULL, > > options, > > - builtin_commit_graph_verify_usage, 0); > > + builtin_commit_graph_verify_usage, > > + PARSE_OPT_KEEP_UNKNOWN); > > + if (argc) > > + usage_with_options(builtin_commit_graph_verify_usage, options); > > Checking 'argc' alone is sufficient to catch unsupported parameters. > > Using PARSE_OPT_KEEP_UNKNOWN is not only unnecessary, but arguably > wrong here And it's wrong in 'multi-pack-index' and 'env--helper' as well. > because 'git commit-graph write --foo' won't print "error: > unknown option `foo'", and we don't want to pass the remaining > unrecognized options to a different command, like e.g. 'git difftool', > or another parse_options(), like e.g. 'git archive'.
