Обсуждение: Fixes bug in strlower_libc_sb()
Hi Hackers,
While reviewing Jeff's patch [1], I found this bug in strlower_libc_sb():
While reviewing Jeff's patch [1], I found this bug in strlower_libc_sb():
```
static size_t
strlower_libc_sb(char *dest, size_t destsize, const char *src, ssize_t srclen,
pg_locale_t locale)
{
if (srclen < 0)
srclen = strlen(src);
if (srclen + 1 <= destsize)
{
locale_t loc = locale->lt;
char *p;
if (srclen + 1 > destsize) <== This check will never be true
return srclen;
```
So I removed the useless check in the patch.
I also noticed strlower_libc_sb’s behavior is different from unicode_strlower(), where the function comment says:
```
* Result string is stored in dst, truncating if larger than dstsize. If
* dstsize is greater than the result length, dst will be NUL-terminated;
* otherwise not.
* dstsize is greater than the result length, dst will be NUL-terminated;
* otherwise not.
```
And Jeff’s new code in [1] matches the description. The behavior is like trying the best to copy as many chars as possible, if not enough space in dest, then NULL-terminator can be omitted. But the current behavior of strlower_libc_sb that only copies data to dest when dest has enough space for src and NULL-terminator.
So I updated strlower_libc_sb(), strtitle_libc_sb() and strupper_libc_sb() to comply with the behavior in this patch as well. “Make check” passed from my side, so the patch doesn't seem to break anything.
[1] https://www.postgresql.org/message-id/450ceb6260cad30d7afdf155d991a9caafee7c0d.camel%40j-davis.com
Вложения
On Tue, Nov 25, 2025 at 11:03 AM Chao Li <li.evan.chao@gmail.com> wrote:
Hi Hackers,
While reviewing Jeff's patch [1], I found this bug in strlower_libc_sb():```static size_tstrlower_libc_sb(char *dest, size_t destsize, const char *src, ssize_t srclen,pg_locale_t locale){if (srclen < 0)srclen = strlen(src);if (srclen + 1 <= destsize){locale_t loc = locale->lt;char *p;if (srclen + 1 > destsize) <== This check will never be truereturn srclen;```So I removed the useless check in the patch.I also noticed strlower_libc_sb’s behavior is different from unicode_strlower(), where the function comment says:```* Result string is stored in dst, truncating if larger than dstsize. If
* dstsize is greater than the result length, dst will be NUL-terminated;
* otherwise not.```And Jeff’s new code in [1] matches the description. The behavior is like trying the best to copy as many chars as possible, if not enough space in dest, then NULL-terminator can be omitted. But the current behavior of strlower_libc_sb that only copies data to dest when dest has enough space for src and NULL-terminator.So I updated strlower_libc_sb(), strtitle_libc_sb() and strupper_libc_sb() to comply with the behavior in this patch as well. “Make check” passed from my side, so the patch doesn't seem to break anything.[1] https://www.postgresql.org/message-id/450ceb6260cad30d7afdf155d991a9caafee7c0d.camel%40j-davis.com
I thought over and splitted the patch into two commits:
0001 - Fixes the obvious bug by simply deleting the useless length check.
0002 - Updates strlower_lic_sb, strtitle_lic_sb, strupper_lic_sb to comply with unicode_strlower's truncation behavior
Best regards,
Chao Li (Evan)---------------------
HighGo Software Co., Ltd.
Вложения
Hi,
On Tue, Nov 25, 2025 at 01:50:42PM +0800, Chao Li wrote:
> On Tue, Nov 25, 2025 at 11:03 AM Chao Li <li.evan.chao@gmail.com> wrote:
>
> > Hi Hackers,
> >
> > While reviewing Jeff's patch [1], I found this bug in strlower_libc_sb():
> >
> > ```
> > static size_t
> > strlower_libc_sb(char *dest, size_t destsize, const char *src, ssize_t
> > srclen,
> > pg_locale_t locale)
> > {
> > if (srclen < 0)
> > srclen = strlen(src);
Nice catch!
FWIW this thread gave me the idea to create a coccinelle script to detect dead
branches [1]. It reports 4 more that are false positives, so I don't think there
is more dead branches in the code tree.
[1]: https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/dead_branches.cocci
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com