Re: replace strtok()

David Steele <david@pgmasters.net>

From: David Steele <david@pgmasters.net>
To: Peter Eisentraut <peter@eisentraut.org>, Michael Paquier <michael@paquier.xyz>, Tom Lane <tgl@sss.pgh.pa.us>
Cc: Ranier Vilela <ranier.vf@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2024-07-08T05:45:50Z
Lists: pgsql-hackers
On 6/24/24 19:57, Peter Eisentraut wrote:
> On 24.06.24 02:34, Michael Paquier wrote:
>> On Sat, Jun 22, 2024 at 11:48:21AM -0400, Tom Lane wrote:
>>> Peter Eisentraut <peter@eisentraut.org> writes:
>>>> On 18.06.24 13:43, Ranier Vilela wrote:
>>>>> I found another implementation of strsep, it seems lighter to me.
>>>>> I will attach it for consideration, however, I have not done any 
>>>>> testing.
>>>
>>>> Yeah, surely there are many possible implementations.  I'm thinking,
>>>> since we already took other str*() functions from OpenBSD, it makes
>>>> sense to do this here as well, so we have only one source to deal with.
>>>
>>> Why not use strpbrk?  That's equally thread-safe, it's been there
>>> since C89, and it doesn't have the problem that you can't find out
>>> which of the delimiter characters was found.
>>
>> Yeah, strpbrk() has been used in the tree as far as 2003 without any
>> port/ implementation.
> 
> The existing uses of strpbrk() are really just checking whether some 
> characters exist in a string, more like an enhanced strchr().  I don't 
> see any uses for tokenizing a string like strtok() or strsep() would do. 
>   I think that would look quite cumbersome.  So I think a simpler and 
> more convenient abstraction like strsep() would still be worthwhile.

I agree that using strsep() in these cases seems more natural. Since 
this patch provides a default implementation compatibility does not seem 
like a big issue.

I've also reviewed the rest of the patch and it looks good to me.

Regards,
-David



Commits

  1. Fix memory leaks from incorrect strsep() uses

  2. Fix strsep() use for SCRAM secrets parsing

  3. Replace remaining strtok() with strtok_r()

  4. Windows replacement for strtok_r()

  5. Replace some strtok() with strsep()

  6. Add port/ replacement for strsep()