Re: [PATCH] Refactor "if(strspn(str, ...) == strlen(str)" code

From: Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Geoff Winkless <pgsqladmin(at)geoff(dot)dj>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Refactor "if(strspn(str, ...) == strlen(str)" code
Date: 2016-12-08 18:23:49
Message-ID: 20161208182349.GA28615@e733.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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 Content-Type Size
pg_str_containsonly-v2.patch text/x-diff 12.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-12-08 18:28:07 Re: postgres_fdw bug in 9.6
Previous Message Robert Haas 2016-12-08 18:04:31 Re: postgres_fdw bug in 9.6