|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|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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.
|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|