| From: | Takeshi Ideriha <iderihatakeshi(at)gmail(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Cc: | Prashant Chinnam <crprashant(at)hotmail(dot)com> |
| Subject: | Re: Return INT_MAX or INT_MIN from strtoint() on overflow |
| Date: | 2026-10-30 01:02:52 |
| Message-ID: | d99efefc-ac7b-41d7-a34e-232234a44fa4@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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/852FF0DD-8C49-4A7A-8CE1-C7421F1E97B7@gmail.com
> [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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Chao Li | 2026-10-30 01:11:19 | Re: Return INT_MAX or INT_MIN from strtoint() on overflow |
| Previous Message | Amit Langote | 2026-10-30 00:30:37 | Re: RI fastpath misses checking EXECUTE on functions |