Hi Chao
On 2026/10/28 10:28, Chao Li wrote:
> Hi,
>
> I recently reviewed a patch [1] that switches strtoint() to strtoi64() to
> handle an int32 overflow problem. I initially thought we could keep using
> strtoint(), but then realized that its current return value cannot reliably
> indicate the direction of overflow, which the caller needs to clamp the value
> correctly.
Thank you for the suggestion.
I suggested using strtoi64 instead of strtoint in the original thread
and this is exactly why I suggested.
> Looking at the current code:
> ```
> /*
> * strtoint --- just like strtol, but returns int not long
> */
> int
> strtoint(const char *pg_restrict str, char **pg_restrict endptr, int base)
> {
> long val;
>
> val = strtol(str, endptr, base);
> if (val != (int) val)
> errno = ERANGE;
> return (int) val;
> }
> ```
>
> When overflow occurs, it returns “(int) val”, and when the value is outside
> the range of int, the cast produces an implementation-defined result, which
> cannot reliably indicate the direction of overflow. See the C11 draft [2],
> section 6.3.1.3, paragraph 3.
>
> The header comment says “just like strtol”, but strtol() returns LONG_MAX or
> LONG_MIN when overflow happens. See [3]:
> ```
> If the correct value is outside the range of representable values,
> {LONG_MIN}, {LONG_MAX}, {LLONG_MIN}, or {LLONG_MAX} shall be
> returned (according to the sign of the value), and errno set to
> [ERANGE].
> ```
>
> So, the current implementation doesn’t fully match the intention described in
> the header comment.
>
> Attached is a simple fix to change strtoint() to return INT_MAX or INT_MIN
> upon overflow, which makes its behavior consistent with strtol(). I have run
> “make check-world”, and all tests passed.
I think it's still caller's responsibility to check if overflow occurs
but providing overflow direction as an additional information would be
no harm and sometimes helpful.
> If this patch is accepted, I will go back to patch [1] and comment my
> original thoughts on that patch.
+1
> [1] https://postgr.es/m/[email protected]
> [2] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf#page=69
> [3] https://man7.org/linux/man-pages/man3/strtol.3p.html
Regards,
Takeshi Ideriha
