Re: [PATCH GSoC v15 02/13] git-compat-util: add `strtoumax_szt()` with error handling
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 1, 2026, 17:30 UTC
- Message-ID
- <xmqqse62obwh.fsf@gitster.g>
- In-Reply-To
- <20260701-ps-eric-work-rebase-v15-2-c88a43b63917@gmail.com>
Pablo Sabater <pabloosabaterr@gmail.com> writes:
Show 9 quoted lines
> From: Eric Ju <eric.peijian@gmail.com> > > We already have `strtoul_ui()` and similar functions that provide proper > error handling using `strtoul` from the standard library. However, > there isn't currently a variant that returns a `size_t`. > > Using `strtoul` is unreliable because `size_`t is platform-dependent, > `unsigned long` could be too big to fit into a `size_t` or too small to > hold a `size_t`.
It is somehow annoying to see that the commit log desciption, which is *clearly* meant to be plaintext, is so heavily riddled with backquoted references to code/program symbols. Yes, `literal` is a correct way to format them in both AsciiDoc and Markdown, so we very much welcome them in our documentation, but not in proposed log messages.
In any case, you dropped 't' in 'size_t' outside the pair of backquotes.
> Use `strtoumax` which returns a `uintmax_t` guaranteed to be at least as
`strtoumax()`, as the convention you used above for strtoul_ui() is to suffix function names with ().
> large as `size_t`, add a range check against `SIZE_MAX` to prevent > `size_t` overflow.
OK.
Show 31 quoted lines
> This variant is needed in a subsequent commit to enable returning a
> `size_t` with proper error handling.
>
> Mentored-by: Karthik Nayak <karthik.188@gmail.com>
> Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> git-compat-util.h | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 8809776407..5ecce5bbd2 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -975,6 +975,26 @@ static inline int strtoul_ui(char const *s, int base, unsigned int *result)
> return 0;
> }
>
> +/*
> + * Convert a string to a size_t using the standard library's strtoumax, with
> + * additional error handling to ensure robustness.
> + */
> +static inline int strtoumax_szt(char const *s, int base, size_t *result)
> +{
> + uintmax_t uim;
> + char *p;
> +
> + errno = 0;
> + /* negative values would be accepted by strtoul */
> + if (strchr(s, '-'))
> + return -1;Hmph, wouldn't
if (*s == '-' || !*s) return -1
cut it? Since your call to strtoumax() checks that the string was parsed to the end by insisting *p is NUL?
If you are trying to more explicitly insist that s[] has only digits, which may not be a bad idea, as that is what we generally expect, then
if (!s[0] || s[strspn(s, "0123456789")]) return -1;
perhaps.
> + uim = strtoumax(s, &p, base); > + if ((errno || *p || p == s) || uim > SIZE_MAX) > + return -1;
And with !s[0] upfront, we can discard (p==s) case from here. Other strto*() wrappers we have may need the "cannot be empty" check, because they do not need any upfront validation of s[] like we do here (we do so to reject negative numbers), but since we do need to check s[] before calling the system strto*() function anyway, it is OK to be different here from the others.
If uintmax_t and size_t are of the same width, then (SIZE_MAX < uim) becomes mathmatically impossible, but hopefully no compiler or static checker is stupid enough to warn against it.
Show 7 quoted lines
> + *result = uim;
> + return 0;
> +}
> +
> static inline int strtol_i(char const *s, int base, int *result)
> {
> long ul;