On Tue, Jul 20, 2021 at 07:55:30PM +0200, SZEDER Gábor wrote: > 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. Thanks for spotting. Obviously this one is new, but the one in the midx builtin is my fault; I'm not sure why it's there, because it's clearly wrong. So we should definitely fix this instance via a reroll of this series, but that still leaves the others up for grabs. Ævar, are those (the ones in the 'multi-pack-index' and 'env--helper' builtins) something that you want to clean up while you're working in this area, or would you rather that I take care of it? I don't mind either way, just want to make sure that we don't duplicate effort. Thanks, Taylor
