Re: [PATCH] Refactor "if(strspn(str, ...) == strlen(str)" code - Mailing list pgsql-hackers

From Aleksander Alekseev
Subject Re: [PATCH] Refactor "if(strspn(str, ...) == strlen(str)" code
Date
Msg-id 20161208182349.GA28615@e733.localdomain
Whole thread Raw
In response to Re: [PATCH] Refactor "if(strspn(str, ...) == strlen(str)" code  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [HACKERS] [PATCH] Refactor "if(strspn(str, ...) == strlen(str)" code
List pgsql-hackers
Tom, Geoff,

Thanks for your feedback! Here is version 2 of this patch.

> You could just change it to
> if (str[strspn(str, " \t\n\r\f")] == '\0')
> to mitigate calling strlen. It's safe to do so because strspn will
> only return values from 0 to strlen(str).

> [...] I have serious doubts that the "optimized" implementation
> you propose is actually faster than a naive one; it may be slower, and
> it's certainly longer and harder to understand/test.

I would like to point out that I never said it's optimized. However I
like the code Geoff proposed. It definitely doesn't make anything worse.
I decided to keep pg_str_contansonly procedure (i.e. not to inline this
code) for now. Code with strspn looks OK in a simple example. However in
a concrete context it looks like a bad Perl code in ROT13 to me:

```
/* try to figure out what's exactly going on */
if(somelongname[strspn(somelongname /* yes, again */, "longlistofchars")] != '\0')
```
> Function name seems weirdly spelled.

I named it the same way pg_str_endswith is named. However I'm open for
better suggestions here.

> Whether it's worth worrying about, I dunno.  This is hardly the only
> C idiom you need to be familiar with to read the PG code.

Well, at least this v2 version of the patch removes second string
scanning. And I still believe that this inlined strspn is not readable
or obvious at all.

--
Best regards,
Aleksander Alekseev

Attachment

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: postgres_fdw bug in 9.6
Next
From: Tom Lane
Date:
Subject: Re: postgres_fdw bug in 9.6