Re: [PATCH GSoC v15 02/13] git-compat-util: add `strtoumax_szt()` with error handling

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 7, 2026, 18:09 UTC
Message-ID: <xmqqcxwy4qp5.fsf@gitster.g>
In-Reply-To: <CAN5EUNTYeDrQMor29eYMhJD0jcdRQq36ZA6BgupV8gG9xs9rFQ@mail.gmail.com>

Pablo Sabater <pabloosabaterr@gmail.com> writes:

Show 16 quoted lines

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

Let's step back a bit and think.

Where do we plan to use this function? Remember that being a superset is not always necessarily good for a helper function that serves as a format checker.

In the output of "git diff master...ps/cat-file-remote-object-info", there is only one caller, which is fetch_object_info(). It reads into object_info_data[].sizep. Do we expect to express the object size in anything but an unsigned decimal integer? Remember that it is better to be unambiguous when designing a protocol. We do not want a third-party reimplementation of whatever is talking to fetch_object_info() to send object size in hex ;-).

It may also be usable to parse the size of the object payload in object-file.c::parse_loose_header() but notice that it is already even stricter not to use strto<anything> system function and instead handcrafts the trivial number parsing. This would avoid system dependent funnyness, which is a good thing.

Show 9 quoted lines

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