> On Oct 30, 2026, at 04:44, surya poondla <[email protected]> wrote:
>
> Hi Chao,
>
> I'm not sure if the change is needed.
Thanks a lot for the review.
>
> strtoint() job has always been to detect overflow through errno. All the core
> callers in that style (datetime.c, read.c, tsearchcmds.c, jsonfuncs.c, psql,
> option_utils.c) check errno == ERANGE and never
> look at the return value after an overflow. For them v1 changes nothing.
That’s true, but it doesn’t mean the current return-value behavior matches the
header comment. As I mentioned in the patch email, the comment explicitly says
“strtoint --- just like strtol, but returns int not long”. However, unlike
strtol(), it doesn’t reliably return the corresponding bound on overflow. When
val is outside the range of int, “(int) val” produces an implementation-defined
result.
> On Windows, where long is 32 bits, strtol() already returns INT_MAX/INT_MIN,
> so nothing changes there either.
>
Agreed, but the problem here is that strtoint() doesn’t reliably return INT_MAX
or INT_MIN on all platforms.
> The only callers that see a difference are the ones that ignore errno:
> pgcrypto's px_crypt_shacrypt() and a few ecpg pgtypeslib/dt_common.c paths.
> For those, the real bug is that they don't check errno.
>
Not checking errno is not necessarily a bug, it depends on how the caller uses
the result.
In the px_crypt_shacrypt() case, an overflowing value needs to be clamped to
PX_SHACRYPT_ROUNDS_MAX or PX_SHACRYPT_ROUNDS_MIN depending on the direction of
overflow. Checking ERANGE alone doesn’t tell us which bound to use.
If strtoint() reliably returned INT_MAX or INT_MIN on overflow, the existing
range checks would clamp correctly, since both values are outside the supported
rounds range. For that purpose, px_crypt_shacrypt() wouldn’t need to check
ERANGE, and patch [1] wouldn’t need to switch from strtoint() to strtoi64().
So, I think the underlying problem is that strtoint()’s overflow behavior
doesn't fully match its “just like strtol” description. Patch [1] works around
this by switching to strtoi64(). If strtoint() reliably returned INT_MAX or
INT_MIN on overflow, the existing call and range checks would already handle
clamping correctly, and that switch would be unnecessary. From this
perspective, I consider this is a bug of strtoint().
* Note: I’m not saying patch [1] is no longer needed, because it also changes
the NOTICE messages to report the original input, which addresses a separate
issue.
[1] https://postgr.es/m/[email protected]
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
