threads / patch / 63708

patch, 6 partsparse-options: add more precision handling

Subject: [PATCH 0/6] parse-options: add more precision handling

## tl;dr

31 messages between Jun 29, 2025 and Jul 9, 2025. Diffs are folded; open one to read it.

replies: 30people: 3as markdown or json

René Scharfe· Jun 29, 2025, 11:43 UTC · lore

Extend precision handling to all options that currently blindly use an int pointer to access value variables. This allows the safe use of smaller and larger integer types.

Sign handling might be nice as well (especially for OPTION_COUNTUP), but is out of scope for this series.

  parse-options: add precision handling for PARSE_OPT_CMDMODE
  parse-options: add precision handling for OPTION_SET_INT
  parse-options: add precision handling for OPTION_BIT
  parse-options: add precision handling for OPTION_NEGBIT
  parse-options: add precision handling for OPTION_BITOP
  parse-options: add precision handling for OPTION_COUNTUP
 builtin/am.c                  |   1 +
 builtin/rebase.c              |   1 +
 builtin/update-index.c        |   6 ++
 builtin/write-tree.c          |   1 +
 parse-options.c               | 142 ++++++++++++++++++++++++----------
 parse-options.h               |   7 ++
 t/helper/test-parse-options.c |  17 +++-
 7 files changed, 129 insertions(+), 46 deletions(-)
-- 
2.50.0
René Scharfe· Jun 29, 2025, 11:50 UTC · re: René Scharfe · lore

[PATCH 1/6] parse-options: add precision handling for PARSE_OPT_CMDMODE

Build on 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) to support value variables of different sizes for PARSE_OPT_CMDMODE options. Do that by requiring their "precision" to be set and casting their "value" pointer accordingly.

get_value() needs to access all PARSE_OPT_CMDMODE values in addition to the actual value it is supposed to get to detect conflicting changes. Give it an example struct option pointer in cmdmode_list instead of just the "value" pointer to allow it to use the proper "precision".

Use optbug() in get_int_value() to report options with unsupported "precision" values without requiring enum opt_parsed flags, as we don't have them in build_cmdmode_list(). Use BUG right afterwards to abort for uses outside of build_cmdmode_list() by aborting immediately.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 builtin/am.c                  |  1 +
 parse-options.c               | 36 ++++++++++++++++++++++++++---------
 parse-options.h               |  1 +
 t/helper/test-parse-options.c | 13 ++++++++++---
 4 files changed, 39 insertions(+), 12 deletions(-)
Show changes to 4 files +39 −12

builtin/am.c, parse-options.c, parse-options.h, t/helper/test-parse-options.c

diff --git a/builtin/am.c b/builtin/am.c
index a800003340..c9d925f7b9 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2406,6 +2406,7 @@ int cmd_am(int argc,
 			.type = OPTION_CALLBACK,
 			.long_name = "show-current-patch",
 			.value = &resume_mode,
+			.precision = sizeof(resume_mode),
 			.argh = "(diff|raw)",
 			.help = N_("show the patch being applied"),
 			.flags = PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
diff --git a/parse-options.c b/parse-options.c
index a9a39ecaef..da07a000a3 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -68,6 +68,23 @@ static char *fix_filename(const char *prefix, const char *file)
 		return prefix_filename_except_for_dash(prefix, file);
 }
 
+static intmax_t get_int_value(const struct option *opt)
+{
+	switch (opt->precision) {
+	case sizeof(int8_t):
+		return *(int8_t *)opt->value;
+	case sizeof(int16_t):
+		return *(int16_t *)opt->value;
+	case sizeof(int32_t):
+		return *(int32_t *)opt->value;
+	case sizeof(int64_t):
+		return *(int64_t *)opt->value;
+	default:
+		optbug(opt, "has invalid precision");
+		BUG("invalid 'struct option'");
+	}
+}
+
 static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 					  const struct option *opt,
 					  enum opt_parsed flags,
