[PATCH v2 0/5] parse-options: harden handling of integer values
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Apr 15, 2025, 12:14 UTC
- Message-ID
- <20250415-b4-pks-parse-options-integers-v2-0-ce07441a1f01@pks.im>
- In-Reply-To
- <20250401-b4-pks-parse-options-integers-v1-0-a628ad40c3b4@pks.im>
Hi,
this patch series addresses the issues raised in [1] and [2]. As discussed in [1], the series also introduces a couple of safeguards to make it harder to misuse `OPT_INTEGER()` and `OPT_MAGNITUDE()`:
- We now track the precision of the underlying integer types. This
makes it possible to pass arbitrarily-sized integers to those
options, not only `int` and `unsigned long`, respectively. - We introduce a build assert to verify that the passed variable has
correct signedness.Furthermore, the series introduces `OPT_UNSIGNED()` to adapt all callsites that previously used variables with the wrong signedness.
Changes in v2:
- Adapt computation of upper bounds to use similar logic to
`maximum_signed_value_of_type()`.
- Link to v1: https://lore.kernel.org/r/20250401-b4-pks-parse-options-integers-v1-0-a628ad40c3b4@pks.imThanks!
Patrick
[1]: <89257ab82cd60d135cce02d51eacee7ec35c1c37.camel@physik.fu-berlin.de> [2]: <Z8HW6petWuMRWSXf@teonanacatl.net>
---
Patrick Steinhardt (5):
global: use designated initializers for options
parse-options: introduce precision handling for `OPTION_INTEGER`
parse-options: introduce precision handling for `OPTION_MAGNITUDE`
parse-options: introduce `OPTION_UNSIGNED`
parse-options: detect mismatches in integer signednessapply.c | 4 +- archive.c | 35 ++++++++--- builtin/am.c | 28 ++++++--- builtin/backfill.c | 4 +- builtin/clone.c | 13 ++++- builtin/column.c | 2 +- builtin/commit-tree.c | 12 +++- builtin/commit.c | 62 +++++++++++++++----- builtin/config.c | 13 ++++- builtin/describe.c | 24 ++++++-- builtin/fetch.c | 10 +++- builtin/fmt-merge-msg.c | 27 ++++++--- builtin/gc.c | 12 +++- builtin/grep.c | 18 ++++-- builtin/init-db.c | 13 +++-- builtin/ls-remote.c | 11 +++- builtin/merge.c | 38 +++++++++--- builtin/read-tree.c | 11 +++- builtin/rebase.c | 25 ++++++-- builtin/revert.c | 12 +++- builtin/show-branch.c | 13 ++++- builtin/tag.c | 24 ++++++-- builtin/update-index.c | 131 +++++++++++++++++++++++++++++------------- builtin/write-tree.c | 12 ++-- diff.c | 13 +++-- git-compat-util.h | 7 +++ parse-options.c | 131 +++++++++++++++++++++++++++++++++++------- parse-options.h | 23 +++++++- ref-filter.h | 15 +++-- t/helper/test-parse-options.c | 46 ++++++++++++--- t/t0040-parse-options.sh | 57 +++++++++++++++++- 31 files changed, 657 insertions(+), 189 deletions(-)
Range-diff versus v1:
1: 632c627b9ba = 1: 92d1cbd1153 global: use designated initializers for options
2: 65c450a4395 ! 2: 8d2400d7470 parse-options: introduce precision handling for `OPTION_INTEGER`
@@ parse-options.c: static enum parse_opt_result do_get_value(struct parse_opt_ctx_
}
case OPTION_INTEGER:
+ {
-+ intmax_t upper_bound = (((intmax_t) 1 << (opt->precision * 8 - 1)) - 1);
++ intmax_t upper_bound = INTMAX_MAX >> (bitsizeof(intmax_t) - CHAR_BIT * opt->precision);
+ intmax_t lower_bound = -upper_bound - 1;
+ intmax_t value;
+
3: 4946ca0f702 ! 3: a9b0d8c1127 parse-options: introduce precision handling for `OPTION_MAGNITUDE`
@@ parse-options.c: static enum parse_opt_result do_get_value(struct parse_opt_ctx_
}
case OPTION_MAGNITUDE:
+ {
-+ uintmax_t upper_bound = 0;
++ uintmax_t upper_bound = UINTMAX_MAX >> (bitsizeof(uintmax_t) - CHAR_BIT * opt->precision);
+ unsigned long value;
-+
-+ /*
-+ * It's stupid, but the obvious way of calculating the upper
-+ * bound via `2 ^ n - 1` overflows.
-+ */
-+ for (size_t i = 0; i < opt->precision * 8; i++)
-+ upper_bound |= ((uintmax_t) 1 << i);
+
if (unset) {
- *(unsigned long *)opt->value = 0;
4: 9728d57d5df ! 4: 743afbf539e parse-options: introduce `OPTION_UNSIGNED`
@@ parse-options.c: static enum parse_opt_result do_get_value(struct parse_opt_ctx_
}
+ case OPTION_UNSIGNED:
+ {
-+ uintmax_t upper_bound = 0;
++ uintmax_t upper_bound = UINTMAX_MAX >> (bitsizeof(uintmax_t) - CHAR_BIT * opt->precision);
+ uintmax_t value;
+
-+ /*
-+ * It's stupid, but the obvious way of calculating the upper
-+ * bound via `2 ^ n - 1` overflows.
-+ */
-+ for (size_t i = 0; i < opt->precision * 8; i++)
-+ upper_bound |= ((uintmax_t) 1 << i);
-+
+ if (unset) {
+ value = 0;
+ } else if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
@@ parse-options.c: static enum parse_opt_result do_get_value(struct parse_opt_ctx_
+ }
case OPTION_MAGNITUDE:
{
- uintmax_t upper_bound = 0;
+ uintmax_t upper_bound = UINTMAX_MAX >> (bitsizeof(uintmax_t) - CHAR_BIT * opt->precision);
## parse-options.h ##
@@ parse-options.h: enum parse_opt_type {
5: 59aacc72548 = 5: 1e42672a439 parse-options: detect mismatches in integer signedness--- base-commit: 5b97a56fa0e7d580dc8865b73107407c9b3f0eff change-id: 20250401-b4-pks-parse-options-integers-9b4bbcf21011