public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: "René Scharfe" <l.s.r@web•de>
To: Git List <git@vger•kernel.org>
Cc: Patrick Steinhardt <ps@pks•im>
Subject: [PATCH v2 0/7] parse-options: add more precision handling
Date: Wed, 9 Jul 2025 11:26:10 +0200	[thread overview]
Message-ID: <802eba72-c100-429a-80b7-7a0e8b6559ed@web.de> (raw)
In-Reply-To: <cf5cd57d-733f-4239-80f8-23bdc1523ab2@web.de>

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:
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

  parent reply	other threads:[~2025-07-09  9:31 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-29 11:43 [PATCH 0/6] parse-options: add more precision handling René Scharfe
2025-06-29 11:50 ` [PATCH 1/6] parse-options: add precision handling for PARSE_OPT_CMDMODE René Scharfe
2025-07-01 10:55   ` Patrick Steinhardt
2025-07-01 15:15     ` René Scharfe
2025-06-29 11:50 ` [PATCH 2/6] parse-options: add precision handling for OPTION_SET_INT René Scharfe
2025-07-01 10:55   ` Patrick Steinhardt
2025-07-01 15:54     ` René Scharfe
2025-07-02  2:31       ` Patrick Steinhardt
2025-06-29 11:50 ` [PATCH 3/6] parse-options: add precision handling for OPTION_BIT René Scharfe
2025-06-29 11:51 ` [PATCH 4/6] parse-options: add precision handling for OPTION_NEGBIT René Scharfe
2025-06-29 11:51 ` [PATCH 5/6] parse-options: add precision handling for OPTION_BITOP René Scharfe
2025-07-01 10:55   ` Patrick Steinhardt
2025-07-01 15:21     ` René Scharfe
2025-07-02  2:33       ` Patrick Steinhardt
2025-06-29 11:51 ` [PATCH 6/6] parse-options: add precision handling for OPTION_COUNTUP René Scharfe
2025-07-01 10:55   ` Patrick Steinhardt
2025-07-01 16:01     ` René Scharfe
2025-07-02  2:29       ` Patrick Steinhardt
2025-07-09  9:26 ` René Scharfe [this message]
2025-07-09  9:44   ` [PATCH v2 1/7] parse-options: require PARSE_OPT_NOARG for OPTION_BITOP René Scharfe
2025-07-09 13:59     ` Patrick Steinhardt
2025-07-09  9:45   ` [PATCH v2 2/7] parse-options: add precision handling for PARSE_OPT_CMDMODE René Scharfe
2025-07-09 13:58     ` Patrick Steinhardt
2025-07-09 15:05       ` René Scharfe
2025-07-09 15:58         ` Patrick Steinhardt
2025-07-09 15:56       ` Junio C Hamano
2025-07-09  9:45   ` [PATCH v2 3/7] parse-options: add precision handling for OPTION_SET_INT René Scharfe
2025-07-09  9:45   ` [PATCH v2 4/7] parse-options: add precision handling for OPTION_BIT René Scharfe
2025-07-09  9:45   ` [PATCH v2 5/7] parse-options: add precision handling for OPTION_NEGBIT René Scharfe
2025-07-09  9:46   ` [PATCH v2 6/7] parse-options: add precision handling for OPTION_BITOP René Scharfe
2025-07-09  9:46   ` [PATCH v2 7/7] parse-options: add precision handling for OPTION_COUNTUP René Scharfe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=802eba72-c100-429a-80b7-7a0e8b6559ed@web.de \
    --to=l.s.r@web$(echo .)de \
    --cc=git@vger$(echo .)kernel.org \
    --cc=ps@pks$(echo .)im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox