Re: replace strtok()

From: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
To: Alexander Lakhin <exclusion(at)gmail(dot)com>
Cc: Peter Eisentraut <peter(at)eisentraut(dot)org>, David Steele <david(at)pgmasters(dot)net>, Michael Paquier <michael(at)paquier(dot)xyz>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: replace strtok()
Date: 2024-10-15 12:07:16
Message-ID: CAEudQArvrBKbcTKVL41rdcBA2ADLiO4yg+zu4UVJSpwZUXMsMQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Em ter., 15 de out. de 2024 às 09:00, Alexander Lakhin <exclusion(at)gmail(dot)com>
escreveu:

> Hello Ranier and Peter,
>
> 15.10.2024 14:44, Ranier Vilela wrote:
>
>
>
> Em ter., 15 de out. de 2024 às 03:45, Peter Eisentraut <
> peter(at)eisentraut(dot)org> escreveu:
>
>>
>> Thanks for the analysis. I think moreover we *only* need to check the
>> "stringp" for NULL, not the return value of strsep(), which would never
>> be NULL in our case. So I propose the attached patch as a variant of
>> yours.
>>
> I'm not 100% sure, but the contrib passwordcheck uses and It's not very
> safe.
> The checks of NULL return are cheap, and will protect unwary users.
>
> So I'm neutral here.
>
>
> I also wonder, if other places touched by 5d2e1cc11 need corrections too.
> I played with
> PG_COLOR=always PG_COLORS="error=01;31" .../initdb
>
> and it looks like this free() call in pg_logging_init():
> char *colors = strdup(pg_colors_env);
>
> if (colors)
> {
> ...
> while ((token = strsep(&colors, ":")))
> {
> ...
> }
>
> free(colors);
> }
> gets null in colors.
>
Yeah, I also saw this usage, but I was waiting for a definition for the
first report.
The solution IMO, would be the same.

diff --git a/src/common/logging.c b/src/common/logging.c
index aedd1ae2d8..45b5316d48 100644
--- a/src/common/logging.c
+++ b/src/common/logging.c
@@ -121,7 +121,7 @@ pg_logging_init(const char *argv0)
{
char *token;

- while ((token = strsep(&colors, ":")))
+ while ((token = strsep(&colors, ":")) != NULL && colors != NULL)
{
char *e = strchr(token, '=');

The advantage of this change is that it would avoid processing unnecessary
tokens.

best regards,
Ranier Vilela

>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Gurev Oleg 2024-10-15 12:44:40 Added anchor links generation function for refsect1,2,3
Previous Message Alexander Lakhin 2024-10-15 12:00:00 Re: replace strtok()