El mié, 1 jul 2026 a las 19:30, Junio C Hamano ([off-list ref]) escribió:

Pablo Sabater [off-list ref] writes:

quoted

From: Eric Ju <redacted>

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.

Ok, I'll drop the backquotes for the commit messages of this series.

quoted

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 ().

ACK.

quoted

large as `size_t`, add a range check against `SIZE_MAX` to prevent
`size_t` overflow.

OK.

quoted

This variant is needed in a subsequent commit to enable returning a
`size_t` with proper error handling.

Mentored-by: Karthik Nayak [off-list ref]
Mentored-by: Chandra Pratap [off-list ref]
Signed-off-by: Pablo Sabater <redacted>
---
 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.

I like the idea of only digits but, even though in this series I only
use this function in base 10, I want the function to work in other
bases, that's why I left the base in the function signature instead of
hardcoding it. strspn(s, "0123456789") rejects bases >10  ("ff" for
base 16) while strtoumax does support higher ones.
I think that it would be better to explicitly reject what we don't
want similarly to "-":

if (!*s || isspace((unsigned char)*s) || *s == '-' || *s == '+')
        return -1;

About that, strtoumax works fine with "+" and ignores starting
whitespaces, but for consistency (we reject "-" and whitespaces
between or at the end) rejecting whitespaces and +/- will be better
and make the caller format it correctly.

I'll do that for the next version.

quoted

+     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.

Agreed

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.

quoted

+     *result = uim;
+     return 0;
+}
+
 static inline int strtol_i(char const *s, int base, int *result)
 {
      long ul;

Thanks for the feedback,
Pablo