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 |
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 |