Re: [PATCH 2/4] parse: add functions for parsing from non-string buffers
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 20, 2026, 20:54 UTC
- Message-ID
- <xmqqldhsxawm.fsf@gitster.g>
- In-Reply-To
- <4d83375b-76e2-4420-80dd-6a04d3201532@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
Show 42 quoted lines
>> There are a few choices regarding the interface and the implementation.
>>
>> First, the implementation:
>> ...
> This all sounds sensible to me and an does the interface description.
> ...
> If we're parsing INTMAX_MIN then this negation tries to calculate
> -INTMAX_MIN which is undefined (I've added some tests for parsing
> INTMAX_MAX and INTMAX_MIN at [1] and verified that UBSAN is triggered
> when parsing INTMAX_MIN). We could do
>
> *ret = u_ret;
> if (*ret != INTMAX_MIN)
> *ret = -*ret;
>
> but I think it might be easier to alter parse_from_buf_internal() to
> make "negate" a local variable, change the function argument to "bool
> allow_negative" and do
>
> *ret = negate ? 0u - val : val;
>
> Then parse_signed_from_buf() can do "*ret = *u_ret;" to convert the
> output of parse_from_buf_internal() to a signed value.
>
>> diff --git a/t/unit-tests/u-parse-int.c b/t/unit-tests/u-parse-int.c
>> new file mode 100644
>> index 0000000000..a1601bb16b
>> --- /dev/null
>> +++ b/t/unit-tests/u-parse-int.c
>> @@ -0,0 +1,98 @@
>> +#include "unit-test.h"
>> +#include "parse.h"
>> +
>> +static void check_int(const char *buf, size_t len,
>> + size_t expect_ep_ofs, int expect_errno,
>> + int expect_result)
>> +{
>> + const char *ep;
>> + int result;
>
> Do we want to set errno=0 here so that we can be sure it has been set by
> parse_int_from_buf() when we check it below?After this message, the discussion stopped and the topic has been dormant since then for a month and a half. I'd drop the topic from 'seen' soonish but that does not mean an improved version of this patch is unwelcome.
Thanks.