diff mbox series

[v3,4/7] parse-options: introduce precision handling for `OPTION_MAGNITUDE`

Message ID 20250416-b4-pks-parse-options-integers-v3-4-d390746bea79@pks.im (mailing list archive)
State Superseded
Headers show
Series parse-options: harden handling of integer values | expand

Commit Message

Patrick Steinhardt April 16, 2025, 10:02 a.m. UTC
This commit is the equivalent to the preceding commit, but instead of
introducing precision handling for `OPTION_INTEGER` we introduce it for
`OPTION_MAGNITUDE`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 parse-options.c               | 45 ++++++++++++++++++++++++++++++++-----------
 parse-options.h               |  1 +
 t/helper/test-parse-options.c |  3 +++
 t/t0040-parse-options.sh      | 18 ++++++++++++++++-
 4 files changed, 55 insertions(+), 12 deletions(-)
diff mbox series

Patch

diff --git a/parse-options.c b/parse-options.c
index 2cb9bd3b5b9..259716efb17 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -202,7 +202,7 @@  static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 
 		if (value < lower_bound || value > upper_bound)
 			return error(_("value %s for %s not in range [%"PRIdMAX",%"PRIdMAX"]"),
-				     arg, optname(opt, flags), lower_bound, upper_bound);
+				     arg, optname(opt, flags), (intmax_t)lower_bound, (intmax_t)upper_bound);
 
 		switch (opt->precision) {
 		case 1:
@@ -223,21 +223,44 @@  static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 		}
 	}
 	case OPTION_MAGNITUDE:
+	{
+		uintmax_t upper_bound = UINTMAX_MAX >> (bitsizeof(uintmax_t) - CHAR_BIT * opt->precision);
+		unsigned long value;
+
 		if (unset) {
-			*(unsigned long *)opt->value = 0;
-			return 0;
-		}
-		if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
-			*(unsigned long *)opt->value = opt->defval;
-			return 0;
-		}
-		if (get_arg(p, opt, flags, &arg))
+			value = 0;
+		} else if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
+			value = opt->defval;
+		} else if (get_arg(p, opt, flags, &arg)) {
 			return -1;
-		if (!git_parse_ulong(arg, opt->value))
+		} else if (!git_parse_ulong(arg, &value)) {
 			return error(_("%s expects a non-negative integer value"
 				       " with an optional k/m/g suffix"),
 				     optname(opt, flags));
-		return 0;
+		}
+
+		if (value > upper_bound)
+			return error(_("value %s for %s not in range [%"PRIuMAX",%"PRIuMAX"]"),
+				     arg, optname(opt, flags), (uintmax_t)0, (uintmax_t)upper_bound);
+
+		switch (opt->precision) {
+		case 1:
+			*(uint8_t *)opt->value = value;
+			return 0;
+		case 2:
+			*(uint16_t *)opt->value = value;
+			return 0;
+		case 4:
+			*(uint32_t *)opt->value = value;
+			return 0;
+		case 8:
+			*(uint64_t *)opt->value = value;
+			return 0;
+		default:
+			BUG("invalid precision for option %s",
+			    optname(opt, flags));
+		}
+	}
 
 	default:
 		BUG("opt->type %d should not happen", opt->type);
diff --git a/parse-options.h b/parse-options.h
index 8db96402c4d..55c42faa29f 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -281,6 +281,7 @@  struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.argh = N_("n"), \
 	.help = (h), \
 	.flags = PARSE_OPT_NONEG, \
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index b1275dfade4..46deb4317ef 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -120,6 +120,7 @@  int cmd__parse_options(int argc, const char **argv)
 	};
 	struct string_list expect = STRING_LIST_INIT_NODUP;
 	struct string_list list = STRING_LIST_INIT_NODUP;
