Hi. I noticed that there is a lot of repeating code like this:
``` if (strspn(str, " \t\n\r\f") == strlen(str)) ``` I personally don't find it particularly readable, not mentioning that traversing a string twice doesn't look as a good idea (you can check using objdump that latest GCC 6.2 doesn't optimize this code). How about rewriting such a code like this? ``` if (pg_str_containsonly(str, " \t\n\r\f")) ``` Corresponding patch is attached. I don't claim that my implementation of pg_str_containsonly procedure is faster that strspn + strlen, but at least such refactoring makes code a little more readable. -- Best regards, Aleksander Alekseev
diff --git a/src/backend/parser/parse_type.c b/src/backend/parser/parse_type.c index a8bb472..a1c5853 100644 --- a/src/backend/parser/parse_type.c +++ b/src/backend/parser/parse_type.c @@ -17,6 +17,7 @@ #include "access/htup_details.h" #include "catalog/namespace.h" #include "catalog/pg_type.h" +#include "common/string.h" #include "lib/stringinfo.h" #include "nodes/makefuncs.h" #include "parser/parser.h" @@ -696,7 +697,7 @@ typeStringToTypeName(const char *str) ErrorContextCallback ptserrcontext; /* make sure we give useful error for empty input */ - if (strspn(str, " \t\n\r\f") == strlen(str)) + if (pg_str_containsonly(str, " \t\n\r\f")) goto fail; initStringInfo(&buf); diff --git a/src/backend/tsearch/ts_utils.c b/src/backend/tsearch/ts_utils.c index 9c3e15c..81d6132 100644 --- a/src/backend/tsearch/ts_utils.c +++ b/src/backend/tsearch/ts_utils.c @@ -17,6 +17,7 @@ #include <ctype.h> #include "miscadmin.h" +#include "common/string.h" #include "tsearch/ts_locale.h" #include "tsearch/ts_utils.h" @@ -45,7 +46,7 @@ get_tsearch_config_filename(const char *basename, * and case-insensitive filesystems, and non-ASCII characters create other * interesting risks, so on the whole a tight policy seems best. */ - if (strspn(basename, "abcdefghijklmnopqrstuvwxyz0123456789_") != strlen(basename)) + if (!pg_str_containsonly(basename, "abcdefghijklmnopqrstuvwxyz0123456789_")) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid text search configuration file name \"%s\"", diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c index 394042c..7e41359 100644 --- a/src/backend/utils/adt/regproc.c +++ b/src/backend/utils/adt/regproc.c @@ -32,6 +32,7 @@ #include "catalog/pg_ts_config.h" #include "catalog/pg_ts_dict.h" #include "catalog/pg_type.h" +#include "common/string.h" #include "lib/stringinfo.h" #include "miscadmin.h" #include "parser/parse_type.h" @@ -75,7 +76,7 @@ regprocin(PG_FUNCTION_ARGS) /* Numeric OID? */ if (pro_name_or_oid[0] >= '0' && pro_name_or_oid[0] <= '9' && - strspn(pro_name_or_oid, "0123456789") == strlen(pro_name_or_oid)) + pg_str_containsonly(pro_name_or_oid, "0123456789")) { result = DatumGetObjectId(DirectFunctionCall1(oidin, CStringGetDatum(pro_name_or_oid))); @@ -286,7 +287,7 @@ regprocedurein(PG_FUNCTION_ARGS) /* Numeric OID? */ if (pro_name_or_oid[0] >= '0' && pro_name_or_oid[0] <= '9' && - strspn(pro_name_or_oid, "0123456789") == strlen(pro_name_or_oid)) + pg_str_containsonly(pro_name_or_oid, "0123456789")) { result = DatumGetObjectId(DirectFunctionCall1(oidin, CStringGetDatum(pro_name_or_oid))); @@ -535,7 +536,7 @@ regoperin(PG_FUNCTION_ARGS) /* Numeric OID? */ if (opr_name_or_oid[0] >= '0' && opr_name_or_oid[0] <= '9' && - strspn(opr_name_or_oid, "0123456789") == strlen(opr_name_or_oid)) + pg_str_containsonly(opr_name_or_oid, "0123456789")) { result = DatumGetObjectId(DirectFunctionCall1(oidin, CStringGetDatum(opr_name_or_oid))); @@ -750,7 +751,7 @@ regoperatorin(PG_FUNCTION_ARGS) /* Numeric OID? */ if (opr_name_or_oid[0] >= '0' && opr_name_or_oid[0] <= '9' && - strspn(opr_name_or_oid, "0123456789") == strlen(opr_name_or_oid)) + pg_str_containsonly(opr_name_or_oid, "0123456789")) { result = DatumGetObjectId(DirectFunctionCall1(oidin, CStringGetDatum(opr_name_or_oid))); @@ -995,7 +996,7 @@ regclassin(PG_FUNCTION_ARGS) /* Numeric OID? */ if (class_name_or_oid[0] >= '0' && class_name_or_oid[0] <= '9' && - strspn(class_name_or_oid, "0123456789") == strlen(class_name_or_oid)) + pg_str_containsonly(class_name_or_oid, "0123456789")) { result = DatumGetObjectId(DirectFunctionCall1(oidin, CStringGetDatum(class_name_or_oid))); @@ -1186,7 +1187,7 @@ regtypein(PG_FUNCTION_ARGS) /* Numeric OID? */ if (typ_name_or_oid[0] >= '0' && typ_name_or_oid[0] <= '9' && - strspn(typ_name_or_oid, "0123456789") == strlen(typ_name_or_oid)) + pg_str_containsonly(typ_name_or_oid, "0123456789")) { result = DatumGetObjectId(DirectFunctionCall1(oidin, CStringGetDatum(typ_name_or_oid))); @@ -1358,7 +1359,7 @@ regconfigin(PG_FUNCTION_ARGS) /* Numeric OID? */ if (cfg_name_or_oid[0] >= '0' && cfg_name_or_oid[0] <= '9' && - strspn(cfg_name_or_oid, "0123456789") == strlen(cfg_name_or_oid)) + pg_str_containsonly(cfg_name_or_oid, "0123456789")) { result = DatumGetObjectId(DirectFunctionCall1(oidin, CStringGetDatum(cfg_name_or_oid))); @@ -1468,7 +1469,7 @@ regdictionaryin(PG_FUNCTION_ARGS) /* Numeric OID? */ if (dict_name_or_oid[0] >= '0' && dict_name_or_oid[0] <= '9' && - strspn(dict_name_or_oid, "0123456789") == strlen(dict_name_or_oid)) + pg_str_containsonly(dict_name_or_oid, "0123456789")) { result = DatumGetObjectId(DirectFunctionCall1(oidin, CStringGetDatum(dict_name_or_oid))); @@ -1578,7 +1579,7 @@ regrolein(PG_FUNCTION_ARGS) /* Numeric OID? */ if (role_name_or_oid[0] >= '0' && role_name_or_oid[0] <= '9' && - strspn(role_name_or_oid, "0123456789") == strlen(role_name_or_oid)) + pg_str_containsonly(role_name_or_oid, "0123456789")) { result = DatumGetObjectId(DirectFunctionCall1(oidin, CStringGetDatum(role_name_or_oid))); @@ -1699,7 +1700,7 @@ regnamespacein(PG_FUNCTION_ARGS) /* Numeric OID? */ if (nsp_name_or_oid[0] >= '0' && nsp_name_or_oid[0] <= '9' && - strspn(nsp_name_or_oid, "0123456789") == strlen(nsp_name_or_oid)) + pg_str_containsonly(nsp_name_or_oid, "0123456789")) { result = DatumGetObjectId(DirectFunctionCall1(oidin, CStringGetDatum(nsp_name_or_oid))); diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 2a68359..5330154 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -62,6 +62,7 @@ #include "catalog/storage.h" #include "commands/policy.h" #include "commands/trigger.h" +#include "common/string.h" #include "miscadmin.h" #include "nodes/nodeFuncs.h" #include "optimizer/clauses.h" @@ -5874,7 +5875,7 @@ RelationCacheInitFileRemove(void) while ((de = ReadDir(dir, tblspcdir)) != NULL) { - if (strspn(de->d_name, "0123456789") == strlen(de->d_name)) + if (pg_str_containsonly(de->d_name, "0123456789")) { /* Scan the tablespace dir for per-database dirs */ snprintf(path, sizeof(path), "%s/%s/%s", @@ -5905,7 +5906,7 @@ RelationCacheInitFileRemoveInDir(const char *tblspcpath) while ((de = ReadDir(dir, tblspcpath)) != NULL) { - if (strspn(de->d_name, "0123456789") == strlen(de->d_name)) + if (pg_str_containsonly(de->d_name, "0123456789")) { /* Try to remove the init file in each database */ snprintf(initfilename, sizeof(initfilename), "%s/%s/%s", diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 6cf3829..db29c38 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -52,6 +52,7 @@ #include "access/xact.h" #include "access/xlog.h" #include "catalog/catalog.h" +#include "common/string.h" #include "lib/pairingheap.h" #include "miscadmin.h" #include "storage/predicate.h" @@ -1423,7 +1424,7 @@ ImportSnapshot(const char *idstr) * Verify the identifier: only 0-9, A-F and hyphens are allowed. We do * this mainly to prevent reading arbitrary files. */ - if (strspn(idstr, "0123456789ABCDEF-") != strlen(idstr)) + if (!pg_str_containsonly(idstr, "0123456789ABCDEF-")) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid snapshot identifier: \"%s\"", idstr))); diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index b89bd99..acfb760 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -27,6 +27,7 @@ #include "pg_backup_utils.h" #include "dumputils.h" #include "fe_utils/string_utils.h" +#include "common/string.h" #include <ctype.h> #include <fcntl.h> @@ -1364,7 +1365,7 @@ SortTocFromFile(Archive *AHX) cmnt[0] = '\0'; /* Ignore if all blank */ - if (strspn(buf, " \t\r\n") == strlen(buf)) + if (pg_str_containsonly(buf, " \t\r\n")) continue; /* Get an ID, check it's valid and not already seen */ diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 7949aad..d2f9555 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -54,6 +54,7 @@ #include "catalog/pg_proc.h" #include "catalog/pg_trigger.h" #include "catalog/pg_type.h" +#include "common/string.h" #include "libpq/libpq-fs.h" #include "dumputils.h" @@ -1908,7 +1909,7 @@ dumpTableData_insert(Archive *fout, void *dcontext) */ const char *s = PQgetvalue(res, tuple, field); - if (strspn(s, "0123456789 +-eE.") == strlen(s)) + if (pg_str_containsonly(s, "0123456789 +-eE.")) archputs(s, fout); else archprintf(fout, "'%s'", s); diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index a9a2fdb..eed8865 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -38,6 +38,7 @@ #include "fe_utils/string_utils.h" #include "common.h" +#include "common/string.h" #include "copy.h" #include "crosstabview.h" #include "describe.h" @@ -591,7 +592,7 @@ exec_command(const char *cmd, { /* only one arg; maybe it is lineno not fname */ if (fname[0] && - strspn(fname, "0123456789") == strlen(fname)) + pg_str_containsonly(fname, "0123456789")) { /* all digits, so assume it is lineno */ ln = fname; diff --git a/src/bin/psql/crosstabview.c b/src/bin/psql/crosstabview.c index b283c24..f8f1942 100644 --- a/src/bin/psql/crosstabview.c +++ b/src/bin/psql/crosstabview.c @@ -10,6 +10,7 @@ #include <string.h> #include "common.h" +#include "common/string.h" #include "crosstabview.h" #include "pqexpbuffer.h" #include "psqlscanslash.h" @@ -599,8 +600,8 @@ rankSort(int num_columns, pivot_field *piv_columns) /* ranking information is valid if non null and matches /^-?\d+$/ */ if (val && ((*val == '-' && - strspn(val + 1, "0123456789") == strlen(val + 1)) || - strspn(val, "0123456789") == strlen(val))) + pg_str_containsonly(val + 1, "0123456789")) || + pg_str_containsonly(val, "0123456789"))) { hmap[i * 2] = atoi(val); hmap[i * 2 + 1] = i; @@ -637,7 +638,7 @@ indexOfColumn(char *arg, const PGresult *res) { int idx; - if (arg[0] && strspn(arg, "0123456789") == strlen(arg)) + if (arg[0] && pg_str_containsonly(arg, "0123456789")) { /* if arg contains only digits, it's a column number */ idx = atoi(arg) - 1; diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index f0d955b..47856e4 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -18,6 +18,7 @@ #include "fe_utils/string_utils.h" #include "common.h" +#include "common/string.h" #include "describe.h" #include "fe_utils/mbprint.h" #include "fe_utils/print.h" @@ -309,7 +310,7 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool /* No "Parallel" column before 9.6 */ static const bool translate_columns_pre_96[] = {false, false, false, false, true, true, false, true, false, false, false, false}; - if (strlen(functypes) != strspn(functypes, "antwS+")) + if (!pg_str_containsonly(functypes, "antwS+")) { psql_error("\\df only takes [antwS+] as options\n"); return true; diff --git a/src/common/string.c b/src/common/string.c index 69b26e7..7463fb9 100644 --- a/src/common/string.c +++ b/src/common/string.c @@ -41,3 +41,40 @@ pg_str_endswith(const char *str, const char *end) str += slen - elen; return strcmp(str, end) == 0; } + +/* + * Returns whether the string `str' contains only letters from `allowed'. + * If `str' is empty procedure always returns `true'. + */ +bool +pg_str_containsonly(const char *str, const char *allowed) +{ + /* + * Cast arguments to unsigned type. Otherwise binary shift will work not as + * you might expect. Don't use `str' or `allowed' directly anywhere below! + */ + const unsigned char *str_u = (const unsigned char *) str; + const unsigned char *allowed_u = (const unsigned char *) allowed; + unsigned char allowed_table[32]; + + /* Always consider an empty string valid */ + if (*str_u == 0) + return true; + + memset(&allowed_table, 0, sizeof(allowed_table)); + + while (*allowed_u != 0) + { + allowed_table[*allowed_u >> 3] |= 1 << (*allowed_u & 0x7); + allowed_u++; + } + + while (*str_u != 0) + { + if (!(allowed_table[*str_u >> 3] & (1 << (*str_u & 0x7)))) + return false; + str_u++; + } + + return true; +} diff --git a/src/include/common/string.h b/src/include/common/string.h index bb54d28..77f1ae0 100644 --- a/src/include/common/string.h +++ b/src/include/common/string.h @@ -11,5 +11,6 @@ #define COMMON_STRING_H extern bool pg_str_endswith(const char *str, const char *end); +extern bool pg_str_containsonly(const char *str, const char *allowed); #endif /* COMMON_STRING_H */
signature.asc
Description: PGP signature