public inbox for [email protected]  

help / color / mirror / Atom feed

From: Ranier Vilela <[email protected]>
To: Peter Eisentraut <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: replace strtok()
Date: Tue, 18 Jun 2024 08:43:23 -0300
Message-ID: <CAEudQApMiHHpx_GA96B8RMWpJ42CEYD4MjtvJpfj7cXrjn8i=Q@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>

Em ter., 18 de jun. de 2024 às 04:18, Peter Eisentraut <[email protected]>
escreveu:

> 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().
>
> A reviewer job here would be to check whether I made that distinction
> correctly in each case.
>
> 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.
>
+1 For making the code thread-safe.
But I would like to see more const char * where this is possible.

For example, in pg_locale.c
IMO, the token variable can be const char *.

At least strchr expects a const char * as the first parameter.

I found another implementation of strsep, it seems lighter to me.
I will attach it for consideration, however, I have not done any testing.

best regards,
Ranier Vilela

/* strsep.h
 *
 *  Provides the 4.4BSD strsep(3) function for those that don't have it.
 *
 *  Copyright 2011 Michael Thomas Greer
 *  Distributed under the Boost Software License, Version 1.0.
 *  ( See accompanying file LICENSE_1_0.txt or copy at
 *   http://www.boost.org/LICENSE_1_0.txt )
 *
 *  Including this file modifies the std namespace in C++.
 *
 *  Don't include this file if your compiler provides the strsep function in <string.h>.
 *  Make sure your build process tests for this and behaves accordingly!
 *
 */

#include <string.h>

char *
strsep(char **stringp, const char *delim)
{
	char *result;

	if ((stringp == NULL) || (*stringp == NULL))
		return NULL;

	result = *stringp;

	while(**stringp && !(strchr(delim, **stringp)))
		++*stringp;

	if (**stringp)
		*(*stringp)++ = '\0';
	else
		*stringp = NULL;

	return result;
}


Attachments:

  [text/plain] strsep.c (885B, ../CAEudQApMiHHpx_GA96B8RMWpJ42CEYD4MjtvJpfj7cXrjn8i=Q@mail.gmail.com/3-strsep.c)
  download | inline:
/* strsep.h
 *
 *  Provides the 4.4BSD strsep(3) function for those that don't have it.
 *
 *  Copyright 2011 Michael Thomas Greer
 *  Distributed under the Boost Software License, Version 1.0.
 *  ( See accompanying file LICENSE_1_0.txt or copy at
 *   http://www.boost.org/LICENSE_1_0.txt )
 *
 *  Including this file modifies the std namespace in C++.
 *
 *  Don't include this file if your compiler provides the strsep function in <string.h>.
 *  Make sure your build process tests for this and behaves accordingly!
 *
 */

#include <string.h>

char *
strsep(char **stringp, const char *delim)
{
	char *result;

	if ((stringp == NULL) || (*stringp == NULL))
		return NULL;

	result = *stringp;

	while(**stringp && !(strchr(delim, **stringp)))
		++*stringp;

	if (**stringp)
		*(*stringp)++ = '\0';
	else
		*stringp = NULL;

	return result;
}

view thread (3+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: replace strtok()
  In-Reply-To: <CAEudQApMiHHpx_GA96B8RMWpJ42CEYD4MjtvJpfj7cXrjn8i=Q@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox