Merge branch 'ps/parse-options-integers'

Update parse-options API to catch mistakes to pass address of an
integral variable of a wrong type/size.

* ps/parse-options-integers:
  parse-options: detect mismatches in integer signedness
  parse-options: introduce precision handling for `OPTION_UNSIGNED`
  parse-options: introduce precision handling for `OPTION_INTEGER`
  parse-options: rename `OPT_MAGNITUDE()` to `OPT_UNSIGNED()`
  parse-options: support unit factors in `OPT_INTEGER()`
  global: use designated initializers for options
  parse: fix off-by-one for minimum signed values

2 files changed, 105 insertions, 38 deletions

diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index bfe45ec68b..f2663dd0c0 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -6,7 +6,7 @@
 
 static int boolean = 0;
 static int integer = 0;
-static unsigned long magnitude = 0;
+static unsigned long unsigned_integer = 0;
 static timestamp_t timestamp;
 static int abbrev = 7;
 static int verbose = -1; /* unspecified */
@@ -120,20 +120,31 @@ 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 u16 = 0;
+	int16_t i16 = 0;
 
 	struct option options[] = {
 		OPT_BOOL(0, "yes", &boolean, "get a boolean"),
 		OPT_BOOL('D', "no-doubt", &boolean, "begins with 'no-'"),
-		{ OPTION_SET_INT, 'B', "no-fear", &boolean, NULL,
-		  "be brave", PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
+		{
+			.type = OPTION_SET_INT,
+			.short_name = 'B',
+			.long_name = "no-fear",
+			.value = &boolean,
+			.help = "be brave",
+			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
+			.defval = 1,
+		},
 		OPT_COUNTUP('b', "boolean", &boolean, "increment by one"),
 		OPT_BIT('4', "or4", &boolean,
 			"bitwise-or boolean with ...0100", 4),
 		OPT_NEGBIT(0, "neg-or4", &boolean, "same as --no-or4", 4),
 		OPT_GROUP(""),
 		OPT_INTEGER('i', "integer", &integer, "get a integer"),
+		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_UNSIGNED('u', "unsigned", &unsigned_integer, "get an unsigned integer"),
+		OPT_UNSIGNED(0, "u16", &u16, "get a 16 bit unsigned integer"),
 		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),
@@ -155,12 +166,27 @@ int cmd__parse_options(int argc, const char **argv)
 		OPT_GROUP("Magic arguments"),
 		OPT_NUMBER_CALLBACK(&integer, "set integer to NUM",
 			number_callback),
-		{ OPTION_COUNTUP, '+', NULL, &boolean, NULL, "same as -b",
-		  PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH },
-		{ OPTION_COUNTUP, 0, "ambiguous", &ambiguous, NULL,
-		  "positive ambiguity", PARSE_OPT_NOARG | PARSE_OPT_NONEG },
-		{ OPTION_COUNTUP, 0, "no-ambiguous", &ambiguous, NULL,
-		  "negative ambiguity", PARSE_OPT_NOARG | PARSE_OPT_NONEG },
+		{
+			.type = OPTION_COUNTUP,
+			.short_name = '+',
+			.value = &boolean,
+			.help = "same as -b",
+			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH,
+		},
+		{
+			.type = OPTION_COUNTUP,
+			.long_name = "ambiguous",
+			.value = &ambiguous,
+			.help = "positive ambiguity",
+			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
+		},
+		{
+			.type = OPTION_COUNTUP,
+			.long_name = "no-ambiguous",
+			.value = &ambiguous,
+			.help = "negative ambiguity",
+			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
+		},
 		OPT_GROUP("Standard options"),
 		OPT__ABBREV(&abbrev),
 		OPT__VERBOSE(&verbose, "be verbose"),
@@ -188,7 +214,9 @@ int cmd__parse_options(int argc, const char **argv)
 	}
 	show(&expect, &ret, "boolean: %d", boolean);
 	show(&expect, &ret, "integer: %d", integer);
-	show(&expect, &ret, "magnitude: %lu", magnitude);
+	show(&expect, &ret, "i16: %"PRIdMAX, (intmax_t) i16);
+	show(&expect, &ret, "unsigned: %lu", unsigned_integer);
+	show(&expect, &ret, "u16: %"PRIuMAX, (uintmax_t) u16);
 	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 2fe3522305..ca55ea8228 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -22,8 +22,10 @@ usage: test-tool parse-options <options>
 
     -i, --[no-]integer <n>
                           get a integer
+    --[no-]i16 <n>        get a 16 bit integer
     -j <n>                get a integer, too
-    -m, --magnitude <n>   get a magnitude
+    -u, --unsigned <n>    get an unsigned integer
+    --u16 <n>             get a 16 bit unsigned integer
     --[no-]set23          set integer to 23
     --mode1               set integer to 1 (cmdmode option)
     --mode2               set integer to 2 (cmdmode option)
@@ -111,32 +113,36 @@ test_expect_success 'OPT_BOOL() no negation #2' 'check_unknown_i18n --no-no-fear
 
 test_expect_success 'OPT_BOOL() positivation' 'check boolean: 0 -D --doubt'
 
-test_expect_success 'OPT_INT() negative' 'check integer: -2345 -i -2345'
+test_expect_success 'OPT_INTEGER() negative' 'check integer: -2345 -i -2345'
+test_expect_success 'OPT_INTEGER() kilo' 'check integer: 239616 -i 234k'
+test_expect_success 'OPT_INTEGER() negative kilo' 'check integer: -239616 -i -234k'
 
-test_expect_success 'OPT_MAGNITUDE() simple' '
-	check magnitude: 2345678 -m 2345678
+test_expect_success 'OPT_UNSIGNED() simple' '
+	check unsigned: 2345678 -u 2345678
 '
 
-test_expect_success 'OPT_MAGNITUDE() kilo' '
-	check magnitude: 239616 -m 234k
+test_expect_success 'OPT_UNSIGNED() kilo' '
+	check unsigned: 239616 -u 234k
 '
 
-test_expect_success 'OPT_MAGNITUDE() mega' '
-	check magnitude: 104857600 -m 100m
+test_expect_success 'OPT_UNSIGNED() mega' '
+	check unsigned: 104857600 -u 100m
 '
 
-test_expect_success 'OPT_MAGNITUDE() giga' '
-	check magnitude: 1073741824 -m 1g
+test_expect_success 'OPT_UNSIGNED() giga' '
+	check unsigned: 1073741824 -u 1g
 '
 
-test_expect_success 'OPT_MAGNITUDE() 3giga' '
-	check magnitude: 3221225472 -m 3g
+test_expect_success 'OPT_UNSIGNED() 3giga' '
+	check unsigned: 3221225472 -u 3g
 '
 
 cat >expect <<\EOF
 boolean: 2
 integer: 1729
-magnitude: 16384
+i16: 0
+unsigned: 16384
+u16: 0
 timestamp: 0
 string: 123
 abbrev: 7
@@ -147,7 +153,7 @@ file: prefix/my.file
 EOF
 
 test_expect_success 'short options' '
-	test-tool parse-options -s123 -b -i 1729 -m 16k -b -vv -n -F my.file \
+	test-tool parse-options -s123 -b -i 1729 -u 16k -b -vv -n -F my.file \
 	>output 2>output.err &&
 	test_cmp expect output &&
 	test_must_be_empty output.err
@@ -156,7 +162,9 @@ test_expect_success 'short options' '
 cat >expect <<\EOF
 boolean: 2
 integer: 1729
-magnitude: 16384
+i16: 9000
+unsigned: 16384
+u16: 32768
 timestamp: 0
 string: 321
 abbrev: 10
@@ -167,8 +175,8 @@ file: prefix/fi.le
 EOF
 
 test_expect_success 'long options' '
-	test-tool parse-options --boolean --integer 1729 --magnitude 16k \
-		--boolean --string2=321 --verbose --verbose --no-dry-run \
+	test-tool parse-options --boolean --integer 1729 --i16 9000 --unsigned 16k \
+		--u16 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 &&
@@ -179,7 +187,9 @@ test_expect_success 'abbreviate to something longer than SHA1 length' '
 	cat >expect <<-EOF &&
 	boolean: 0
 	integer: 0
-	magnitude: 0
+	i16: 0
+	unsigned: 0
+	u16: 0
 	timestamp: 0
 	string: (not set)
 	abbrev: 100
@@ -253,7 +263,9 @@ test_expect_success 'superfluous value provided: cmdmode' '
 cat >expect <<\EOF
 boolean: 1
 integer: 13
-magnitude: 0
+i16: 0
+unsigned: 0
+u16: 0
 timestamp: 0
 string: 123
 abbrev: 7
@@ -276,7 +288,9 @@ test_expect_success 'intermingled arguments' '
 cat >expect <<\EOF
 boolean: 0
 integer: 2
-magnitude: 0
+i16: 0
+unsigned: 0
+u16: 0
 timestamp: 0
 string: (not set)
 abbrev: 7
@@ -343,7 +357,9 @@ cat >expect <<\EOF
 Callback: "four", 0
 boolean: 5
 integer: 4
-magnitude: 0
+i16: 0
+unsigned: 0
+u16: 0
 timestamp: 0
 string: (not set)
 abbrev: 7
@@ -368,7 +384,9 @@ test_expect_success 'OPT_CALLBACK() and callback errors work' '
 cat >expect <<\EOF
 boolean: 1
 integer: 23
-magnitude: 0
+i16: 0
+unsigned: 0
+u16: 0
 timestamp: 0
 string: (not set)
 abbrev: 7
@@ -447,7 +465,9 @@ test_expect_success 'OPT_NUMBER_CALLBACK() works' '
 cat >expect <<\EOF
 boolean: 0
 integer: 0
-magnitude: 0
+i16: 0
+unsigned: 0
+u16: 0
 timestamp: 0
 string: (not set)
 abbrev: 7
@@ -771,16 +791,35 @@ test_expect_success 'subcommands are incompatible with KEEP_DASHDASH unless in c
 	grep ^BUG err
 '
 
-test_expect_success 'negative magnitude' '
-	test_must_fail test-tool parse-options --magnitude -1 >out 2>err &&
+test_expect_success 'negative unsigned' '
+	test_must_fail test-tool parse-options --unsigned -1 >out 2>err &&
 	grep "non-negative integer" err &&
 	test_must_be_empty out
 '
 
-test_expect_success 'magnitude with units but no numbers' '
-	test_must_fail test-tool parse-options --magnitude m >out 2>err &&
+test_expect_success 'unsigned with units but no numbers' '
+	test_must_fail test-tool parse-options --unsigned m >out 2>err &&
 	grep "non-negative integer" err &&
 	test_must_be_empty out
 '
 
+test_expect_success 'i16 limits range' '
+	test-tool parse-options --i16 32767 >out &&
+	test_grep "i16: 32767" out &&
+	test_must_fail test-tool parse-options --i16 32768 2>err &&
+	test_grep "value 32768 for option .i16. not in range \[-32768,32767\]" err &&
+
+	test-tool parse-options --i16 -32768 >out &&
+	test_grep "i16: -32768" out &&
+	test_must_fail test-tool parse-options --i16 -32769 2>err &&
+	test_grep "value -32769 for option .i16. not in range \[-32768,32767\]" err
+'
+
+test_expect_success 'u16 limits range' '
+	test-tool parse-options --u16 65535 >out &&
+	test_grep "u16: 65535" out &&
+	test_must_fail test-tool parse-options --u16 65536 2>err &&
+	test_grep "value 65536 for option .u16. not in range \[0,65535\]" err
+'
+
 test_done