+	uint16_t m16 = 0;
 	int16_t i16 = 0;
 
 	struct option options[] = {
@@ -143,6 +144,7 @@  int cmd__parse_options(int argc, const char **argv)
 		OPT_INTEGER(0, "i16", &i16, "get a 16 bit integer"),
 		OPT_INTEGER('j', NULL, &integer, "get a integer, too"),
 		OPT_MAGNITUDE('m', "magnitude", &magnitude, "get a magnitude"),
+		OPT_MAGNITUDE(0, "m16", &m16, "get a 16 bit magnitude"),
 		OPT_SET_INT(0, "set23", &integer, "set integer to 23", 23),
 		OPT_CMDMODE(0, "mode1", &integer, "set integer to 1 (cmdmode option)", 1),
 		OPT_CMDMODE(0, "mode2", &integer, "set integer to 2 (cmdmode option)", 2),
@@ -214,6 +216,7 @@  int cmd__parse_options(int argc, const char **argv)
 	show(&expect, &ret, "integer: %d", integer);
 	show(&expect, &ret, "i16: %"PRIdMAX, (intmax_t) i16);
 	show(&expect, &ret, "magnitude: %lu", magnitude);
+	show(&expect, &ret, "m16: %"PRIuMAX, (uintmax_t) m16);
 	show(&expect, &ret, "timestamp: %"PRItime, timestamp);
 	show(&expect, &ret, "string: %s", string ? string : "(not set)");
 	show(&expect, &ret, "abbrev: %d", abbrev);
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 95951436cda..8daaf568485 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -25,6 +25,7 @@  usage: test-tool parse-options <options>
     --[no-]i16 <n>        get a 16 bit integer
     -j <n>                get a integer, too
     -m, --magnitude <n>   get a magnitude
+    --m16 <n>             get a 16 bit magnitude
     --[no-]set23          set integer to 23
     --mode1               set integer to 1 (cmdmode option)
     --mode2               set integer to 2 (cmdmode option)
@@ -139,6 +140,7 @@  boolean: 2
 integer: 1729
 i16: 0
 magnitude: 16384
+m16: 0
 timestamp: 0
 string: 123
 abbrev: 7
@@ -160,6 +162,7 @@  boolean: 2
 integer: 1729
 i16: 9000
 magnitude: 16384
+m16: 32768
 timestamp: 0
 string: 321
 abbrev: 10
@@ -171,7 +174,7 @@  EOF
 
 test_expect_success 'long options' '
 	test-tool parse-options --boolean --integer 1729 --i16 9000 --magnitude 16k \
-		--boolean --string2=321 --verbose --verbose --no-dry-run \
+		--m16 32k --boolean --string2=321 --verbose --verbose --no-dry-run \
 		--abbrev=10 --file fi.le --obsolete \
 		>output 2>output.err &&
 	test_must_be_empty output.err &&
@@ -184,6 +187,7 @@  test_expect_success 'abbreviate to something longer than SHA1 length' '
 	integer: 0
 	i16: 0
 	magnitude: 0
+	m16: 0
 	timestamp: 0
 	string: (not set)
 	abbrev: 100
@@ -259,6 +263,7 @@  boolean: 1
 integer: 13
 i16: 0
 magnitude: 0
+m16: 0
 timestamp: 0
 string: 123
 abbrev: 7
@@ -283,6 +288,7 @@  boolean: 0
 integer: 2
 i16: 0
 magnitude: 0
+m16: 0
 timestamp: 0
 string: (not set)
 abbrev: 7
@@ -351,6 +357,7 @@  boolean: 5
 integer: 4
 i16: 0
 magnitude: 0
+m16: 0
 timestamp: 0
 string: (not set)
 abbrev: 7
@@ -377,6 +384,7 @@  boolean: 1
 integer: 23
 i16: 0
 magnitude: 0
+m16: 0
 timestamp: 0
 string: (not set)
 abbrev: 7
@@ -457,6 +465,7 @@  boolean: 0
 integer: 0
 i16: 0
 magnitude: 0
+m16: 0
 timestamp: 0
 string: (not set)
 abbrev: 7
@@ -810,4 +819,11 @@  test_expect_success 'i16 limits range' '
 	test_grep "value -32769 for option .i16. not in range \[-32768,32767\]" err
 '
 
+test_expect_success 'm16 limits range' '
+	test-tool parse-options --m16 65535 >out &&
+	test_grep "m16: 65535" out &&
+	test_must_fail test-tool parse-options --m16 65536 2>err &&
+	test_grep "value 65536 for option .m16. not in range \[0,65535\]" err
+'
+
 test_done