Re: [PATCHv2 3/5] revision.c: stricter parsing of '--early-output'
From: Junio C Hamano <hidden>
Date: 2017-06-10 04:52:51
SZEDER Gábor [off-list ref] writes:
quoted hunk
The parsing of '--early-output' with or without its optional integer argument allowed bogus options like '--early-output-foobarbaz' to slip through and be ignored. Fix it by parsing '--early-output' in the same way as other options with an optional argument are parsed. Furthermore, use strtoul_ui() to parse the optional integer argument and to refuse negative numbers. While at it, use skip_prefix() instead of starts_with() and magic numbers. Signed-off-by: SZEDER Gábor <redacted> --- revision.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-)diff --git a/revision.c b/revision.c index 2f37e1e3a..68531ff5d 100644 --- a/revision.c +++ b/revision.c@@ -1785,16 +1785,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg } else if (!strcmp(arg, "--author-date-order")) { revs->sort_order = REV_SORT_BY_AUTHOR_DATE; revs->topo_order = 1; - } else if (starts_with(arg, "--early-output")) { - int count = 100; - switch (arg[14]) { - case '=': - count = atoi(arg+15); - /* Fallthrough */ - case 0: - revs->topo_order = 1; - revs->early_output = count; - }
This shows how correct patch 1/5 is, huh? It's kind of surprising that nobody complained about the breakage since May 2011 which in turn makes me suspect that early-output might not be missed if we dropped it someday, but that is a separate issue. This series makes it work as the feature was envisioned to. Thanks.
quoted hunk
+ } else if (!strcmp(arg, "--early-output")) { + revs->early_output = 100; + revs->topo_order = 1; + } else if (skip_prefix(arg, "--early-output=", &optarg)) { + if (strtoul_ui(optarg, 10, &revs->early_output) < 0) + die("'%s': not a non-negative integer", optarg); + revs->topo_order = 1; } else if (!strcmp(arg, "--parents")) { revs->rewrite_parents = 1; revs->print_parents = 1;