Subject: Re: replace strtok()

From: Kyotaro Horiguchi <[email-masked]>
Date: Wed, 19 Jun 2024 17:30:21 +0900 (JST)

At Tue, 18 Jun 2024 09:18:28 +0200, Peter Eisentraut <[email-masked]> wrote in 
> Under the topic of getting rid of thread-unsafe functions in the
> backend [0], here is a patch series to deal with strtok().
> 
> Of course, strtok() is famously not thread-safe and can be replaced by
> strtok_r().  But it also has the wrong semantics in some cases,
> because it considers adjacent delimiters to be one delimiter.  So if
> you parse
> 
>     SCRAM-SHA-256$<iterations>:<salt>$<storedkey>:<serverkey>
> 
> with strtok(), then
> 
>     SCRAM-SHA-256$$<iterations>::<salt>$$<storedkey>::<serverkey>
> 
> parses just the same.  In many cases, this is arguably wrong and could
> hide mistakes.
> 
> So I'm suggesting to use strsep() in those places.  strsep() is
> nonstandard but widely available.
> 
> There are a few places where strtok() has the right semantics, such as
> parsing tokens separated by whitespace.  For those, I'm using
> strtok_r().

I agree with the distinction.

> A reviewer job here would be to check whether I made that distinction
> correctly in each case.

0001 and 0002 look correct to me regarding that distinction. They
applied correctly to the master HEAD and all tests passed on Linux.

> On the portability side, I'm including a port/ replacement for
> strsep() and some workaround to get strtok_r() for Windows.  I have
> included these here as separate patches for clarity.

0003 looks fine and successfully built and seems working on an MSVC
build.

About 0004, Cygwin seems to have its own strtok_r, but I haven't
checked how that fact affects the build.

> [0]:
> https://www.postgresql.org/message-id/[email-masked]

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center