@@ -266,8 +283,8 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 }
 
 struct parse_opt_cmdmode_list {
-	int value, *value_ptr;
-	const struct option *opt;
+	intmax_t value;
+	const struct option *opt, *reference_opt;
 	const char *arg;
 	enum opt_parsed flags;
 	struct parse_opt_cmdmode_list *next;
@@ -280,19 +297,18 @@ static void build_cmdmode_list(struct parse_opt_ctx_t *ctx,
 
 	for (; opts->type != OPTION_END; opts++) {
 		struct parse_opt_cmdmode_list *elem = ctx->cmdmode_list;
-		int *value_ptr = opts->value;
 
-		if (!(opts->flags & PARSE_OPT_CMDMODE) || !value_ptr)
+		if (!(opts->flags & PARSE_OPT_CMDMODE) || !opts->value)
 			continue;
 
-		while (elem && elem->value_ptr != value_ptr)
+		while (elem && elem->reference_opt->value != opts->value)
 			elem = elem->next;
 		if (elem)
 			continue;
 
 		CALLOC_ARRAY(elem, 1);
-		elem->value_ptr = value_ptr;
-		elem->value = *value_ptr;
+		elem->reference_opt = opts;
+		elem->value = get_int_value(opts);
 		elem->next = ctx->cmdmode_list;
 		ctx->cmdmode_list = elem;
 	}
@@ -317,7 +333,9 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
 	char *opt_name, *other_opt_name;
 
 	for (; elem; elem = elem->next) {
-		if (*elem->value_ptr == elem->value)
+		intmax_t new_value = get_int_value(elem->reference_opt);
+
+		if (new_value == elem->value)
 			continue;
 
 		if (elem->opt &&
@@ -327,7 +345,7 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
 		elem->opt = opt;
 		elem->arg = arg;
 		elem->flags = flags;
-		elem->value = *elem->value_ptr;
+		elem->value = new_value;
 	}
 
 	if (result || !elem)
diff --git a/parse-options.h b/parse-options.h
index 91c3e3c29b..c75a473c9e 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -269,6 +269,7 @@ struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.help = (h), \
 	.flags = PARSE_OPT_CMDMODE|PARSE_OPT_NOARG|PARSE_OPT_NONEG | (f), \
 	.defval = (i), \
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index f2663dd0c0..1e03ff88f6 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -148,9 +148,16 @@ int cmd__parse_options(int argc, const char **argv)
 		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),
-		OPT_CALLBACK_F(0, "mode34", &integer, "(3|4)",
-			"set integer to 3 or 4 (cmdmode option)",
-			PARSE_OPT_CMDMODE, mode34_callback),
+		{
+			.type = OPTION_CALLBACK,
+			.long_name = "mode34",
+			.value = &integer,
+			.precision = sizeof(integer),
+			.argh = "(3|4)",
+			.help = "set integer to 3 or 4 (cmdmode option)",
+			.flags = PARSE_OPT_CMDMODE,
+			.callback = mode34_callback,
+		},
 		OPT_CALLBACK('L', "length", &integer, "str",
 			"get length of <str>", length_callback),
 		OPT_FILENAME('F', "file", &file, "set file to <file>"),
-- 
2.50.0
Patrick Steinhardt· Jul 1, 2025, 10:55 UTC · re: René Scharfe · lore

Re: [PATCH 1/6] parse-options: add precision handling for PARSE_OPT_CMDMODE

On Sun, Jun 29, 2025 at 01:50:31PM +0200, René Scharfe wrote:
> Build on 09705696f7 (parse-options: introduce precision handling for
> `OPTION_INTEGER`, 2025-04-17) to support value variables of different
> sizes for PARSE_OPT_CMDMODE options.  Do that by requiring their
> "precision" to be set and casting their "value" pointer accordingly.
Makes sense.
Show 9 quoted lines
> get_value() needs to access all PARSE_OPT_CMDMODE values in addition to
> the actual value it is supposed to get to detect conflicting changes.
> Give it an example struct option pointer in cmdmode_list instead of just
> the "value" pointer to allow it to use the proper "precision".
> 
> Use optbug() in get_int_value() to report options with unsupported
> "precision" values without requiring enum opt_parsed flags, as we don't
> have them in build_cmdmode_list().  Use BUG right afterwards to abort
> for uses outside of build_cmdmode_list() by aborting immediately.

Hm. I have a bit of a hard time understanding these two paragraphs, to be honest. Might be that my brain is still in vacation mode.

Show 13 quoted lines
> @@ -280,19 +297,18 @@ static void build_cmdmode_list(struct parse_opt_ctx_t *ctx,
>  
>  	for (; opts->type != OPTION_END; opts++) {
>  		struct parse_opt_cmdmode_list *elem = ctx->cmdmode_list;
> -		int *value_ptr = opts->value;
>  
> -		if (!(opts->flags & PARSE_OPT_CMDMODE) || !value_ptr)
> +		if (!(opts->flags & PARSE_OPT_CMDMODE) || !opts->value)
>  			continue;
>  
> -		while (elem && elem->value_ptr != value_ptr)
> +		while (elem && elem->reference_opt->value != opts->value)
>  			elem = elem->next;

Hm. Previously we checked for the pointers to be equal, now we check for the value to be equal. Are we sure that this is always equivalent? Can't it ever be that two elements might have the same value?

Patrick
René Scharfe· Jul 1, 2025, 15:15 UTC · re: Patrick Steinhardt · lore

Re: [PATCH 1/6] parse-options: add precision handling for PARSE_OPT_CMDMODE

On 7/1/25 12:55 PM, Patrick Steinhardt wrote:
Show 20 quoted lines
> On Sun, Jun 29, 2025 at 01:50:31PM +0200, René Scharfe wrote:
>> Build on 09705696f7 (parse-options: introduce precision handling for
>> `OPTION_INTEGER`, 2025-04-17) to support value variables of different
>> sizes for PARSE_OPT_CMDMODE options.  Do that by requiring their
>> "precision" to be set and casting their "value" pointer accordingly.
> 
> Makes sense.
> 
>> get_value() needs to access all PARSE_OPT_CMDMODE values in addition to
>> the actual value it is supposed to get to detect conflicting changes.
>> Give it an example struct option pointer in cmdmode_list instead of just
>> the "value" pointer to allow it to use the proper "precision".
>>
>> Use optbug() in get_int_value() to report options with unsupported
>> "precision" values without requiring enum opt_parsed flags, as we don't
>> have them in build_cmdmode_list().  Use BUG right afterwards to abort
>> for uses outside of build_cmdmode_list() by aborting immediately.
> 
> Hm. I have a bit of a hard time understanding these two paragraphs, to
> be honest. Might be that my brain is still in vacation mode.
Or it might be my needing-a-vacation mode.

get_value() checks all PARSE_OPT_CMDMODE value variables. It does that to detect changes to such a variable by non-PARSE_OPT_CMDMODE options. "All" sounds grand, but actual commands only have at most a single PARSE_OPT_CMDMODE value variable. But they could have many more. Anyway, the patch gives it a struct option so that it can use the proper precision when dereferencing value, a void pointer.

optbug() calls the macro "bug". It reports a bug just like BUG, but does not abort. It's intended for cases where we want to report all the bugs that we can find instead of forcing developers to fix them one by one and recompile in between. That would be nice in build_cmdmode_list(). I still force an abort by following it with BUG, though, because handling invalid "precision" values would be tedious in the other callers.

Show 18 quoted lines
> 
>> @@ -280,19 +297,18 @@ static void build_cmdmode_list(struct parse_opt_ctx_t *ctx,
>>  
>>  	for (; opts->type != OPTION_END; opts++) {
>>  		struct parse_opt_cmdmode_list *elem = ctx->cmdmode_list;
>> -		int *value_ptr = opts->value;
>>  
>> -		if (!(opts->flags & PARSE_OPT_CMDMODE) || !value_ptr)
>> +		if (!(opts->flags & PARSE_OPT_CMDMODE) || !opts->value)
>>  			continue;
>>  
>> -		while (elem && elem->value_ptr != value_ptr)
>> +		while (elem && elem->reference_opt->value != opts->value)
>>  			elem = elem->next;
> 
> Hm. Previously we checked for the pointers to be equal, now we check for
> the value to be equal. Are we sure that this is always equivalent? Can't
> it ever be that two elements might have the same value?

The "value" member of struct option is a void pointer. We still compare pointers here.

René
René Scharfe· Jun 29, 2025, 11:50 UTC · re: René Scharfe · lore

[PATCH 2/6] parse-options: add precision handling for OPTION_SET_INT

Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_SET_INT. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits.

Factor out the casting code from the part of do_get_value() that handles OPTION_INTEGER to avoid code duplication. We're going to use it in the next patches as well.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 builtin/update-index.c        |  6 ++++
 parse-options.c               | 56 ++++++++++++++++++++++-------------
 parse-options.h               |  2 ++
 t/helper/test-parse-options.c |  1 +
 4 files changed, 45 insertions(+), 20 deletions(-)
Show changes to 4 files +45 −20

builtin/update-index.c, parse-options.c, parse-options.h, t/helper/test-parse-options.c

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 538b619ba4..0c1d4ed55b 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -981,6 +981,7 @@ int cmd_update_index(int argc,
 			.type = OPTION_SET_INT,
 			.long_name = "assume-unchanged",
 			.value = &mark_valid_only,
+			.precision = sizeof(mark_valid_only),
 			.help = N_("mark files as \"not changing\""),
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			.defval = MARK_FLAG,
@@ -989,6 +990,7 @@ int cmd_update_index(int argc,
 			.type = OPTION_SET_INT,
 			.long_name = "no-assume-unchanged",
 			.value = &mark_valid_only,
+			.precision = sizeof(mark_valid_only),
 			.help = N_("clear assumed-unchanged bit"),
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			.defval = UNMARK_FLAG,
@@ -997,6 +999,7 @@ int cmd_update_index(int argc,
 			.type = OPTION_SET_INT,
 			.long_name = "skip-worktree",
 			.value = &mark_skip_worktree_only,
+			.precision = sizeof(mark_skip_worktree_only),
 			.help = N_("mark files as \"index-only\""),
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			.defval = MARK_FLAG,
@@ -1005,6 +1008,7 @@ int cmd_update_index(int argc,
 			.type = OPTION_SET_INT,
 			.long_name = "no-skip-worktree",
 			.value = &mark_skip_worktree_only,
+			.precision = sizeof(mark_skip_worktree_only),
 			.help = N_("clear skip-worktree bit"),
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			.defval = UNMARK_FLAG,
@@ -1079,6 +1083,7 @@ int cmd_update_index(int argc,
 			.type = OPTION_SET_INT,
 			.long_name = "fsmonitor-valid",
 			.value = &mark_fsmonitor_only,
+			.precision = sizeof(mark_fsmonitor_only),
 			.help = N_("mark files as fsmonitor valid"),
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			.defval = MARK_FLAG,
@@ -1087,6 +1092,7 @@ int cmd_update_index(int argc,
 			.type = OPTION_SET_INT,
 			.long_name = "no-fsmonitor-valid",
 			.value = &mark_fsmonitor_only,
+			.precision = sizeof(mark_fsmonitor_only),
 			.help = N_("clear fsmonitor valid bit"),
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			.defval = UNMARK_FLAG,
diff --git a/parse-options.c b/parse-options.c
index da07a000a3..bbb68603cc 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -85,6 +85,36 @@ static intmax_t get_int_value(const struct option *opt)
 	}
 }
 
+static enum parse_opt_result set_int_value(const struct option *opt,
+					   enum opt_parsed flags,
+					   intmax_t value)
+{
+	switch (opt->precision) {
+	case sizeof(int8_t):
+		*(int8_t *)opt->value = value;
+		return 0;
+	case sizeof(int16_t):
+		*(int16_t *)opt->value = value;
+		return 0;
+	case sizeof(int32_t):
+		*(int32_t *)opt->value = value;
+		return 0;
+	case sizeof(int64_t):
+		*(int64_t *)opt->value = value;
+		return 0;
+	default:
+		BUG("invalid precision for option %s", optname(opt, flags));
+	}
+}
+
+static int signed_int_fits(intmax_t value, size_t size)
+{
+	size_t bits = size * CHAR_BIT;
+	intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits);
+	intmax_t lower_bound = -upper_bound - 1;
+	return lower_bound <= value && value <= upper_bound;
+}
+
 static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 					  const struct option *opt,
 					  enum opt_parsed flags,
@@ -133,8 +163,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 		return 0;
 
 	case OPTION_SET_INT:
-		*(int *)opt->value = unset ? 0 : opt->defval;
-		return 0;
+		return set_int_value(opt, flags, unset ? 0 : opt->defval);
 
 	case OPTION_STRING:
 		if (unset)
@@ -216,23 +245,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 			return error(_("value %s for %s not in range [%"PRIdMAX",%"PRIdMAX"]"),
 				     arg, optname(opt, flags), (intmax_t)lower_bound, (intmax_t)upper_bound);
 
-		switch (opt->precision) {
-		case 1:
-			*(int8_t *)opt->value = value;
-			return 0;
-		case 2:
-			*(int16_t *)opt->value = value;
-			return 0;
-		case 4:
-			*(int32_t *)opt->value = value;
-			return 0;
-		case 8:
-			*(int64_t *)opt->value = value;
-			return 0;
-		default:
-			BUG("invalid precision for option %s",
-			    optname(opt, flags));
-		}
+		return set_int_value(opt, flags, value);
 	}
 	case OPTION_UNSIGNED:
 	{
@@ -604,10 +617,13 @@ static void parse_options_check(const struct option *opts)
 		    opts->long_name && !(opts->flags & PARSE_OPT_NONEG))
 			optbug(opts, "OPTION_SET_INT 0 should not be negatable");
 		switch (opts->type) {
+		case OPTION_SET_INT:
+			if (!signed_int_fits(opts->defval, opts->precision))
+				optbug(opts, "has invalid defval");
+			/* fallthru */
 		case OPTION_COUNTUP:
 		case OPTION_BIT:
 		case OPTION_NEGBIT:
-		case OPTION_SET_INT:
 		case OPTION_NUMBER:
 			if ((opts->flags & PARSE_OPT_OPTARG) ||
 			    !(opts->flags & PARSE_OPT_NOARG))
diff --git a/parse-options.h b/parse-options.h
index c75a473c9e..71516e4b5b 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -190,6 +190,7 @@ struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.help = (h), \
 	.flags = PARSE_OPT_NOARG | (f), \
 	.defval = (i), \
@@ -260,6 +261,7 @@ struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.help = (h), \
 	.flags = PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, \
 	.defval = 1, \
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index 1e03ff88f6..2ba2546d70 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -131,6 +131,7 @@ int cmd__parse_options(int argc, const char **argv)
 			.short_name = 'B',
 			.long_name = "no-fear",
 			.value = &boolean,
+			.precision = sizeof(boolean),
 			.help = "be brave",
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			.defval = 1,
-- 
2.50.0
Patrick Steinhardt· Jul 1, 2025, 10:55 UTC · re: René Scharfe · lore

Re: [PATCH 2/6] parse-options: add precision handling for OPTION_SET_INT

On Sun, Jun 29, 2025 at 01:50:39PM +0200, René Scharfe wrote:
Show 29 quoted lines
> diff --git a/parse-options.c b/parse-options.c
> index da07a000a3..bbb68603cc 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -85,6 +85,36 @@ static intmax_t get_int_value(const struct option *opt)
>  	}
>  }
>  
> +static enum parse_opt_result set_int_value(const struct option *opt,
> +					   enum opt_parsed flags,
> +					   intmax_t value)
> +{
> +	switch (opt->precision) {
> +	case sizeof(int8_t):
> +		*(int8_t *)opt->value = value;
> +		return 0;
> +	case sizeof(int16_t):
> +		*(int16_t *)opt->value = value;
> +		return 0;
> +	case sizeof(int32_t):
> +		*(int32_t *)opt->value = value;
> +		return 0;
> +	case sizeof(int64_t):
> +		*(int64_t *)opt->value = value;
> +		return 0;
> +	default:
> +		BUG("invalid precision for option %s", optname(opt, flags));
> +	}
> +}

The function only ever dies or returns successfully, so can't we make it return nothing instead? On the other hand it does make a couple of callsites a bit nicer to read.

Show 8 quoted lines
> +static int signed_int_fits(intmax_t value, size_t size)
> +{
> +	size_t bits = size * CHAR_BIT;
> +	intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits);
> +	intmax_t lower_bound = -upper_bound - 1;
> +	return lower_bound <= value && value <= upper_bound;
> +}
> +

Should we s/size/precision/ so that it's clear what kind of size this exactly is?

Patrick
René Scharfe· Jul 1, 2025, 15:54 UTC · re: Patrick Steinhardt · lore

Re: [PATCH 2/6] parse-options: add precision handling for OPTION_SET_INT

On 7/1/25 12:55 PM, Patrick Steinhardt wrote:
Show 34 quoted lines
> On Sun, Jun 29, 2025 at 01:50:39PM +0200, René Scharfe wrote:
>> diff --git a/parse-options.c b/parse-options.c
>> index da07a000a3..bbb68603cc 100644
>> --- a/parse-options.c
>> +++ b/parse-options.c
>> @@ -85,6 +85,36 @@ static intmax_t get_int_value(const struct option *opt)
>>  	}
>>  }
>>  
>> +static enum parse_opt_result set_int_value(const struct option *opt,
>> +					   enum opt_parsed flags,
>> +					   intmax_t value)
>> +{
>> +	switch (opt->precision) {
>> +	case sizeof(int8_t):
>> +		*(int8_t *)opt->value = value;
>> +		return 0;
>> +	case sizeof(int16_t):
>> +		*(int16_t *)opt->value = value;
>> +		return 0;
>> +	case sizeof(int32_t):
>> +		*(int32_t *)opt->value = value;
>> +		return 0;
>> +	case sizeof(int64_t):
>> +		*(int64_t *)opt->value = value;
>> +		return 0;
>> +	default:
>> +		BUG("invalid precision for option %s", optname(opt, flags));
>> +	}
>> +}
> 
> The function only ever dies or returns successfully, so can't we make it
> return nothing instead? On the other hand it does make a couple of
> callsites a bit nicer to read.

We can. Not sure we should, obviously, but I don't have strong feelings either way.

Show 12 quoted lines
> 
>> +static int signed_int_fits(intmax_t value, size_t size)
>> +{
>> +	size_t bits = size * CHAR_BIT;
>> +	intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits);
>> +	intmax_t lower_bound = -upper_bound - 1;
>> +	return lower_bound <= value && value <= upper_bound;
>> +}
>> +
> 
> Should we s/size/precision/ so that it's clear what kind of size this
> exactly is?

It's the width of an integer variable as in sizeof(), so the name fits. We can inline this single-caller function if it's indeed confusing.

René
Patrick Steinhardt· Jul 2, 2025, 02:31 UTC · re: René Scharfe · lore

Re: [PATCH 2/6] parse-options: add precision handling for OPTION_SET_INT

On Tue, Jul 01, 2025 at 05:54:41PM +0200, René Scharfe wrote:
Show 19 quoted lines
> On 7/1/25 12:55 PM, Patrick Steinhardt wrote:
> > On Sun, Jun 29, 2025 at 01:50:39PM +0200, René Scharfe wrote:
> >> diff --git a/parse-options.c b/parse-options.c
> >> index da07a000a3..bbb68603cc 100644
> >> --- a/parse-options.c
> >> +++ b/parse-options.c
> >> +static int signed_int_fits(intmax_t value, size_t size)
> >> +{
> >> +	size_t bits = size * CHAR_BIT;
> >> +	intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits);
> >> +	intmax_t lower_bound = -upper_bound - 1;
> >> +	return lower_bound <= value && value <= upper_bound;
> >> +}
> >> +
> > 
> > Should we s/size/precision/ so that it's clear what kind of size this
> > exactly is?
> It's the width of an integer variable as in sizeof(), so the name fits.
> We can inline this single-caller function if it's indeed confusing.

The issue to me is rather that it's unclear what the unit is. Is it size in bytes, bits, nibbles? You wouldn't know that the expectation is that the caller passes in `sizeof()` without taking a deeper look.

In any case, this is only a minor nit in the first place, not worth much bikeshedding.

Patrick
René Scharfe· Jun 29, 2025, 11:50 UTC · re: René Scharfe · lore

[PATCH 3/6] parse-options: add precision handling for OPTION_BIT

Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_BIT. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 builtin/write-tree.c |  1 +
 parse-options.c      | 11 +++++++----
 parse-options.h      |  1 +
 3 files changed, 9 insertions(+), 4 deletions(-)
Show changes to 3 files +9 −4

builtin/write-tree.c, parse-options.c, parse-options.h

diff --git a/builtin/write-tree.c b/builtin/write-tree.c
index 5a8dc377ec..cfec044710 100644
--- a/builtin/write-tree.c
+++ b/builtin/write-tree.c
@@ -35,6 +35,7 @@ int cmd_write_tree(int argc,
 			.type = OPTION_BIT,
 			.long_name = "ignore-cache-tree",
 			.value = &flags,
+			.precision = sizeof(flags),
 			.help = N_("only useful for debugging"),
 			.flags = PARSE_OPT_HIDDEN | PARSE_OPT_NOARG,
 			.defval = WRITE_TREE_IGNORE_CACHE_TREE,
diff --git a/parse-options.c b/parse-options.c
index bbb68603cc..47a77d2cea 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -136,11 +136,14 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 		return opt->ll_callback(p, opt, NULL, unset);
 
 	case OPTION_BIT:
+	{
+		intmax_t value = get_int_value(opt);
 		if (unset)
-			*(int *)opt->value &= ~opt->defval;
+			value &= ~opt->defval;
 		else
-			*(int *)opt->value |= opt->defval;
-		return 0;
+			value |= opt->defval;
+		return set_int_value(opt, flags, value);
+	}
 
 	case OPTION_NEGBIT:
 		if (unset)
@@ -618,11 +621,11 @@ static void parse_options_check(const struct option *opts)
 			optbug(opts, "OPTION_SET_INT 0 should not be negatable");
 		switch (opts->type) {
 		case OPTION_SET_INT:
+		case OPTION_BIT:
 			if (!signed_int_fits(opts->defval, opts->precision))
 				optbug(opts, "has invalid defval");
 			/* fallthru */
 		case OPTION_COUNTUP:
-		case OPTION_BIT:
 		case OPTION_NEGBIT:
 		case OPTION_NUMBER:
 			if ((opts->flags & PARSE_OPT_OPTARG) ||
diff --git a/parse-options.h b/parse-options.h
index 71516e4b5b..6501ca3c27 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -172,6 +172,7 @@ struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.help = (h), \
 	.flags = PARSE_OPT_NOARG|(f), \
 	.callback = NULL, \
-- 
2.50.0
René Scharfe· Jun 29, 2025, 11:51 UTC · re: René Scharfe · lore

[PATCH 4/6] parse-options: add precision handling for OPTION_NEGBIT

Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_NEGBIT. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 builtin/rebase.c |  1 +
 parse-options.c  | 11 +++++++----
 parse-options.h  |  1 +
 3 files changed, 9 insertions(+), 4 deletions(-)
Show changes to 3 files +9 −4

builtin/rebase.c, parse-options.c, parse-options.h

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 2e8c4ee678..e90562a3b8 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1128,6 +1128,7 @@ int cmd_rebase(int argc,
 			.short_name = 'n',
 			.long_name = "no-stat",
 			.value = &options.flags,
+			.precision = sizeof(options.flags),
 			.help = N_("do not show diffstat of what changed upstream"),
 			.flags = PARSE_OPT_NOARG,
 			.defval = REBASE_DIFFSTAT,
diff --git a/parse-options.c b/parse-options.c
index 47a77d2cea..6bd7158806 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -146,11 +146,14 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 	}
 
 	case OPTION_NEGBIT:
+	{
+		intmax_t value = get_int_value(opt);
 		if (unset)
-			*(int *)opt->value |= opt->defval;
+			value |= opt->defval;
 		else
-			*(int *)opt->value &= ~opt->defval;
-		return 0;
+			value &= ~opt->defval;
+		return set_int_value(opt, flags, value);
+	}
 
 	case OPTION_BITOP:
 		if (unset)
@@ -622,11 +625,11 @@ static void parse_options_check(const struct option *opts)
 		switch (opts->type) {
 		case OPTION_SET_INT:
 		case OPTION_BIT:
+		case OPTION_NEGBIT:
 			if (!signed_int_fits(opts->defval, opts->precision))
 				optbug(opts, "has invalid defval");
 			/* fallthru */
 		case OPTION_COUNTUP:
-		case OPTION_NEGBIT:
 		case OPTION_NUMBER:
 			if ((opts->flags & PARSE_OPT_OPTARG) ||
 			    !(opts->flags & PARSE_OPT_NOARG))
diff --git a/parse-options.h b/parse-options.h
index 6501ca3c27..076f88b384 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -250,6 +250,7 @@ struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.help = (h), \
 	.flags = PARSE_OPT_NOARG, \
 	.defval = (b), \
-- 
2.50.0
René Scharfe· Jun 29, 2025, 11:51 UTC · re: René Scharfe · lore

[PATCH 5/6] parse-options: add precision handling for OPTION_BITOP

Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_BITOP. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits.

Checking "defval" has the side-effect of also requiring PARSE_OPT_NOARG. This is sensible, as OPTION_BITOP doesn't handle any arguments, so we take this unintended benefit.

Don't check "extra", though, as its value is only used to clear bits, so cannot lead to an overflow. Not checking continues to allow e.g., using -1 to clear all bits even if the value variable has a narrower type than intptr_t.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 parse-options.c | 10 +++++++---
 parse-options.h |  1 +
 2 files changed, 8 insertions(+), 3 deletions(-)
Show changes to 2 files +8 −3

parse-options.c, parse-options.h

diff --git a/parse-options.c b/parse-options.c
index 6bd7158806..0dc9b0324a 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -156,11 +156,14 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 	}
 
 	case OPTION_BITOP:
+	{
+		intmax_t value = get_int_value(opt);
 		if (unset)
 			BUG("BITOP can't have unset form");
-		*(int *)opt->value &= ~opt->extra;
-		*(int *)opt->value |= opt->defval;
-		return 0;
+		value &= ~opt->extra;
+		value |= opt->defval;
+		return set_int_value(opt, flags, value);
+	}
 
 	case OPTION_COUNTUP:
 		if (*(int *)opt->value < 0)
@@ -626,6 +629,7 @@ static void parse_options_check(const struct option *opts)
 		case OPTION_SET_INT:
 		case OPTION_BIT:
 		case OPTION_NEGBIT:
+		case OPTION_BITOP:
 			if (!signed_int_fits(opts->defval, opts->precision))
 				optbug(opts, "has invalid defval");
 			/* fallthru */
diff --git a/parse-options.h b/parse-options.h
index 076f88b384..8bdf469ae9 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -240,6 +240,7 @@ struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.help = (h), \
 	.flags = PARSE_OPT_NOARG|PARSE_OPT_NONEG, \
 	.defval = (set), \
-- 
2.50.0
Patrick Steinhardt· Jul 1, 2025, 10:55 UTC · re: René Scharfe · lore

Re: [PATCH 5/6] parse-options: add precision handling for OPTION_BITOP

On Sun, Jun 29, 2025 at 01:51:19PM +0200, René Scharfe wrote:
Show 7 quoted lines
> Similar to 09705696f7 (parse-options: introduce precision handling for
> `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes
> for OPTION_BITOP.  Do that by requiring their "precision" to be set,
> casting their "value" pointer accordingly and checking whether the value
> fits.
> 
> Checking "defval" has the side-effect of also requiring PARSE_OPT_NOARG.

Hm, requiring PARSE_OPT_NOARG for what? I cannot see it being touched in this patch at all, so I'm a but puzzled.

Patrick
René Scharfe· Jul 1, 2025, 15:21 UTC · re: Patrick Steinhardt · lore

Re: [PATCH 5/6] parse-options: add precision handling for OPTION_BITOP

On 7/1/25 12:55 PM, Patrick Steinhardt wrote:
Show 11 quoted lines
> On Sun, Jun 29, 2025 at 01:51:19PM +0200, René Scharfe wrote:
>> Similar to 09705696f7 (parse-options: introduce precision handling for
>> `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes
>> for OPTION_BITOP.  Do that by requiring their "precision" to be set,
>> casting their "value" pointer accordingly and checking whether the value
>> fits.
>>
>> Checking "defval" has the side-effect of also requiring PARSE_OPT_NOARG.
> 
> Hm, requiring PARSE_OPT_NOARG for what? I cannot see it being touched in
> this patch at all, so I'm a but puzzled.

For options with OPTION_BITOP. Adding the defval check also adds the no-argument check by falling through to it:

Show changes to parse-options.c +1 −0
diff --git a/parse-options.c b/parse-options.c
index 6bd7158806..0dc9b0324a 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -620,18 +623,19 @@ static void parse_options_check(const struct option *opts)
 			optbug(opts, "uses feature "
 			       "not supported for dashless options");
 		if (opts->type == OPTION_SET_INT && !opts->defval &&
 		    opts->long_name && !(opts->flags & PARSE_OPT_NONEG))
 			optbug(opts, "OPTION_SET_INT 0 should not be negatable");
 		switch (opts->type) {
 		case OPTION_SET_INT:
 		case OPTION_BIT:
 		case OPTION_NEGBIT:
+		case OPTION_BITOP:
 			if (!signed_int_fits(opts->defval, opts->precision))
 				optbug(opts, "has invalid defval");
 			/* fallthru */
 		case OPTION_COUNTUP:
 		case OPTION_NUMBER:
 			if ((opts->flags & PARSE_OPT_OPTARG) ||
 			    !(opts->flags & PARSE_OPT_NOARG))
 				optbug(opts, "should not accept an argument");
 			break;
Patrick Steinhardt· Jul 2, 2025, 02:33 UTC · re: René Scharfe · lore

Re: [PATCH 5/6] parse-options: add precision handling for OPTION_BITOP

On Tue, Jul 01, 2025 at 05:21:11PM +0200, René Scharfe wrote:
Show 40 quoted lines
> On 7/1/25 12:55 PM, Patrick Steinhardt wrote:
> > On Sun, Jun 29, 2025 at 01:51:19PM +0200, René Scharfe wrote:
> >> Similar to 09705696f7 (parse-options: introduce precision handling for
> >> `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes
> >> for OPTION_BITOP.  Do that by requiring their "precision" to be set,
> >> casting their "value" pointer accordingly and checking whether the value
> >> fits.
> >>
> >> Checking "defval" has the side-effect of also requiring PARSE_OPT_NOARG.
> > 
> > Hm, requiring PARSE_OPT_NOARG for what? I cannot see it being touched in
> > this patch at all, so I'm a but puzzled.
> 
> For options with OPTION_BITOP.  Adding the defval check also adds the
> no-argument check by falling through to it:
> 
> diff --git a/parse-options.c b/parse-options.c
> index 6bd7158806..0dc9b0324a 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -620,18 +623,19 @@ static void parse_options_check(const struct option *opts)
>  			optbug(opts, "uses feature "
>  			       "not supported for dashless options");
>  		if (opts->type == OPTION_SET_INT && !opts->defval &&
>  		    opts->long_name && !(opts->flags & PARSE_OPT_NONEG))
>  			optbug(opts, "OPTION_SET_INT 0 should not be negatable");
>  		switch (opts->type) {
>  		case OPTION_SET_INT:
>  		case OPTION_BIT:
>  		case OPTION_NEGBIT:
> +		case OPTION_BITOP:
>  			if (!signed_int_fits(opts->defval, opts->precision))
>  				optbug(opts, "has invalid defval");
>  			/* fallthru */
>  		case OPTION_COUNTUP:
>  		case OPTION_NUMBER:
>  			if ((opts->flags & PARSE_OPT_OPTARG) ||
>  			    !(opts->flags & PARSE_OPT_NOARG))
>  				optbug(opts, "should not accept an argument");
>  			break;

Ah, now I see it, the extended context definitely helps. Might be nice to point out explicitly in the commit message that it's about the fallthrough behaviour.

Patrick
René Scharfe· Jun 29, 2025, 11:51 UTC · re: René Scharfe · lore

[PATCH 6/6] parse-options: add precision handling for OPTION_COUNTUP

Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_COUNTUP. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 parse-options.c               | 22 +++++++++++++++++-----
 parse-options.h               |  1 +
 t/helper/test-parse-options.c |  3 +++
 3 files changed, 21 insertions(+), 5 deletions(-)
Show changes to 3 files +21 −5

parse-options.c, parse-options.h, t/helper/test-parse-options.c

diff --git a/parse-options.c b/parse-options.c
index 0dc9b0324a..0dd08a3a77 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -166,10 +166,22 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 	}
 
 	case OPTION_COUNTUP:
-		if (*(int *)opt->value < 0)
-			*(int *)opt->value = 0;
-		*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
-		return 0;
+	{
+		size_t bits = CHAR_BIT * opt->precision;
+		intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits);
+		intmax_t value = get_int_value(opt);
+
+		if (value < 0)
+			value = 0;
+		if (unset)
+			value = 0;
+		else if (value < upper_bound)
+			value++;
+		else
+			return error(_("value for %s exceeds %"PRIdMAX),
+				     optname(opt, flags), upper_bound);
+		return set_int_value(opt, flags, value);
+	}
 
 	case OPTION_SET_INT:
 		return set_int_value(opt, flags, unset ? 0 : opt->defval);
@@ -630,10 +642,10 @@ static void parse_options_check(const struct option *opts)
 		case OPTION_BIT:
 		case OPTION_NEGBIT:
 		case OPTION_BITOP:
+		case OPTION_COUNTUP:
 			if (!signed_int_fits(opts->defval, opts->precision))
 				optbug(opts, "has invalid defval");
 			/* fallthru */
-		case OPTION_COUNTUP:
 		case OPTION_NUMBER:
 			if ((opts->flags & PARSE_OPT_OPTARG) ||
 			    !(opts->flags & PARSE_OPT_NOARG))
diff --git a/parse-options.h b/parse-options.h
index 8bdf469ae9..312045604d 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -183,6 +183,7 @@ struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.help = (h), \
 	.flags = PARSE_OPT_NOARG|(f), \
 }
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index 2ba2546d70..68579d83f3 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -178,6 +178,7 @@ int cmd__parse_options(int argc, const char **argv)
 			.type = OPTION_COUNTUP,
 			.short_name = '+',
 			.value = &boolean,
+			.precision = sizeof(boolean),
 			.help = "same as -b",
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH,
 		},
@@ -185,6 +186,7 @@ int cmd__parse_options(int argc, const char **argv)
 			.type = OPTION_COUNTUP,
 			.long_name = "ambiguous",
 			.value = &ambiguous,
+			.precision = sizeof(ambiguous),
 			.help = "positive ambiguity",
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 		},
@@ -192,6 +194,7 @@ int cmd__parse_options(int argc, const char **argv)
 			.type = OPTION_COUNTUP,
 			.long_name = "no-ambiguous",
 			.value = &ambiguous,
+			.precision = sizeof(ambiguous),
 			.help = "negative ambiguity",
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 		},
-- 
2.50.0
Patrick Steinhardt· Jul 1, 2025, 10:55 UTC · re: René Scharfe · lore

Re: [PATCH 6/6] parse-options: add precision handling for OPTION_COUNTUP

On Sun, Jun 29, 2025 at 01:51:36PM +0200, René Scharfe wrote:
Show 51 quoted lines
> diff --git a/parse-options.c b/parse-options.c
> index 0dc9b0324a..0dd08a3a77 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -166,10 +166,22 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
>  	}
>  
>  	case OPTION_COUNTUP:
> -		if (*(int *)opt->value < 0)
> -			*(int *)opt->value = 0;
> -		*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
> -		return 0;
> +	{
> +		size_t bits = CHAR_BIT * opt->precision;
> +		intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits);
> +		intmax_t value = get_int_value(opt);
> +
> +		if (value < 0)
> +			value = 0;
> +		if (unset)
> +			value = 0;
> +		else if (value < upper_bound)
> +			value++;
> +		else
> +			return error(_("value for %s exceeds %"PRIdMAX),
> +				     optname(opt, flags), upper_bound);
> +		return set_int_value(opt, flags, value);
> +	}
>  
>  	case OPTION_SET_INT:
>  		return set_int_value(opt, flags, unset ? 0 : opt->defval);
> @@ -630,10 +642,10 @@ static void parse_options_check(const struct option *opts)
>  		case OPTION_BIT:
>  		case OPTION_NEGBIT:
>  		case OPTION_BITOP:
> +		case OPTION_COUNTUP:
>  			if (!signed_int_fits(opts->defval, opts->precision))
>  				optbug(opts, "has invalid defval");
>  			/* fallthru */
> -		case OPTION_COUNTUP:
>  		case OPTION_NUMBER:
>  			if ((opts->flags & PARSE_OPT_OPTARG) ||
>  			    !(opts->flags & PARSE_OPT_NOARG))
> diff --git a/parse-options.h b/parse-options.h
> index 8bdf469ae9..312045604d 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -183,6 +183,7 @@ struct option {
>  	.short_name = (s), \
>  	.long_name = (l), \
>  	.value = (v), \

It's a bit surprising that `COUNTUP` accepts a signed integer, so should we maybe add `BARF_UNLESS_SIGNED(*(v))` here?

> +	.precision = sizeof(*v), \
>  	.help = (h), \
>  	.flags = PARSE_OPT_NOARG|(f), \
>  }
Patrick
René Scharfe· Jul 1, 2025, 16:01 UTC · re: Patrick Steinhardt · lore

Re: [PATCH 6/6] parse-options: add precision handling for OPTION_COUNTUP

On 7/1/25 12:55 PM, Patrick Steinhardt wrote:
Show 55 quoted lines
> On Sun, Jun 29, 2025 at 01:51:36PM +0200, René Scharfe wrote:
>> diff --git a/parse-options.c b/parse-options.c
>> index 0dc9b0324a..0dd08a3a77 100644
>> --- a/parse-options.c
>> +++ b/parse-options.c
>> @@ -166,10 +166,22 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
>>  	}
>>  
>>  	case OPTION_COUNTUP:
>> -		if (*(int *)opt->value < 0)
>> -			*(int *)opt->value = 0;
>> -		*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
>> -		return 0;
>> +	{
>> +		size_t bits = CHAR_BIT * opt->precision;
>> +		intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits);
>> +		intmax_t value = get_int_value(opt);
>> +
>> +		if (value < 0)
>> +			value = 0;
>> +		if (unset)
>> +			value = 0;
>> +		else if (value < upper_bound)
>> +			value++;
>> +		else
>> +			return error(_("value for %s exceeds %"PRIdMAX),
>> +				     optname(opt, flags), upper_bound);
>> +		return set_int_value(opt, flags, value);
>> +	}
>>  
>>  	case OPTION_SET_INT:
>>  		return set_int_value(opt, flags, unset ? 0 : opt->defval);
>> @@ -630,10 +642,10 @@ static void parse_options_check(const struct option *opts)
>>  		case OPTION_BIT:
>>  		case OPTION_NEGBIT:
>>  		case OPTION_BITOP:
>> +		case OPTION_COUNTUP:
>>  			if (!signed_int_fits(opts->defval, opts->precision))
>>  				optbug(opts, "has invalid defval");
>>  			/* fallthru */
>> -		case OPTION_COUNTUP:
>>  		case OPTION_NUMBER:
>>  			if ((opts->flags & PARSE_OPT_OPTARG) ||
>>  			    !(opts->flags & PARSE_OPT_NOARG))
>> diff --git a/parse-options.h b/parse-options.h
>> index 8bdf469ae9..312045604d 100644
>> --- a/parse-options.h
>> +++ b/parse-options.h
>> @@ -183,6 +183,7 @@ struct option {
>>  	.short_name = (s), \
>>  	.long_name = (l), \
>>  	.value = (v), \
> 
> It's a bit surprising that `COUNTUP` accepts a signed integer, so should
> we maybe add `BARF_UNLESS_SIGNED(*(v))` here?

Perhaps, but that would require more changes to callers that use unsigned variables than I can stomach. That's why I declared it out of scope for this series in its cover letter. Later, unless (hopefully) someone beats me to it.

Show 7 quoted lines
> 
>> +	.precision = sizeof(*v), \
>>  	.help = (h), \
>>  	.flags = PARSE_OPT_NOARG|(f), \
>>  }
> 
> Patrick
Patrick Steinhardt· Jul 2, 2025, 02:29 UTC · re: René Scharfe · lore

Re: [PATCH 6/6] parse-options: add precision handling for OPTION_COUNTUP

On Tue, Jul 01, 2025 at 06:01:44PM +0200, René Scharfe wrote:
Show 22 quoted lines
> On 7/1/25 12:55 PM, Patrick Steinhardt wrote:
> > On Sun, Jun 29, 2025 at 01:51:36PM +0200, René Scharfe wrote:
> >> diff --git a/parse-options.c b/parse-options.c
> >> index 0dc9b0324a..0dd08a3a77 100644
> >> --- a/parse-options.c
> >> +++ b/parse-options.c
> >> diff --git a/parse-options.h b/parse-options.h
> >> index 8bdf469ae9..312045604d 100644
> >> --- a/parse-options.h
> >> +++ b/parse-options.h
> >> @@ -183,6 +183,7 @@ struct option {
> >>  	.short_name = (s), \
> >>  	.long_name = (l), \
> >>  	.value = (v), \
> > 
> > It's a bit surprising that `COUNTUP` accepts a signed integer, so should
> > we maybe add `BARF_UNLESS_SIGNED(*(v))` here?
> 
> Perhaps, but that would require more changes to callers that use unsigned
> variables than I can stomach.  That's why I declared it out of scope for
> this series in its cover letter.  Later, unless (hopefully) someone beats
> me to it.
Fine with me, thanks!
Patrick
René Scharfe· Jul 9, 2025, 09:26 UTC · re: René Scharfe · lore

[PATCH v2 0/7] parse-options: add more precision handling

Extend precision handling to all options that currently blindly use an int pointer to access value variables. This allows the safe use of smaller and larger integer types.

Sign handling might be nice as well (especially for OPTION_COUNTUP), but is out of scope for this series.

Changes since v1:
- Enable PARSE_OPT_NOARG checking for OPTION_BITOP explicitly in a new
  separate patch instead of as a side-effect.
- Split out do_get_int_value().
- This allows us to use optbug() only where it makes sense, in
  build_cmdmode_list().  This lets the function report all
  PARSE_OPT_CMDMODE options with invalid precision in one go.
- Use the same BUG call in get_int_value() as in set_int_value(),
  for consistency.
- Store the precision in build_cmdmode_list() instead of an example
  option, which is hopefully easier to follow.
- Rename the size parameter of signed_int_fits() for consistency.
  parse-options: require PARSE_OPT_NOARG for OPTION_BITOP
  parse-options: add precision handling for PARSE_OPT_CMDMODE
  parse-options: add precision handling for OPTION_SET_INT
  parse-options: add precision handling for OPTION_BIT
  parse-options: add precision handling for OPTION_NEGBIT
  parse-options: add precision handling for OPTION_BITOP
  parse-options: add precision handling for OPTION_COUNTUP
 builtin/am.c                  |   1 +
 builtin/rebase.c              |   1 +
 builtin/update-index.c        |   6 ++
 builtin/write-tree.c          |   1 +
 parse-options.c               | 155 +++++++++++++++++++++++++---------
 parse-options.h               |   7 ++
 t/helper/test-parse-options.c |  17 +++-
 7 files changed, 146 insertions(+), 42 deletions(-)
Interdiff against v1:
Show changes to parse-options.c +41 −20
diff --git a/parse-options.c b/parse-options.c
index 0dd08a3a77..5224203ffe 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -68,23 +68,34 @@ static char *fix_filename(const char *prefix, const char *file)
 		return prefix_filename_except_for_dash(prefix, file);
 }
 
-static intmax_t get_int_value(const struct option *opt)
+static int do_get_int_value(const void *value, size_t precision, intmax_t *ret)
 {
-	switch (opt->precision) {
+	switch (precision) {
 	case sizeof(int8_t):
-		return *(int8_t *)opt->value;
+		*ret = *(int8_t *)value;
+		return 0;
 	case sizeof(int16_t):
-		return *(int16_t *)opt->value;
+		*ret = *(int16_t *)value;
+		return 0;
 	case sizeof(int32_t):
-		return *(int32_t *)opt->value;
+		*ret = *(int32_t *)value;
+		return 0;
 	case sizeof(int64_t):
-		return *(int64_t *)opt->value;
+		*ret = *(int64_t *)value;
+		return 0;
 	default:
-		optbug(opt, "has invalid precision");
-		BUG("invalid 'struct option'");
+		return -1;
 	}
 }
 
+static intmax_t get_int_value(const struct option *opt, enum opt_parsed flags)
+{
+	intmax_t ret;
+	if (do_get_int_value(opt->value, opt->precision, &ret))
+		BUG("invalid precision for option %s", optname(opt, flags));
+	return ret;
+}
+
 static enum parse_opt_result set_int_value(const struct option *opt,
 					   enum opt_parsed flags,
 					   intmax_t value)
@@ -107,9 +118,9 @@ static enum parse_opt_result set_int_value(const struct option *opt,
 	}
 }
 
-static int signed_int_fits(intmax_t value, size_t size)
+static int signed_int_fits(intmax_t value, size_t precision)
 {
-	size_t bits = size * CHAR_BIT;
+	size_t bits = precision * CHAR_BIT;
 	intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits);
 	intmax_t lower_bound = -upper_bound - 1;
 	return lower_bound <= value && value <= upper_bound;
@@ -137,7 +148,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 
 	case OPTION_BIT:
 	{
-		intmax_t value = get_int_value(opt);
+		intmax_t value = get_int_value(opt, flags);
 		if (unset)
 			value &= ~opt->defval;
 		else
@@ -147,7 +158,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 
 	case OPTION_NEGBIT:
 	{
-		intmax_t value = get_int_value(opt);
+		intmax_t value = get_int_value(opt, flags);
 		if (unset)
 			value |= opt->defval;
 		else
@@ -157,7 +168,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 
 	case OPTION_BITOP:
 	{
-		intmax_t value = get_int_value(opt);
+		intmax_t value = get_int_value(opt, flags);
 		if (unset)
 			BUG("BITOP can't have unset form");
 		value &= ~opt->extra;
@@ -169,7 +180,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 	{
 		size_t bits = CHAR_BIT * opt->precision;
 		intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits);
-		intmax_t value = get_int_value(opt);
+		intmax_t value = get_int_value(opt, flags);
 
 		if (value < 0)
 			value = 0;
@@ -318,7 +329,9 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 
 struct parse_opt_cmdmode_list {
 	intmax_t value;
-	const struct option *opt, *reference_opt;
+	void *value_ptr;
+	size_t precision;
+	const struct option *opt;
 	const char *arg;
 	enum opt_parsed flags;
 	struct parse_opt_cmdmode_list *next;
@@ -331,21 +344,25 @@ static void build_cmdmode_list(struct parse_opt_ctx_t *ctx,
 
 	for (; opts->type != OPTION_END; opts++) {
 		struct parse_opt_cmdmode_list *elem = ctx->cmdmode_list;
+		void *value_ptr = opts->value;
 
-		if (!(opts->flags & PARSE_OPT_CMDMODE) || !opts->value)
+		if (!(opts->flags & PARSE_OPT_CMDMODE) || !value_ptr)
 			continue;
 
-		while (elem && elem->reference_opt->value != opts->value)
+		while (elem && elem->value_ptr != value_ptr)
 			elem = elem->next;
 		if (elem)
 			continue;
 
 		CALLOC_ARRAY(elem, 1);
-		elem->reference_opt = opts;
-		elem->value = get_int_value(opts);
+		elem->value_ptr = value_ptr;
+		elem->precision = opts->precision;
+		if (do_get_int_value(value_ptr, opts->precision, &elem->value))
+			optbug(opts, "has invalid precision");
 		elem->next = ctx->cmdmode_list;
 		ctx->cmdmode_list = elem;
 	}
+	BUG_if_bug("invalid 'struct option'");
 }
 
 static char *optnamearg(const struct option *opt, const char *arg,
@@ -367,7 +384,11 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
 	char *opt_name, *other_opt_name;
 
 	for (; elem; elem = elem->next) {
-		intmax_t new_value = get_int_value(elem->reference_opt);
+		intmax_t new_value;
+
+		if (do_get_int_value(elem->value_ptr, elem->precision,
+				     &new_value))
+			BUG("impossible: invalid precision");
 
 		if (new_value == elem->value)
 			continue;
-- 
2.50.0
René Scharfe· Jul 9, 2025, 09:44 UTC · re: René Scharfe · lore

[PATCH v2 1/7] parse-options: require PARSE_OPT_NOARG for OPTION_BITOP

OPTION_BITOP options don't take arguments. Make sure they are declared that way using the flag PARSE_OPT_NOARG.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 parse-options.c | 1 +
 1 file changed, 1 insertion(+)
Show changes to parse-options.c +1 −0
diff --git a/parse-options.c b/parse-options.c
index a9a39ecaef..68ff494492 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -591,6 +591,7 @@ static void parse_options_check(const struct option *opts)
 		case OPTION_NEGBIT:
 		case OPTION_SET_INT:
 		case OPTION_NUMBER:
+		case OPTION_BITOP:
 			if ((opts->flags & PARSE_OPT_OPTARG) ||
 			    !(opts->flags & PARSE_OPT_NOARG))
 				optbug(opts, "should not accept an argument");
-- 
2.50.0
Patrick Steinhardt· Jul 9, 2025, 13:59 UTC · re: René Scharfe · lore

Re: [PATCH v2 1/7] parse-options: require PARSE_OPT_NOARG for OPTION_BITOP

On Wed, Jul 09, 2025 at 11:44:09AM +0200, René Scharfe wrote:
> OPTION_BITOP options don't take arguments.  Make sure they are declared
> that way using the flag PARSE_OPT_NOARG.
> 
> Signed-off-by: René Scharfe <l.s.r@web.de>
Nice, splitting out this commit makes it way easier to understand.
Patrick
René Scharfe· Jul 9, 2025, 09:45 UTC · re: René Scharfe · lore

[PATCH v2 2/7] parse-options: add precision handling for PARSE_OPT_CMDMODE

Build on 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) to support value variables of different sizes for PARSE_OPT_CMDMODE options. Do that by requiring their "precision" to be set and casting their "value" pointer accordingly.

Call the function that does the raw casting do_get_int_value() to reserve the name get_int_value() for a more friendly wrapper we're going to introduce in one of the next patches.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 builtin/am.c                  |  1 +
 parse-options.c               | 41 ++++++++++++++++++++++++++++++-----
 parse-options.h               |  1 +
 t/helper/test-parse-options.c | 13 ++++++++---
 4 files changed, 48 insertions(+), 8 deletions(-)
Show changes to 4 files +48 −8

builtin/am.c, parse-options.c, parse-options.h, t/helper/test-parse-options.c

diff --git a/builtin/am.c b/builtin/am.c
index a800003340..c9d925f7b9 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2406,6 +2406,7 @@ int cmd_am(int argc,
 			.type = OPTION_CALLBACK,
 			.long_name = "show-current-patch",
 			.value = &resume_mode,
+			.precision = sizeof(resume_mode),
 			.argh = "(diff|raw)",
 			.help = N_("show the patch being applied"),
 			.flags = PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
diff --git a/parse-options.c b/parse-options.c
index 68ff494492..ddac008a5e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -68,6 +68,26 @@ static char *fix_filename(const char *prefix, const char *file)
 		return prefix_filename_except_for_dash(prefix, file);
 }
 
+static int do_get_int_value(const void *value, size_t precision, intmax_t *ret)
+{
+	switch (precision) {
+	case sizeof(int8_t):
+		*ret = *(int8_t *)value;
+		return 0;
+	case sizeof(int16_t):
+		*ret = *(int16_t *)value;
+		return 0;
+	case sizeof(int32_t):
+		*ret = *(int32_t *)value;
+		return 0;
+	case sizeof(int64_t):
+		*ret = *(int64_t *)value;
+		return 0;
+	default:
+		return -1;
+	}
+}
+
 static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 					  const struct option *opt,
 					  enum opt_parsed flags,
@@ -266,7 +286,9 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 }
 
 struct parse_opt_cmdmode_list {
-	int value, *value_ptr;
+	intmax_t value;
+	void *value_ptr;
+	size_t precision;
 	const struct option *opt;
 	const char *arg;
 	enum opt_parsed flags;
@@ -280,7 +302,7 @@ static void build_cmdmode_list(struct parse_opt_ctx_t *ctx,
 
 	for (; opts->type != OPTION_END; opts++) {
 		struct parse_opt_cmdmode_list *elem = ctx->cmdmode_list;
-		int *value_ptr = opts->value;
+		void *value_ptr = opts->value;
 
 		if (!(opts->flags & PARSE_OPT_CMDMODE) || !value_ptr)
 			continue;
@@ -292,10 +314,13 @@ static void build_cmdmode_list(struct parse_opt_ctx_t *ctx,
 
 		CALLOC_ARRAY(elem, 1);
 		elem->value_ptr = value_ptr;
-		elem->value = *value_ptr;
+		elem->precision = opts->precision;
+		if (do_get_int_value(value_ptr, opts->precision, &elem->value))
+			optbug(opts, "has invalid precision");
 		elem->next = ctx->cmdmode_list;
 		ctx->cmdmode_list = elem;
 	}
+	BUG_if_bug("invalid 'struct option'");
 }
 
 static char *optnamearg(const struct option *opt, const char *arg,
@@ -317,7 +342,13 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
 	char *opt_name, *other_opt_name;
 
 	for (; elem; elem = elem->next) {
-		if (*elem->value_ptr == elem->value)
+		intmax_t new_value;
+
+		if (do_get_int_value(elem->value_ptr, elem->precision,
+				     &new_value))
+			BUG("impossible: invalid precision");
+
+		if (new_value == elem->value)
 			continue;
 
 		if (elem->opt &&
@@ -327,7 +358,7 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
 		elem->opt = opt;
 		elem->arg = arg;
 		elem->flags = flags;
-		elem->value = *elem->value_ptr;
+		elem->value = new_value;
 	}
 
 	if (result || !elem)
diff --git a/parse-options.h b/parse-options.h
index 91c3e3c29b..c75a473c9e 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -269,6 +269,7 @@ struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.help = (h), \
 	.flags = PARSE_OPT_CMDMODE|PARSE_OPT_NOARG|PARSE_OPT_NONEG | (f), \
 	.defval = (i), \
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index f2663dd0c0..1e03ff88f6 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -148,9 +148,16 @@ int cmd__parse_options(int argc, const char **argv)
 		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),
-		OPT_CALLBACK_F(0, "mode34", &integer, "(3|4)",
-			"set integer to 3 or 4 (cmdmode option)",
-			PARSE_OPT_CMDMODE, mode34_callback),
+		{
+			.type = OPTION_CALLBACK,
+			.long_name = "mode34",
+			.value = &integer,
+			.precision = sizeof(integer),
+			.argh = "(3|4)",
+			.help = "set integer to 3 or 4 (cmdmode option)",
+			.flags = PARSE_OPT_CMDMODE,
+			.callback = mode34_callback,
+		},
 		OPT_CALLBACK('L', "length", &integer, "str",
 			"get length of <str>", length_callback),
 		OPT_FILENAME('F', "file", &file, "set file to <file>"),
-- 
2.50.0
Patrick Steinhardt· Jul 9, 2025, 13:58 UTC · re: René Scharfe · lore

Re: [PATCH v2 2/7] parse-options: add precision handling for PARSE_OPT_CMDMODE

On Wed, Jul 09, 2025 at 11:45:14AM +0200, René Scharfe wrote:
Show 38 quoted lines
> Build on 09705696f7 (parse-options: introduce precision handling for
> `OPTION_INTEGER`, 2025-04-17) to support value variables of different
> sizes for PARSE_OPT_CMDMODE options.  Do that by requiring their
> "precision" to be set and casting their "value" pointer accordingly.
> 
> Call the function that does the raw casting do_get_int_value() to
> reserve the name get_int_value() for a more friendly wrapper we're
> going to introduce in one of the next patches.
> 
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  builtin/am.c                  |  1 +
>  parse-options.c               | 41 ++++++++++++++++++++++++++++++-----
>  parse-options.h               |  1 +
>  t/helper/test-parse-options.c | 13 ++++++++---
>  4 files changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/builtin/am.c b/builtin/am.c
> index a800003340..c9d925f7b9 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -2406,6 +2406,7 @@ int cmd_am(int argc,
>  			.type = OPTION_CALLBACK,
>  			.long_name = "show-current-patch",
>  			.value = &resume_mode,
> +			.precision = sizeof(resume_mode),
>  			.argh = "(diff|raw)",
>  			.help = N_("show the patch being applied"),
>  			.flags = PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
> diff --git a/parse-options.c b/parse-options.c
> index 68ff494492..ddac008a5e 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -68,6 +68,26 @@ static char *fix_filename(const char *prefix, const char *file)
>  		return prefix_filename_except_for_dash(prefix, file);
>  }
>  
> +static int do_get_int_value(const void *value, size_t precision, intmax_t *ret)
Nit: after the fourth patch we have `do_get_int_value()` and
`get_int_value()`, where the major difference is that the latter dies if
we failed to parse the value. It might be easier to discern which is
which if we called them `get_int_value()` and `get_int_value_or_die()`.
Patrick
René Scharfe· Jul 9, 2025, 15:05 UTC · re: Patrick Steinhardt · lore

Re: [PATCH v2 2/7] parse-options: add precision handling for PARSE_OPT_CMDMODE

On 7/9/25 3:58 PM, Patrick Steinhardt wrote:
Show 5 quoted lines
> On Wed, Jul 09, 2025 at 11:45:14AM +0200, René Scharfe wrote:
>>
>> Call the function that does the raw casting do_get_int_value() to
>> reserve the name get_int_value() for a more friendly wrapper we're
>> going to introduce in one of the next patches.
Show 14 quoted lines
>> diff --git a/parse-options.c b/parse-options.c
>> index 68ff494492..ddac008a5e 100644
>> --- a/parse-options.c
>> +++ b/parse-options.c
>> @@ -68,6 +68,26 @@ static char *fix_filename(const char *prefix, const char *file)
>>  		return prefix_filename_except_for_dash(prefix, file);
>>  }
>>  
>> +static int do_get_int_value(const void *value, size_t precision, intmax_t *ret)
> 
> Nit: after the fourth patch we have `do_get_int_value()` and
> `get_int_value()`, where the major difference is that the latter dies if
> we failed to parse the value. It might be easier to discern which is
> which if we called them `get_int_value()` and `get_int_value_or_die()`.

That would be misleading because get_int_value() doesn't die() like a function from write-or-die.c, it BUGs instead. I don't think it makes sense to advertise the presence of assertions in a function's name. But we do have a tradition of using a prefix of "do_" with wrapped functions that have a more raw interface and do the actual work.

Nit: They don't parse, but cast a void pointer to the appropriate type
and dereference it.
René
Patrick Steinhardt· Jul 9, 2025, 15:58 UTC · re: René Scharfe · lore

Re: [PATCH v2 2/7] parse-options: add precision handling for PARSE_OPT_CMDMODE

On Wed, Jul 09, 2025 at 05:05:35PM +0200, René Scharfe wrote:
Show 30 quoted lines
> On 7/9/25 3:58 PM, Patrick Steinhardt wrote:
> > On Wed, Jul 09, 2025 at 11:45:14AM +0200, René Scharfe wrote:
> >>
> >> Call the function that does the raw casting do_get_int_value() to
> >> reserve the name get_int_value() for a more friendly wrapper we're
> >> going to introduce in one of the next patches.
> 
> >> diff --git a/parse-options.c b/parse-options.c
> >> index 68ff494492..ddac008a5e 100644
> >> --- a/parse-options.c
> >> +++ b/parse-options.c
> >> @@ -68,6 +68,26 @@ static char *fix_filename(const char *prefix, const char *file)
> >>  		return prefix_filename_except_for_dash(prefix, file);
> >>  }
> >>  
> >> +static int do_get_int_value(const void *value, size_t precision, intmax_t *ret)
> > 
> > Nit: after the fourth patch we have `do_get_int_value()` and
> > `get_int_value()`, where the major difference is that the latter dies if
> > we failed to parse the value. It might be easier to discern which is
> > which if we called them `get_int_value()` and `get_int_value_or_die()`.
> 
> That would be misleading because get_int_value() doesn't die() like a
> function from write-or-die.c, it BUGs instead.  I don't think it makes
> sense to advertise the presence of assertions in a function's name.
> But we do have a tradition of using a prefix of "do_" with wrapped
> functions that have a more raw interface and do the actual work.
> 
> Nit: They don't parse, but cast a void pointer to the appropriate type
> and dereference it.
Fair enough. I don't mind it much either way, thanks!
Patrick
Junio C Hamano· Jul 9, 2025, 15:56 UTC · re: Patrick Steinhardt · lore

Re: [PATCH v2 2/7] parse-options: add precision handling for PARSE_OPT_CMDMODE

Patrick Steinhardt <ps@pks.im> writes:
Show 26 quoted lines
>> diff --git a/builtin/am.c b/builtin/am.c
>> index a800003340..c9d925f7b9 100644
>> --- a/builtin/am.c
>> +++ b/builtin/am.c
>> @@ -2406,6 +2406,7 @@ int cmd_am(int argc,
>>  			.type = OPTION_CALLBACK,
>>  			.long_name = "show-current-patch",
>>  			.value = &resume_mode,
>> +			.precision = sizeof(resume_mode),
>>  			.argh = "(diff|raw)",
>>  			.help = N_("show the patch being applied"),
>>  			.flags = PARSE_OPT_CMDMODE | PARSE_OPT_OPTARG | PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
>> diff --git a/parse-options.c b/parse-options.c
>> index 68ff494492..ddac008a5e 100644
>> --- a/parse-options.c
>> +++ b/parse-options.c
>> @@ -68,6 +68,26 @@ static char *fix_filename(const char *prefix, const char *file)
>>  		return prefix_filename_except_for_dash(prefix, file);
>>  }
>>  
>> +static int do_get_int_value(const void *value, size_t precision, intmax_t *ret)
>
> Nit: after the fourth patch we have `do_get_int_value()` and
> `get_int_value()`, where the major difference is that the latter dies if
> we failed to parse the value. It might be easier to discern which is
> which if we called them `get_int_value()` and `get_int_value_or_die()`.

Seeing the symmetry between set_int_value() and do_get_int_value(), I tend to agree that it would be easier to remember what the latter does if it were named get_int_value().

I am not so sure about _or_die(), though. The only kind of error the current get_int_value() detects and acts on is the mismatched precision bug, which is a programmer error; there is no "die" but we call "BUG".

get_int_value_with_precision_check() is quite a mouthful, but it is known that I am bad at naming X-<, so I dunno.

René Scharfe· Jul 9, 2025, 09:45 UTC · re: René Scharfe · lore

[PATCH v2 3/7] parse-options: add precision handling for OPTION_SET_INT

Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_SET_INT. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits.

Factor out the casting code from the part of do_get_value() that handles OPTION_INTEGER to avoid code duplication. We're going to use it in the next patches as well.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 builtin/update-index.c        |  6 ++++
 parse-options.c               | 56 ++++++++++++++++++++++-------------
 parse-options.h               |  2 ++
 t/helper/test-parse-options.c |  1 +
 4 files changed, 45 insertions(+), 20 deletions(-)
Show changes to 4 files +45 −20

builtin/update-index.c, parse-options.c, parse-options.h, t/helper/test-parse-options.c

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 538b619ba4..0c1d4ed55b 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -981,6 +981,7 @@ int cmd_update_index(int argc,
 			.type = OPTION_SET_INT,
 			.long_name = "assume-unchanged",
 			.value = &mark_valid_only,
+			.precision = sizeof(mark_valid_only),
 			.help = N_("mark files as \"not changing\""),
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			.defval = MARK_FLAG,
@@ -989,6 +990,7 @@ int cmd_update_index(int argc,
 			.type = OPTION_SET_INT,
 			.long_name = "no-assume-unchanged",
 			.value = &mark_valid_only,
+			.precision = sizeof(mark_valid_only),
 			.help = N_("clear assumed-unchanged bit"),
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			.defval = UNMARK_FLAG,
@@ -997,6 +999,7 @@ int cmd_update_index(int argc,
 			.type = OPTION_SET_INT,
 			.long_name = "skip-worktree",
 			.value = &mark_skip_worktree_only,
+			.precision = sizeof(mark_skip_worktree_only),
 			.help = N_("mark files as \"index-only\""),
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			.defval = MARK_FLAG,
@@ -1005,6 +1008,7 @@ int cmd_update_index(int argc,
 			.type = OPTION_SET_INT,
 			.long_name = "no-skip-worktree",
 			.value = &mark_skip_worktree_only,
+			.precision = sizeof(mark_skip_worktree_only),
 			.help = N_("clear skip-worktree bit"),
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			.defval = UNMARK_FLAG,
@@ -1079,6 +1083,7 @@ int cmd_update_index(int argc,
 			.type = OPTION_SET_INT,
 			.long_name = "fsmonitor-valid",
 			.value = &mark_fsmonitor_only,
+			.precision = sizeof(mark_fsmonitor_only),
 			.help = N_("mark files as fsmonitor valid"),
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			.defval = MARK_FLAG,
@@ -1087,6 +1092,7 @@ int cmd_update_index(int argc,
 			.type = OPTION_SET_INT,
 			.long_name = "no-fsmonitor-valid",
 			.value = &mark_fsmonitor_only,
+			.precision = sizeof(mark_fsmonitor_only),
 			.help = N_("clear fsmonitor valid bit"),
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			.defval = UNMARK_FLAG,
diff --git a/parse-options.c b/parse-options.c
index ddac008a5e..639f41b83b 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -88,6 +88,36 @@ static int do_get_int_value(const void *value, size_t precision, intmax_t *ret)
 	}
 }
 
+static enum parse_opt_result set_int_value(const struct option *opt,
+					   enum opt_parsed flags,
+					   intmax_t value)
+{
+	switch (opt->precision) {
+	case sizeof(int8_t):
+		*(int8_t *)opt->value = value;
+		return 0;
+	case sizeof(int16_t):
+		*(int16_t *)opt->value = value;
+		return 0;
+	case sizeof(int32_t):
+		*(int32_t *)opt->value = value;
+		return 0;
+	case sizeof(int64_t):
+		*(int64_t *)opt->value = value;
+		return 0;
+	default:
+		BUG("invalid precision for option %s", optname(opt, flags));
+	}
+}
+
+static int signed_int_fits(intmax_t value, size_t precision)
+{
+	size_t bits = precision * CHAR_BIT;
+	intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits);
+	intmax_t lower_bound = -upper_bound - 1;
+	return lower_bound <= value && value <= upper_bound;
+}
+
 static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 					  const struct option *opt,
 					  enum opt_parsed flags,
@@ -136,8 +166,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 		return 0;
 
 	case OPTION_SET_INT:
-		*(int *)opt->value = unset ? 0 : opt->defval;
-		return 0;
+		return set_int_value(opt, flags, unset ? 0 : opt->defval);
 
 	case OPTION_STRING:
 		if (unset)
@@ -219,23 +248,7 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 			return error(_("value %s for %s not in range [%"PRIdMAX",%"PRIdMAX"]"),
 				     arg, optname(opt, flags), (intmax_t)lower_bound, (intmax_t)upper_bound);
 
-		switch (opt->precision) {
-		case 1:
-			*(int8_t *)opt->value = value;
-			return 0;
-		case 2:
-			*(int16_t *)opt->value = value;
-			return 0;
-		case 4:
-			*(int32_t *)opt->value = value;
-			return 0;
-		case 8:
-			*(int64_t *)opt->value = value;
-			return 0;
-		default:
-			BUG("invalid precision for option %s",
-			    optname(opt, flags));
-		}
+		return set_int_value(opt, flags, value);
 	}
 	case OPTION_UNSIGNED:
 	{
@@ -617,10 +630,13 @@ static void parse_options_check(const struct option *opts)
 		    opts->long_name && !(opts->flags & PARSE_OPT_NONEG))
 			optbug(opts, "OPTION_SET_INT 0 should not be negatable");
 		switch (opts->type) {
+		case OPTION_SET_INT:
+			if (!signed_int_fits(opts->defval, opts->precision))
+				optbug(opts, "has invalid defval");
+			/* fallthru */
 		case OPTION_COUNTUP:
 		case OPTION_BIT:
 		case OPTION_NEGBIT:
-		case OPTION_SET_INT:
 		case OPTION_NUMBER:
 		case OPTION_BITOP:
 			if ((opts->flags & PARSE_OPT_OPTARG) ||
diff --git a/parse-options.h b/parse-options.h
index c75a473c9e..71516e4b5b 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -190,6 +190,7 @@ struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.help = (h), \
 	.flags = PARSE_OPT_NOARG | (f), \
 	.defval = (i), \
@@ -260,6 +261,7 @@ struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.help = (h), \
 	.flags = PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, \
 	.defval = 1, \
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index 1e03ff88f6..2ba2546d70 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -131,6 +131,7 @@ int cmd__parse_options(int argc, const char **argv)
 			.short_name = 'B',
 			.long_name = "no-fear",
 			.value = &boolean,
+			.precision = sizeof(boolean),
 			.help = "be brave",
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			.defval = 1,
-- 
2.50.0
René Scharfe· Jul 9, 2025, 09:45 UTC · re: René Scharfe · lore

[PATCH v2 4/7] parse-options: add precision handling for OPTION_BIT

Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_BIT. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 builtin/write-tree.c |  1 +
 parse-options.c      | 19 +++++++++++++++----
 parse-options.h      |  1 +
 3 files changed, 17 insertions(+), 4 deletions(-)
Show changes to 3 files +17 −4

builtin/write-tree.c, parse-options.c, parse-options.h

diff --git a/builtin/write-tree.c b/builtin/write-tree.c
index 5a8dc377ec..cfec044710 100644
--- a/builtin/write-tree.c
+++ b/builtin/write-tree.c
@@ -35,6 +35,7 @@ int cmd_write_tree(int argc,
 			.type = OPTION_BIT,
 			.long_name = "ignore-cache-tree",
 			.value = &flags,
+			.precision = sizeof(flags),
 			.help = N_("only useful for debugging"),
 			.flags = PARSE_OPT_HIDDEN | PARSE_OPT_NOARG,
 			.defval = WRITE_TREE_IGNORE_CACHE_TREE,
diff --git a/parse-options.c b/parse-options.c
index 639f41b83b..b5c877d5e1 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -88,6 +88,14 @@ static int do_get_int_value(const void *value, size_t precision, intmax_t *ret)
 	}
 }
 
+static intmax_t get_int_value(const struct option *opt, enum opt_parsed flags)
+{
+	intmax_t ret;
+	if (do_get_int_value(opt->value, opt->precision, &ret))
+		BUG("invalid precision for option %s", optname(opt, flags));
+	return ret;
+}
+
 static enum parse_opt_result set_int_value(const struct option *opt,
 					   enum opt_parsed flags,
 					   intmax_t value)
@@ -139,11 +147,14 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 		return opt->ll_callback(p, opt, NULL, unset);
 
 	case OPTION_BIT:
+	{
+		intmax_t value = get_int_value(opt, flags);
 		if (unset)
-			*(int *)opt->value &= ~opt->defval;
+			value &= ~opt->defval;
 		else
-			*(int *)opt->value |= opt->defval;
-		return 0;
+			value |= opt->defval;
+		return set_int_value(opt, flags, value);
+	}
 
 	case OPTION_NEGBIT:
 		if (unset)
@@ -631,11 +642,11 @@ static void parse_options_check(const struct option *opts)
 			optbug(opts, "OPTION_SET_INT 0 should not be negatable");
 		switch (opts->type) {
 		case OPTION_SET_INT:
+		case OPTION_BIT:
 			if (!signed_int_fits(opts->defval, opts->precision))
 				optbug(opts, "has invalid defval");
 			/* fallthru */
 		case OPTION_COUNTUP:
-		case OPTION_BIT:
 		case OPTION_NEGBIT:
 		case OPTION_NUMBER:
 		case OPTION_BITOP:
diff --git a/parse-options.h b/parse-options.h
index 71516e4b5b..6501ca3c27 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -172,6 +172,7 @@ struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.help = (h), \
 	.flags = PARSE_OPT_NOARG|(f), \
 	.callback = NULL, \
-- 
2.50.0
René Scharfe· Jul 9, 2025, 09:45 UTC · re: René Scharfe · lore

[PATCH v2 5/7] parse-options: add precision handling for OPTION_NEGBIT

Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_NEGBIT. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 builtin/rebase.c |  1 +
 parse-options.c  | 11 +++++++----
 parse-options.h  |  1 +
 3 files changed, 9 insertions(+), 4 deletions(-)
Show changes to 3 files +9 −4

builtin/rebase.c, parse-options.c, parse-options.h

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 2e8c4ee678..e90562a3b8 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1128,6 +1128,7 @@ int cmd_rebase(int argc,
 			.short_name = 'n',
 			.long_name = "no-stat",
 			.value = &options.flags,
+			.precision = sizeof(options.flags),
 			.help = N_("do not show diffstat of what changed upstream"),
 			.flags = PARSE_OPT_NOARG,
 			.defval = REBASE_DIFFSTAT,
diff --git a/parse-options.c b/parse-options.c
index b5c877d5e1..ba89dc4d09 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -157,11 +157,14 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 	}
 
 	case OPTION_NEGBIT:
+	{
+		intmax_t value = get_int_value(opt, flags);
 		if (unset)
-			*(int *)opt->value |= opt->defval;
+			value |= opt->defval;
 		else
-			*(int *)opt->value &= ~opt->defval;
-		return 0;
+			value &= ~opt->defval;
+		return set_int_value(opt, flags, value);
+	}
 
 	case OPTION_BITOP:
 		if (unset)
@@ -643,11 +646,11 @@ static void parse_options_check(const struct option *opts)
 		switch (opts->type) {
 		case OPTION_SET_INT:
 		case OPTION_BIT:
+		case OPTION_NEGBIT:
 			if (!signed_int_fits(opts->defval, opts->precision))
 				optbug(opts, "has invalid defval");
 			/* fallthru */
 		case OPTION_COUNTUP:
-		case OPTION_NEGBIT:
 		case OPTION_NUMBER:
 		case OPTION_BITOP:
 			if ((opts->flags & PARSE_OPT_OPTARG) ||
diff --git a/parse-options.h b/parse-options.h
index 6501ca3c27..076f88b384 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -250,6 +250,7 @@ struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.help = (h), \
 	.flags = PARSE_OPT_NOARG, \
 	.defval = (b), \
-- 
2.50.0
René Scharfe· Jul 9, 2025, 09:46 UTC · re: René Scharfe · lore

[PATCH v2 6/7] parse-options: add precision handling for OPTION_BITOP

Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_BITOP. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits.

Check if "devfal" fits into an integer variable with the given "precision", but don't check "extra", as its value is only used to clear bits, so cannot lead to an overflow. Not checking continues to allow e.g., using -1 to clear all bits even if the value variable has a narrower type than intptr_t.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 parse-options.c | 11 +++++++----
 parse-options.h |  1 +
 2 files changed, 8 insertions(+), 4 deletions(-)
Show changes to 2 files +8 −4

parse-options.c, parse-options.h

diff --git a/parse-options.c b/parse-options.c
index ba89dc4d09..a813511b1b 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -167,11 +167,14 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 	}
 
 	case OPTION_BITOP:
+	{
+		intmax_t value = get_int_value(opt, flags);
 		if (unset)
 			BUG("BITOP can't have unset form");
-		*(int *)opt->value &= ~opt->extra;
-		*(int *)opt->value |= opt->defval;
-		return 0;
+		value &= ~opt->extra;
+		value |= opt->defval;
+		return set_int_value(opt, flags, value);
+	}
 
 	case OPTION_COUNTUP:
 		if (*(int *)opt->value < 0)
@@ -647,12 +650,12 @@ static void parse_options_check(const struct option *opts)
 		case OPTION_SET_INT:
 		case OPTION_BIT:
 		case OPTION_NEGBIT:
+		case OPTION_BITOP:
 			if (!signed_int_fits(opts->defval, opts->precision))
 				optbug(opts, "has invalid defval");
 			/* fallthru */
 		case OPTION_COUNTUP:
 		case OPTION_NUMBER:
-		case OPTION_BITOP:
 			if ((opts->flags & PARSE_OPT_OPTARG) ||
 			    !(opts->flags & PARSE_OPT_NOARG))
 				optbug(opts, "should not accept an argument");
diff --git a/parse-options.h b/parse-options.h
index 076f88b384..8bdf469ae9 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -240,6 +240,7 @@ struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.help = (h), \
 	.flags = PARSE_OPT_NOARG|PARSE_OPT_NONEG, \
 	.defval = (set), \
-- 
2.50.0
René Scharfe· Jul 9, 2025, 09:46 UTC · re: René Scharfe · lore

[PATCH v2 7/7] parse-options: add precision handling for OPTION_COUNTUP

Similar to 09705696f7 (parse-options: introduce precision handling for `OPTION_INTEGER`, 2025-04-17) support value variables of different sizes for OPTION_COUNTUP. Do that by requiring their "precision" to be set, casting their "value" pointer accordingly and checking whether the value fits.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 parse-options.c               | 22 +++++++++++++++++-----
 parse-options.h               |  1 +
 t/helper/test-parse-options.c |  3 +++
 3 files changed, 21 insertions(+), 5 deletions(-)
Show changes to 3 files +21 −5

parse-options.c, parse-options.h, t/helper/test-parse-options.c

diff --git a/parse-options.c b/parse-options.c
index a813511b1b..5224203ffe 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -177,10 +177,22 @@ static enum parse_opt_result do_get_value(struct parse_opt_ctx_t *p,
 	}
 
 	case OPTION_COUNTUP:
-		if (*(int *)opt->value < 0)
-			*(int *)opt->value = 0;
-		*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
-		return 0;
+	{
+		size_t bits = CHAR_BIT * opt->precision;
+		intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - bits);
+		intmax_t value = get_int_value(opt, flags);
+
+		if (value < 0)
+			value = 0;
+		if (unset)
+			value = 0;
+		else if (value < upper_bound)
+			value++;
+		else
+			return error(_("value for %s exceeds %"PRIdMAX),
+				     optname(opt, flags), upper_bound);
+		return set_int_value(opt, flags, value);
+	}
 
 	case OPTION_SET_INT:
 		return set_int_value(opt, flags, unset ? 0 : opt->defval);
@@ -651,10 +663,10 @@ static void parse_options_check(const struct option *opts)
 		case OPTION_BIT:
 		case OPTION_NEGBIT:
 		case OPTION_BITOP:
+		case OPTION_COUNTUP:
 			if (!signed_int_fits(opts->defval, opts->precision))
 				optbug(opts, "has invalid defval");
 			/* fallthru */
-		case OPTION_COUNTUP:
 		case OPTION_NUMBER:
 			if ((opts->flags & PARSE_OPT_OPTARG) ||
 			    !(opts->flags & PARSE_OPT_NOARG))
diff --git a/parse-options.h b/parse-options.h
index 8bdf469ae9..312045604d 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -183,6 +183,7 @@ struct option {
 	.short_name = (s), \
 	.long_name = (l), \
 	.value = (v), \
+	.precision = sizeof(*v), \
 	.help = (h), \
 	.flags = PARSE_OPT_NOARG|(f), \
 }
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index 2ba2546d70..68579d83f3 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -178,6 +178,7 @@ int cmd__parse_options(int argc, const char **argv)
 			.type = OPTION_COUNTUP,
 			.short_name = '+',
 			.value = &boolean,
+			.precision = sizeof(boolean),
 			.help = "same as -b",
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH,
 		},
@@ -185,6 +186,7 @@ int cmd__parse_options(int argc, const char **argv)
 			.type = OPTION_COUNTUP,
 			.long_name = "ambiguous",
 			.value = &ambiguous,
+			.precision = sizeof(ambiguous),
 			.help = "positive ambiguity",
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 		},
@@ -192,6 +194,7 @@ int cmd__parse_options(int argc, const char **argv)
 			.type = OPTION_COUNTUP,
 			.long_name = "no-ambiguous",
 			.value = &ambiguous,
+			.precision = sizeof(ambiguous),
 			.help = "negative ambiguity",
 			.flags = PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 		},
-- 
2.50.0

← back to recent threads