git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 3/3] parse-options: check empty value in OPT_INTEGER and OPT_ABBREV

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
May 29, 2019, 09:11 UTC
Message-ID
<20190529091116.21898-4-pclouds@gmail.com>
In-Reply-To
<20190529091116.21898-1-pclouds@gmail.com>

When parsing the argument for OPT_INTEGER and OPT_ABBREV, we check if we can parse the entire argument to a number with "if (*s)". There is one missing check: if "arg" is empty to begin with, we fail to notice.

This could happen with long option by writing like
  git diff --inter-hunk-context= blah blah

Before 16ed6c97cc (diff-parseopt: convert --inter-hunk-context, 2019-03-24), --inter-hunk-context is handled by a custom parser opt_arg() and does detect this correctly.

This restores the bahvior for --inter-hunk-context and make sure all other integer options are handled the same (sane) way. For OPT_ABBREV this is new behavior. But it makes it consistent with the rest.

PS. OPT_MAGNITUDE has similar code but git_parse_ulong() does detect empty "arg". So it's good to go.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 parse-options-cb.c | 3 +++
 parse-options.c    | 3 +++
 2 files changed, 6 insertions(+)
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 4b95d04a37..a3de795c58 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -16,6 +16,9 @@ int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset)
 	if (!arg) {
 		v = unset ? 0 : DEFAULT_ABBREV;
 	} else {
+		if (!*arg)
+			return error(_("option `%s' expects a numerical value"),
+				     opt->long_name);
 		v = strtol(arg, (char **)&arg, 10);
 		if (*arg)
 			return error(_("option `%s' expects a numerical value"),
diff --git a/parse-options.c b/parse-options.c
index 987e27cb91..87b26a1d92 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -195,6 +195,9 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
 		}
 		if (get_arg(p, opt, flags, &arg))
 			return -1;
+		if (!*arg)
+			return error(_("%s expects a numerical value"),
+				     optname(opt, flags));
 		*(int *)opt->value = strtol(arg, (char **)&s, 10);
 		if (*s)
 			return error(_("%s expects a numerical value"),
-- 
2.22.0.rc0.322.g2b0371e29a
Previous: Nguyễn Thái Ngọc DuyNext: Junio C Hamano
Message 17 of 18 in “fix diff-parseopt regressions”
  1. 0/3 fix diff-parseopt regressionsNguyễn Thái Ngọc Duy, May 24, 2019
  2. 1/3 diff-parseopt: correct variable types that are used by parseoptNguyễn Thái Ngọc Duy, May 24, 2019
  3. Junio C HamanoMay 28, 2019
  4. 2/3 diff-parseopt: restore -U (no argument) behaviorNguyễn Thái Ngọc Duy, May 24, 2019
  5. 3/3 parse-options: check empty value in OPT_INTEGER and OPT_ABBREVNguyễn Thái Ngọc Duy, May 24, 2019
  6. 4/3 parse-options: make compiler check value type mismatchNguyễn Thái Ngọc Duy, May 24, 2019
  7. Todd ZullingerMay 24, 2019
  8. Todd ZullingerMay 24, 2019
  9. Duy NguyenMay 25, 2019
  10. Todd ZullingerMay 25, 2019
  11. 0/3 fix diff-parseopt regressionsNguyễn Thái Ngọc Duy, May 29, 2019
  12. 1/3 diff-parseopt: correct variable types that are used by parseoptNguyễn Thái Ngọc Duy, May 29, 2019
  13. Eric SunshineMay 29, 2019
  14. Todd ZullingerMay 29, 2019
  15. Junio C HamanoMay 29, 2019
  16. 2/3 diff-parseopt: restore -U (no argument) behaviorNguyễn Thái Ngọc Duy, May 29, 2019
  17. 3/3 parse-options: check empty value in OPT_INTEGER and OPT_ABBREVNguyễn Thái Ngọc Duy, May 29, 2019
  18. Junio C HamanoMay 29, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.