On Mon, Sep 09, 2019 at 05:27:04AM -0700, Andres Freund wrote: > On 2019-09-09 20:57:46 +0900, Michael Paquier wrote: > But ISTM all of them ought to just use the C types, rather than the SQL > types however. Since in the above proposal the caller determines the > type names, if you want a different type - like the SQL input routines - > can just invoke pg_strtoint_error() themselves (or just have it open > coded).
Yep, that was my line of thoughts. >> And for errors which should never happen we could just use >> elog(). For the input functions of int2/4/8 we still need the >> existing errors of course. > > Right, there it makes sense to continue to refer the SQL level types. Actually, I found your suggestion of using a noreturn function for the error reporting to be a very clean alternative. I didn't know though that gcc is not able to detect that a function does not return if you don't have a default in the switch for all the status codes. And this even if all the values of the enum for the switch are listed. > I'm saying that we shouldn't need the whole logic of trying to parse the > string as an int, and then fail to float if it's not that. But that it's > not this patchset's task to fix this. Ah, sure. Agreed. >> Not sure about that. I would keep the scope of the patch simple as of >> now, where we make sure that we have the right interface for >> everything. There are a couple of extra improvements which could be >> done afterwards, and if we move everything in the same place that >> should be easier to move on with more improvements. Hopefully. > > The only reason for thinking about it now is that we'd then avoid > changing the API twice. What I think we would be looking for here is an extra argument for the low-level routines to control the behavior of the function in an extensible way, say a bits16 for a set of flags, with one flag to ignore checks for trailing and leading whitespace. This feels a bit over-engineered though for this purpose. Attached is an updated patch? How does it look? I have left the parts of readfuncs.c for now as there are more issues behind that than doing a single switch, short reads are one, long reads a second. And the patch already does a lot. There could be also an argument for having extra _check wrappers for the unsigned portions but these would be mostly unused in the backend code, so I have left that out on purpose. After all that stuff, there are still some issues which need more care, in short: - the T_Float conversion. - removal of strtoint() - the part for readfuncs.c -- Michael
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 221b47298c..1ed03aae02 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -62,6 +62,7 @@ #include <unistd.h> #include "catalog/pg_authid.h" +#include "common/string.h" #include "executor/instrument.h" #include "funcapi.h" #include "mb/pg_wchar.h" @@ -1022,7 +1023,7 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString, /* parse command tag to retrieve the number of affected rows. */ if (completionTag && strncmp(completionTag, "COPY ", 5) == 0) - rows = pg_strtouint64(completionTag + 5, NULL, 10); + (void) pg_strtouint64(completionTag + 5, &rows); else rows = 0; diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c index adf0490f85..b063f1e122 100644 --- a/contrib/spi/refint.c +++ b/contrib/spi/refint.c @@ -306,7 +306,7 @@ check_foreign_key(PG_FUNCTION_ARGS) /* internal error */ elog(ERROR, "check_foreign_key: too short %d (< 5) list of arguments", nargs); - nrefs = pg_strtoint32(args[0]); + nrefs = pg_strtoint32_check(args[0]); if (nrefs < 1) /* internal error */ elog(ERROR, "check_foreign_key: %d (< 1) number of references specified", nrefs); diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 2c0ae395ba..8e75d52b06 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -21,6 +21,7 @@ #include "catalog/heap.h" #include "catalog/pg_type.h" #include "commands/trigger.h" +#include "common/string.h" #include "executor/executor.h" #include "executor/spi_priv.h" #include "miscadmin.h" @@ -2338,8 +2339,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, CreateTableAsStmt *ctastmt = (CreateTableAsStmt *) stmt->utilityStmt; if (strncmp(completionTag, "SELECT ", 7) == 0) - _SPI_current->processed = - pg_strtouint64(completionTag + 7, NULL, 10); + (void) pg_strtouint64(completionTag + 7, &_SPI_current->processed); else { /* @@ -2361,8 +2361,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI, else if (IsA(stmt->utilityStmt, CopyStmt)) { Assert(strncmp(completionTag, "COPY ", 5) == 0); - _SPI_current->processed = pg_strtouint64(completionTag + 5, - NULL, 10); + (void) pg_strtouint64(completionTag + 5, &_SPI_current->processed); } } diff --git a/src/backend/libpq/pqmq.c b/src/backend/libpq/pqmq.c index a9bd47d937..ac2bf2f2c4 100644 --- a/src/backend/libpq/pqmq.c +++ b/src/backend/libpq/pqmq.c @@ -286,10 +286,10 @@ pq_parse_errornotice(StringInfo msg, ErrorData *edata) edata->hint = pstrdup(value); break; case PG_DIAG_STATEMENT_POSITION: - edata->cursorpos = pg_strtoint32(value); + edata->cursorpos = pg_strtoint32_check(value); break; case PG_DIAG_INTERNAL_POSITION: - edata->internalpos = pg_strtoint32(value); + edata->internalpos = pg_strtoint32_check(value); break; case PG_DIAG_INTERNAL_QUERY: edata->internalquery = pstrdup(value); @@ -316,7 +316,7 @@ pq_parse_errornotice(StringInfo msg, ErrorData *edata) edata->filename = pstrdup(value); break; case PG_DIAG_SOURCE_LINE: - edata->lineno = pg_strtoint32(value); + edata->lineno = pg_strtoint32_check(value); break; case PG_DIAG_SOURCE_FUNCTION: edata->funcname = pstrdup(value); diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 764e3bb90c..2a1e9be3eb 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -32,6 +32,7 @@ #include <math.h> +#include "common/string.h" #include "fmgr.h" #include "miscadmin.h" #include "nodes/extensible.h" @@ -80,7 +81,7 @@ #define READ_UINT64_FIELD(fldname) \ token = pg_strtok(&length); /* skip :fldname */ \ token = pg_strtok(&length); /* get field value */ \ - local_node->fldname = pg_strtouint64(token, NULL, 10) + (void) pg_strtouint64(token, &local_node->fldname) /* Read a long integer field (anything written as ":fldname %ld") */ #define READ_LONG_FIELD(fldname) \ diff --git a/src/backend/parser/parse_node.c b/src/backend/parser/parse_node.c index 1baf7ef31f..cc846a71f1 100644 --- a/src/backend/parser/parse_node.c +++ b/src/backend/parser/parse_node.c @@ -17,6 +17,7 @@ #include "access/htup_details.h" #include "access/table.h" #include "catalog/pg_type.h" +#include "common/string.h" #include "mb/pg_wchar.h" #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" @@ -25,7 +26,6 @@ #include "parser/parse_expr.h" #include "parser/parse_relation.h" #include "utils/builtins.h" -#include "utils/int8.h" #include "utils/lsyscache.h" #include "utils/syscache.h" #include "utils/varbit.h" @@ -496,7 +496,7 @@ make_const(ParseState *pstate, Value *value, int location) case T_Float: /* could be an oversize integer as well as a float ... */ - if (scanint8(strVal(value), true, &val64)) + if (pg_strtoint64(strVal(value), &val64) == PG_STRTOINT_OK) { /* * It might actually fit in int32. Probably only INT_MIN can diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c index 6eba08a920..1ca6542c2e 100644 --- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c +++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c @@ -341,7 +341,7 @@ libpqrcv_identify_system(WalReceiverConn *conn, TimeLineID *primary_tli) ntuples, nfields, 3, 1))); } primary_sysid = pstrdup(PQgetvalue(res, 0, 0)); - *primary_tli = pg_strtoint32(PQgetvalue(res, 0, 1)); + *primary_tli = pg_strtoint32_check(PQgetvalue(res, 0, 1)); PQclear(res); return primary_sysid; @@ -487,7 +487,7 @@ libpqrcv_endstreaming(WalReceiverConn *conn, TimeLineID *next_tli) if (PQnfields(res) < 2 || PQntuples(res) != 1) ereport(ERROR, (errmsg("unexpected result set after end-of-streaming"))); - *next_tli = pg_strtoint32(PQgetvalue(res, 0, 0)); + *next_tli = pg_strtoint32_check(PQgetvalue(res, 0, 0)); PQclear(res); /* the result set should be followed by CommandComplete */ diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c index 9c08757fca..d8a2c4979f 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -13,6 +13,7 @@ #include "postgres.h" #include "catalog/pg_publication.h" +#include "common/string.h" #include "fmgr.h" @@ -22,7 +23,6 @@ #include "replication/pgoutput.h" #include "utils/inval.h" -#include "utils/int8.h" #include "utils/memutils.h" #include "utils/syscache.h" #include "utils/varlena.h" @@ -113,7 +113,7 @@ parse_output_parameters(List *options, uint32 *protocol_version, errmsg("conflicting or redundant options"))); protocol_version_given = true; - if (!scanint8(strVal(defel->arg), true, &parsed)) + if (pg_strtoint64(strVal(defel->arg), &parsed) != PG_STRTOINT_OK) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid proto_version"))); diff --git a/src/backend/tsearch/wparser_def.c b/src/backend/tsearch/wparser_def.c index f18e761af8..abde3fdf6d 100644 --- a/src/backend/tsearch/wparser_def.c +++ b/src/backend/tsearch/wparser_def.c @@ -2460,13 +2460,13 @@ prsd_headline(PG_FUNCTION_ARGS) char *val = defGetString(defel); if (pg_strcasecmp(defel->defname, "MaxWords") == 0) - max_words = pg_strtoint32(val); + max_words = pg_strtoint32_check(val); else if (pg_strcasecmp(defel->defname, "MinWords") == 0) - min_words = pg_strtoint32(val); + min_words = pg_strtoint32_check(val); else if (pg_strcasecmp(defel->defname, "ShortWord") == 0) - shortword = pg_strtoint32(val); + shortword = pg_strtoint32_check(val); else if (pg_strcasecmp(defel->defname, "MaxFragments") == 0) - max_fragments = pg_strtoint32(val); + max_fragments = pg_strtoint32_check(val); else if (pg_strcasecmp(defel->defname, "StartSel") == 0) prs->startsel = pstrdup(val); else if (pg_strcasecmp(defel->defname, "StopSel") == 0) diff --git a/src/backend/utils/adt/arrayutils.c b/src/backend/utils/adt/arrayutils.c index 6170e35ac2..29d9784f41 100644 --- a/src/backend/utils/adt/arrayutils.c +++ b/src/backend/utils/adt/arrayutils.c @@ -226,7 +226,7 @@ ArrayGetIntegerTypmods(ArrayType *arr, int *n) result = (int32 *) palloc(*n * sizeof(int32)); for (i = 0; i < *n; i++) - result[i] = pg_strtoint32(DatumGetCString(elem_values[i])); + result[i] = pg_strtoint32_check(DatumGetCString(elem_values[i])); pfree(elem_values); diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c index 6515fc8ec6..15f3babc05 100644 --- a/src/backend/utils/adt/cash.c +++ b/src/backend/utils/adt/cash.c @@ -26,7 +26,6 @@ #include "libpq/pqformat.h" #include "utils/builtins.h" #include "utils/cash.h" -#include "utils/int8.h" #include "utils/numeric.h" #include "utils/pg_locale.h" diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index 755ca6e277..f8cc5bc48a 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -92,7 +92,6 @@ #include "utils/datetime.h" #include "utils/float.h" #include "utils/formatting.h" -#include "utils/int8.h" #include "utils/memutils.h" #include "utils/numeric.h" #include "utils/pg_locale.h" diff --git a/src/backend/utils/adt/int.c b/src/backend/utils/adt/int.c index 04825fc77d..c038a47f8c 100644 --- a/src/backend/utils/adt/int.c +++ b/src/backend/utils/adt/int.c @@ -63,8 +63,16 @@ Datum int2in(PG_FUNCTION_ARGS) { char *num = PG_GETARG_CSTRING(0); + int16 res; + pg_strtoint_status status; - PG_RETURN_INT16(pg_strtoint16(num)); + /* Use a custom set of error messages here adapted to the data type */ + status = pg_strtoint16(num, &res); + + if (unlikely(status != PG_STRTOINT_OK)) + pg_strtoint_error(status, num, "smallint"); + + PG_RETURN_INT16(res); } /* @@ -268,8 +276,16 @@ Datum int4in(PG_FUNCTION_ARGS) { char *num = PG_GETARG_CSTRING(0); + int32 res; + pg_strtoint_status status; - PG_RETURN_INT32(pg_strtoint32(num)); + /* Use a custom set of error messages here adapted to the data type */ + status = pg_strtoint32(num, &res); + + if (unlikely(status != PG_STRTOINT_OK)) + pg_strtoint_error(status, num, "integer"); + + PG_RETURN_INT32(res); } /* diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c index 0ff9394a2f..9933bfa4b4 100644 --- a/src/backend/utils/adt/int8.c +++ b/src/backend/utils/adt/int8.c @@ -23,7 +23,6 @@ #include "nodes/nodeFuncs.h" #include "nodes/supportnodes.h" #include "optimizer/optimizer.h" -#include "utils/int8.h" #include "utils/builtins.h" @@ -47,99 +46,22 @@ typedef struct * Formatting and conversion routines. *---------------------------------------------------------*/ -/* - * scanint8 --- try to parse a string into an int8. - * - * If errorOK is false, ereport a useful error message if the string is bad. - * If errorOK is true, just return "false" for bad input. - */ -bool -scanint8(const char *str, bool errorOK, int64 *result) -{ - const char *ptr = str; - int64 tmp = 0; - bool neg = false; - - /* - * Do our own scan, rather than relying on sscanf which might be broken - * for long long. - * - * As INT64_MIN can't be stored as a positive 64 bit integer, accumulate - * value as a negative number. - */ - - /* skip leading spaces */ - while (*ptr && isspace((unsigned char) *ptr)) - ptr++; - - /* handle sign */ - if (*ptr == '-') - { - ptr++; - neg = true; - } - else if (*ptr == '+') - ptr++; - - /* require at least one digit */ - if (unlikely(!isdigit((unsigned char) *ptr))) - goto invalid_syntax; - - /* process digits */ - while (*ptr && isdigit((unsigned char) *ptr)) - { - int8 digit = (*ptr++ - '0'); - - if (unlikely(pg_mul_s64_overflow(tmp, 10, &tmp)) || - unlikely(pg_sub_s64_overflow(tmp, digit, &tmp))) - goto out_of_range; - } - - /* allow trailing whitespace, but not other trailing chars */ - while (*ptr != '\0' && isspace((unsigned char) *ptr)) - ptr++; - - if (unlikely(*ptr != '\0')) - goto invalid_syntax; - - if (!neg) - { - /* could fail if input is most negative number */ - if (unlikely(tmp == PG_INT64_MIN)) - goto out_of_range; - tmp = -tmp; - } - - *result = tmp; - return true; - -out_of_range: - if (!errorOK) - ereport(ERROR, - (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("value \"%s\" is out of range for type %s", - str, "bigint"))); - return false; - -invalid_syntax: - if (!errorOK) - ereport(ERROR, - (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), - errmsg("invalid input syntax for type %s: \"%s\"", - "bigint", str))); - return false; -} - /* int8in() */ Datum int8in(PG_FUNCTION_ARGS) { - char *str = PG_GETARG_CSTRING(0); - int64 result; + char *str = PG_GETARG_CSTRING(0); + int64 res; + pg_strtoint_status status; - (void) scanint8(str, false, &result); - PG_RETURN_INT64(result); + /* Use a custom set of error messages here adapted to the data type */ + status = pg_strtoint64(str, &res); + + if (unlikely(status != PG_STRTOINT_OK)) + pg_strtoint_error(status, str, "bigint"); + + PG_RETURN_INT64(res); } diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index a00db3ce7a..bc02958950 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -39,7 +39,6 @@ #include "utils/float.h" #include "utils/guc.h" #include "utils/hashutils.h" -#include "utils/int8.h" #include "utils/numeric.h" #include "utils/sortsupport.h" diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c index 70138feb29..845c3bfe0a 100644 --- a/src/backend/utils/adt/numutils.c +++ b/src/backend/utils/adt/numutils.c @@ -19,6 +19,7 @@ #include <ctype.h> #include "common/int.h" +#include "common/string.h" #include "utils/builtins.h" /* @@ -110,157 +111,92 @@ pg_atoi(const char *s, int size, int c) } /* - * Convert input string to a signed 16 bit integer. + * pg_strtoint_error * - * Allows any number of leading or trailing whitespace characters. Will throw - * ereport() upon bad input format or overflow. - * - * NB: Accumulate input as a negative number, to deal with two's complement - * representation of the most negative number, which can't be represented as a - * positive number. + * utility wrapper to report an error with string-to-integer conversion + * functions. */ -int16 -pg_strtoint16(const char *s) +void +pg_strtoint_error(pg_strtoint_status status, const char *input, + const char *type) { - const char *ptr = s; - int16 tmp = 0; - bool neg = false; - - /* skip leading spaces */ - while (likely(*ptr) && isspace((unsigned char) *ptr)) - ptr++; - - /* handle sign */ - if (*ptr == '-') + switch (status) { - ptr++; - neg = true; + case PG_STRTOINT_SYNTAX_ERROR: + ereport(ERROR, + (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), + errmsg("invalid input syntax for type %s: \"%s\"", + type, input))); + break; + + case PG_STRTOINT_RANGE_ERROR: + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("value \"%s\" is out of range for type %s", + input, type))); + break; + + default: + elog(ERROR, "unexpected status for string-to-integer conversion"); + break; } - else if (*ptr == '+') - ptr++; - - /* require at least one digit */ - if (unlikely(!isdigit((unsigned char) *ptr))) - goto invalid_syntax; - - /* process digits */ - while (*ptr && isdigit((unsigned char) *ptr)) - { - int8 digit = (*ptr++ - '0'); - - if (unlikely(pg_mul_s16_overflow(tmp, 10, &tmp)) || - unlikely(pg_sub_s16_overflow(tmp, digit, &tmp))) - goto out_of_range; - } - - /* allow trailing whitespace, but not other trailing chars */ - while (*ptr != '\0' && isspace((unsigned char) *ptr)) - ptr++; - - if (unlikely(*ptr != '\0')) - goto invalid_syntax; - - if (!neg) - { - /* could fail if input is most negative number */ - if (unlikely(tmp == PG_INT16_MIN)) - goto out_of_range; - tmp = -tmp; - } - - return tmp; - -out_of_range: - ereport(ERROR, - (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("value \"%s\" is out of range for type %s", - s, "smallint"))); - -invalid_syntax: - ereport(ERROR, - (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), - errmsg("invalid input syntax for type %s: \"%s\"", - "smallint", s))); - - return 0; /* keep compiler quiet */ } /* - * Convert input string to a signed 32 bit integer. + * pg_strtoint16_check * - * Allows any number of leading or trailing whitespace characters. Will throw - * ereport() upon bad input format or overflow. + * Convert input string to a signed 16-bit integer. * - * NB: Accumulate input as a negative number, to deal with two's complement - * representation of the most negative number, which can't be represented as a - * positive number. + * This throws ereport() upon bad input format or overflow. + */ +int16 +pg_strtoint16_check(const char *s) +{ + int16 result; + pg_strtoint_status status = pg_strtoint16(s, &result); + + if (unlikely(status != PG_STRTOINT_OK)) + pg_strtoint_error(status, s, "int16"); + return result; +} + +/* + * pg_strtoint32_check + * + * Convert input string to a signed 32-bit integer. + * + * This throws ereport() upon bad input format or overflow. */ int32 -pg_strtoint32(const char *s) +pg_strtoint32_check(const char *s) { - const char *ptr = s; - int32 tmp = 0; - bool neg = false; + int32 result; + pg_strtoint_status status = pg_strtoint32(s, &result); - /* skip leading spaces */ - while (likely(*ptr) && isspace((unsigned char) *ptr)) - ptr++; - - /* handle sign */ - if (*ptr == '-') - { - ptr++; - neg = true; - } - else if (*ptr == '+') - ptr++; - - /* require at least one digit */ - if (unlikely(!isdigit((unsigned char) *ptr))) - goto invalid_syntax; - - /* process digits */ - while (*ptr && isdigit((unsigned char) *ptr)) - { - int8 digit = (*ptr++ - '0'); - - if (unlikely(pg_mul_s32_overflow(tmp, 10, &tmp)) || - unlikely(pg_sub_s32_overflow(tmp, digit, &tmp))) - goto out_of_range; - } - - /* allow trailing whitespace, but not other trailing chars */ - while (*ptr != '\0' && isspace((unsigned char) *ptr)) - ptr++; - - if (unlikely(*ptr != '\0')) - goto invalid_syntax; - - if (!neg) - { - /* could fail if input is most negative number */ - if (unlikely(tmp == PG_INT32_MIN)) - goto out_of_range; - tmp = -tmp; - } - - return tmp; - -out_of_range: - ereport(ERROR, - (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("value \"%s\" is out of range for type %s", - s, "integer"))); - -invalid_syntax: - ereport(ERROR, - (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), - errmsg("invalid input syntax for type %s: \"%s\"", - "integer", s))); - - return 0; /* keep compiler quiet */ + if (unlikely(status != PG_STRTOINT_OK)) + pg_strtoint_error(status, s, "int32"); + return result; } +/* + * pg_strtoint64_check + * + * Convert input string to a signed 64-bit integer. + * + * This throws ereport() upon bad input format or overflow. + */ +int64 +pg_strtoint64_check(const char *s) +{ + int64 result; + pg_strtoint_status status = pg_strtoint64(s, &result); + + if (unlikely(status != PG_STRTOINT_OK)) + pg_strtoint_error(status, s, "int64"); + return result; +} + + /* * pg_itoa: converts a signed 16-bit integer to its string representation * @@ -543,25 +479,3 @@ pg_ltostr(char *str, int32 value) return end; } - -/* - * pg_strtouint64 - * Converts 'str' into an unsigned 64-bit integer. - * - * This has the identical API to strtoul(3), except that it will handle - * 64-bit ints even where "long" is narrower than that. - * - * For the moment it seems sufficient to assume that the platform has - * such a function somewhere; let's not roll our own. - */ -uint64 -pg_strtouint64(const char *str, char **endptr, int base) -{ -#ifdef _MSC_VER /* MSVC only */ - return _strtoui64(str, endptr, base); -#elif defined(HAVE_STRTOULL) && SIZEOF_LONG < 8 - return strtoull(str, endptr, base); -#else - return strtoul(str, endptr, base); -#endif -} diff --git a/src/backend/utils/adt/rangetypes.c b/src/backend/utils/adt/rangetypes.c index e5c7e5c7ee..749fc44623 100644 --- a/src/backend/utils/adt/rangetypes.c +++ b/src/backend/utils/adt/rangetypes.c @@ -37,7 +37,6 @@ #include "utils/builtins.h" #include "utils/date.h" #include "utils/hashutils.h" -#include "utils/int8.h" #include "utils/lsyscache.h" #include "utils/rangetypes.h" #include "utils/timestamp.h" diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index d36156f4e4..8c8c98bc3f 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -5626,8 +5626,11 @@ text_format(PG_FUNCTION_ARGS) str = OutputFunctionCall(&typoutputinfo_width, value); - /* pg_strtoint32 will complain about bad data or overflow */ - width = pg_strtoint32(str); + /* + * pg_strtoint32_check will complain about bad data or + * overflow. + */ + width = pg_strtoint32_check(str); pfree(str); } diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l index e9020ad231..84a9d39795 100644 --- a/src/bin/pgbench/exprscan.l +++ b/src/bin/pgbench/exprscan.l @@ -23,6 +23,7 @@ *------------------------------------------------------------------------- */ +#include "common/string.h" #include "fe_utils/psqlscan_int.h" /* context information for reporting errors in expressions */ @@ -205,7 +206,7 @@ notnull [Nn][Oo][Tt][Nn][Uu][Ll][Ll] return MAXINT_PLUS_ONE_CONST; } {digit}+ { - if (!strtoint64(yytext, true, &yylval->ival)) + if (pg_strtoint64(yytext, &yylval->ival) != PG_STRTOINT_OK) expr_yyerror_more(yyscanner, "bigint constant overflow", strdup(yytext)); return INTEGER_CONST; diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index ed7652bfbf..600f1deb71 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -34,6 +34,7 @@ #include "postgres_fe.h" #include "common/int.h" #include "common/logging.h" +#include "common/string.h" #include "fe_utils/conditional.h" #include "getopt_long.h" #include "libpq-fe.h" @@ -664,116 +665,6 @@ usage(void) progname, progname); } -/* return whether str matches "^\s*[-+]?[0-9]+$" */ -static bool -is_an_int(const char *str) -{ - const char *ptr = str; - - /* skip leading spaces; cast is consistent with strtoint64 */ - while (*ptr && isspace((unsigned char) *ptr)) - ptr++; - - /* skip sign */ - if (*ptr == '+' || *ptr == '-') - ptr++; - - /* at least one digit */ - if (*ptr && !isdigit((unsigned char) *ptr)) - return false; - - /* eat all digits */ - while (*ptr && isdigit((unsigned char) *ptr)) - ptr++; - - /* must have reached end of string */ - return *ptr == '\0'; -} - - -/* - * strtoint64 -- convert a string to 64-bit integer - * - * This function is a slightly modified version of scanint8() from - * src/backend/utils/adt/int8.c. - * - * The function returns whether the conversion worked, and if so - * "*result" is set to the result. - * - * If not errorOK, an error message is also printed out on errors. - */ -bool -strtoint64(const char *str, bool errorOK, int64 *result) -{ - const char *ptr = str; - int64 tmp = 0; - bool neg = false; - - /* - * Do our own scan, rather than relying on sscanf which might be broken - * for long long. - * - * As INT64_MIN can't be stored as a positive 64 bit integer, accumulate - * value as a negative number. - */ - - /* skip leading spaces */ - while (*ptr && isspace((unsigned char) *ptr)) - ptr++; - - /* handle sign */ - if (*ptr == '-') - { - ptr++; - neg = true; - } - else if (*ptr == '+') - ptr++; - - /* require at least one digit */ - if (unlikely(!isdigit((unsigned char) *ptr))) - goto invalid_syntax; - - /* process digits */ - while (*ptr && isdigit((unsigned char) *ptr)) - { - int8 digit = (*ptr++ - '0'); - - if (unlikely(pg_mul_s64_overflow(tmp, 10, &tmp)) || - unlikely(pg_sub_s64_overflow(tmp, digit, &tmp))) - goto out_of_range; - } - - /* allow trailing whitespace, but not other trailing chars */ - while (*ptr != '\0' && isspace((unsigned char) *ptr)) - ptr++; - - if (unlikely(*ptr != '\0')) - goto invalid_syntax; - - if (!neg) - { - if (unlikely(tmp == PG_INT64_MIN)) - goto out_of_range; - tmp = -tmp; - } - - *result = tmp; - return true; - -out_of_range: - if (!errorOK) - fprintf(stderr, - "value \"%s\" is out of range for type bigint\n", str); - return false; - -invalid_syntax: - if (!errorOK) - fprintf(stderr, - "invalid input syntax for type bigint: \"%s\"\n", str); - return false; -} - /* convert string to double, detecting overflows/underflows */ bool strtodouble(const char *str, bool errorOK, double *dv) @@ -1288,6 +1179,7 @@ static bool makeVariableValue(Variable *var) { size_t slen; + int64 ival; if (var->value.type != PGBT_NO_VALUE) return true; /* no work */ @@ -1320,15 +1212,16 @@ makeVariableValue(Variable *var) { setBoolValue(&var->value, false); } - else if (is_an_int(var->svalue)) + + /* + * Attempt an int64 conversion. + * + * A too-large but syntactically correct int64 will be parsed as a double + * below. + */ + else if (pg_strtoint64(var->svalue, &ival) == PG_STRTOINT_OK) { - /* if it looks like an int, it must be an int without overflow */ - int64 iv; - - if (!strtoint64(var->svalue, false, &iv)) - return false; - - setIntValue(&var->value, iv); + setIntValue(&var->value, ival); } else /* type should be double */ { diff --git a/src/bin/pgbench/pgbench.h b/src/bin/pgbench/pgbench.h index c4a1e298e0..da3f8fa055 100644 --- a/src/bin/pgbench/pgbench.h +++ b/src/bin/pgbench/pgbench.h @@ -160,7 +160,6 @@ extern void syntax_error(const char *source, int lineno, const char *line, const char *cmd, const char *msg, const char *more, int col) pg_attribute_noreturn(); -extern bool strtoint64(const char *str, bool errorOK, int64 *pi); extern bool strtodouble(const char *str, bool errorOK, double *pd); #endif /* PGBENCH_H */ diff --git a/src/common/string.c b/src/common/string.c index c9b8482cb0..82cbee4c3d 100644 --- a/src/common/string.c +++ b/src/common/string.c @@ -21,6 +21,7 @@ #include "postgres_fe.h" #endif +#include "common/int.h" #include "common/string.h" @@ -58,6 +59,376 @@ strtoint(const char *pg_restrict str, char **pg_restrict endptr, int base) } +/* + * pg_strtoint16 + * + * Convert input string to a signed 16-bit integer. Allows any number of + * leading or trailing whitespace characters. + * + * NB: Accumulate input as a negative number, to deal with two's complement + * representation of the most negative number, which can't be represented as a + * positive number. + * + * The function returns immediately if the conversion failed with a status + * value to let the caller handle the error. On success, the result is + * stored in "*result". + */ +pg_strtoint_status +pg_strtoint16(const char *s, int16 *result) +{ + const char *ptr = s; + int16 tmp = 0; + bool neg = false; + + /* skip leading spaces */ + while (likely(*ptr) && isspace((unsigned char) *ptr)) + ptr++; + + /* handle sign */ + if (*ptr == '-') + { + ptr++; + neg = true; + } + else if (*ptr == '+') + ptr++; + + /* require at least one digit */ + if (unlikely(!isdigit((unsigned char) *ptr))) + return PG_STRTOINT_SYNTAX_ERROR; + + /* process digits, we know that we have one ahead per the last check */ + do + { + int8 digit = (*ptr++ - '0'); + + if (unlikely(pg_mul_s16_overflow(tmp, 10, &tmp)) || + unlikely(pg_sub_s16_overflow(tmp, digit, &tmp))) + return PG_STRTOINT_RANGE_ERROR; + } + while (isdigit((unsigned char) *ptr)); + + /* allow trailing whitespace, but not other trailing chars */ + while (isspace((unsigned char) *ptr)) + ptr++; + + if (unlikely(*ptr != '\0')) + return PG_STRTOINT_SYNTAX_ERROR; + + if (!neg) + { + /* could fail if input is most negative number */ + if (unlikely(tmp == PG_INT16_MIN)) + return PG_STRTOINT_RANGE_ERROR; + tmp = -tmp; + } + + *result = tmp; + return PG_STRTOINT_OK; +} + +/* + * pg_strtoint32 + * + * Convert input string to a signed 32-bit integer. Allows any number of + * leading or trailing whitespace characters. + * + * NB: Accumulate input as a negative number, to deal with two's complement + * representation of the most negative number, which can't be represented as a + * positive number. + * + * The function returns immediately if the conversion failed with a status + * value to let the caller handle the error. On success, the result is + * stored in "*result". + */ +pg_strtoint_status +pg_strtoint32(const char *s, int32 *result) +{ + const char *ptr = s; + int32 tmp = 0; + bool neg = false; + + /* skip leading spaces */ + while (likely(*ptr) && isspace((unsigned char) *ptr)) + ptr++; + + /* handle sign */ + if (*ptr == '-') + { + ptr++; + neg = true; + } + else if (*ptr == '+') + ptr++; + + /* require at least one digit */ + if (unlikely(!isdigit((unsigned char) *ptr))) + return PG_STRTOINT_SYNTAX_ERROR; + + /* process digits, we know that we have one ahead per the last check */ + do + { + int8 digit = (*ptr++ - '0'); + + if (unlikely(pg_mul_s32_overflow(tmp, 10, &tmp)) || + unlikely(pg_sub_s32_overflow(tmp, digit, &tmp))) + return PG_STRTOINT_RANGE_ERROR; + } + while (isdigit((unsigned char) *ptr)); + + /* allow trailing whitespace, but not other trailing chars */ + while (isspace((unsigned char) *ptr)) + ptr++; + + if (unlikely(*ptr != '\0')) + return PG_STRTOINT_SYNTAX_ERROR; + + if (!neg) + { + /* could fail if input is most negative number */ + if (unlikely(tmp == PG_INT32_MIN)) + return PG_STRTOINT_RANGE_ERROR; + tmp = -tmp; + } + + *result = tmp; + return PG_STRTOINT_OK; +} + +/* + * pg_strtoint64 + * + * Convert input string to a signed 32-bit integer. Allows any number of + * leading or trailing whitespace characters. + * + * NB: Accumulate input as a negative number, to deal with two's complement + * representation of the most negative number, which can't be represented as a + * positive number. + * + * The function returns immediately if the conversion failed with a status + * value to let the caller handle the error. On success, the result is + * stored in "*result". + */ +pg_strtoint_status +pg_strtoint64(const char *str, int64 *result) +{ + const char *ptr = str; + int64 tmp = 0; + bool neg = false; + + /* + * Do our own scan, rather than relying on sscanf which might be broken + * for long long. + * + * As INT64_MIN can't be stored as a positive 64 bit integer, accumulate + * value as a negative number. + */ + + /* skip leading spaces */ + while (isspace((unsigned char) *ptr)) + ptr++; + + /* handle sign */ + if (*ptr == '-') + { + ptr++; + neg = true; + } + else if (*ptr == '+') + ptr++; + + /* require at least one digit */ + if (unlikely(!isdigit((unsigned char) *ptr))) + return PG_STRTOINT_SYNTAX_ERROR; + + /* process digits, we know that we have one ahead per the last check */ + do + { + int64 digit = (*ptr++ - '0'); + + if (unlikely(pg_mul_s64_overflow(tmp, 10, &tmp)) || + unlikely(pg_sub_s64_overflow(tmp, digit, &tmp))) + return PG_STRTOINT_RANGE_ERROR; + } + while (isdigit((unsigned char) *ptr)); + + /* allow trailing whitespace, but not other trailing chars */ + while (isspace((unsigned char) *ptr)) + ptr++; + + if (unlikely(*ptr != '\0')) + return PG_STRTOINT_SYNTAX_ERROR; + + if (!neg) + { + if (unlikely(tmp == PG_INT64_MIN)) + return PG_STRTOINT_RANGE_ERROR; + tmp = -tmp; + } + + *result = tmp; + return PG_STRTOINT_OK; +} + +/* + * pg_strtouint16 + * + * Convert input string to an unsigned 16-bit integer. Allows any number of + * leading or trailing whitespace characters. + * + * The function returns immediately if the conversion failed with a status + * value to let the caller handle the error. On success, the result is + * stored in "*result". + */ +pg_strtoint_status +pg_strtouint16(const char *str, uint16 *result) +{ + const char *ptr = str; + uint16 tmp = 0; + + /* skip leading spaces */ + while (isspace((unsigned char) *ptr)) + ptr++; + + /* handle sign */ + if (*ptr == '+') + ptr++; + else if (unlikely(*ptr == '-')) + return PG_STRTOINT_SYNTAX_ERROR; + + /* require at least one digit */ + if (!unlikely(isdigit((unsigned char) *ptr))) + return PG_STRTOINT_SYNTAX_ERROR; + + /* process digits, we know that we have one ahead per the last check */ + do + { + uint8 digit = (*ptr++ - '0'); + + if (unlikely(pg_mul_u16_overflow(tmp, 10, &tmp)) || + unlikely(pg_add_u16_overflow(tmp, digit, &tmp))) + return PG_STRTOINT_RANGE_ERROR; + } + while (isdigit((unsigned char) *ptr)); + + /* allow trailing whitespace */ + while (isspace((unsigned char) *ptr)) + ptr++; + + /* but not other trailing chars */ + if (unlikely(*ptr != '\0')) + return PG_STRTOINT_SYNTAX_ERROR; + + *result = tmp; + return PG_STRTOINT_OK; +} + +/* + * pg_strtouint32 + * + * Convert input string to an unsigned 32-bit integer. Allows any number of + * leading or trailing whitespace characters. + * + * The function returns immediately if the conversion failed with a status + * value to let the caller handle the error. On success, the result is + * stored in "*result". + */ +pg_strtoint_status +pg_strtouint32(const char *str, uint32 *result) +{ + const char *ptr = str; + uint32 tmp = 0; + + /* skip leading spaces */ + while (isspace((unsigned char) *ptr)) + ptr++; + + /* handle sign */ + if (*ptr == '+') + ptr++; + else if (unlikely(*ptr == '-')) + return PG_STRTOINT_SYNTAX_ERROR; + + /* require at least one digit */ + if (!unlikely(isdigit((unsigned char) *ptr))) + return PG_STRTOINT_SYNTAX_ERROR; + + /* process digits, we know that we have one ahead per the last check */ + do + { + uint8 digit = (*ptr++ - '0'); + + if (unlikely(pg_mul_u32_overflow(tmp, 10, &tmp)) || + unlikely(pg_add_u32_overflow(tmp, digit, &tmp))) + return PG_STRTOINT_RANGE_ERROR; + } + while (isdigit((unsigned char) *ptr)); + + /* allow trailing whitespace */ + while (isspace((unsigned char) *ptr)) + ptr++; + + /* but not other trailing chars */ + if (unlikely(*ptr != '\0')) + return PG_STRTOINT_SYNTAX_ERROR; + + *result = tmp; + return PG_STRTOINT_OK; +} + +/* + * pg_strtouint64 + * + * Convert input string to an unsigned 64-bit integer. Allows any number of + * leading or trailing whitespace characters. + * + * The function returns immediately if the conversion failed with a status + * value to let the caller handle the error. On success, the result is + * stored in "*result". + */ +pg_strtoint_status +pg_strtouint64(const char *str, uint64 *result) +{ + const char *ptr = str; + uint64 tmp = 0; + + /* skip leading spaces */ + while (isspace((unsigned char) *ptr)) + ptr++; + + /* handle sign */ + if (*ptr == '+') + ptr++; + else if (unlikely(*ptr == '-')) + return PG_STRTOINT_SYNTAX_ERROR; + + /* require at least one digit */ + if (!unlikely(isdigit((unsigned char) *ptr))) + return PG_STRTOINT_SYNTAX_ERROR; + + /* process digits, we know that we have one ahead per the last check */ + do + { + uint64 digit = (*ptr++ - '0'); + + if (unlikely(pg_mul_u64_overflow(tmp, 10, &tmp)) || + unlikely(pg_add_u64_overflow(tmp, digit, &tmp))) + return PG_STRTOINT_RANGE_ERROR; + } + while (isdigit((unsigned char) *ptr)); + + /* allow trailing whitespace */ + while (isspace((unsigned char) *ptr)) + ptr++; + + /* but not other trailing chars */ + if (unlikely(*ptr != '\0')) + return PG_STRTOINT_SYNTAX_ERROR; + + *result = tmp; + return PG_STRTOINT_OK; +} + /* * pg_clean_ascii -- Replace any non-ASCII chars with a '?' char * diff --git a/src/include/common/string.h b/src/include/common/string.h index 94f653fdd7..eb0f273d82 100644 --- a/src/include/common/string.h +++ b/src/include/common/string.h @@ -11,8 +11,26 @@ #define COMMON_STRING_H extern bool pg_str_endswith(const char *str, const char *end); -extern int strtoint(const char *pg_restrict str, char **pg_restrict endptr, - int base); +extern int strtoint(const char *pg_restrict str, char **pg_restrict endptr, + int base); + +/* + * Set of routines for conversion from string to various integer types. + */ +typedef enum pg_strtoint_status +{ + PG_STRTOINT_OK, + PG_STRTOINT_SYNTAX_ERROR, + PG_STRTOINT_RANGE_ERROR +} pg_strtoint_status; + +extern pg_strtoint_status pg_strtoint16(const char *str, int16 *result); +extern pg_strtoint_status pg_strtoint32(const char *str, int32 *result); +extern pg_strtoint_status pg_strtoint64(const char *str, int64 *result); +extern pg_strtoint_status pg_strtouint16(const char *str, uint16 *result); +extern pg_strtoint_status pg_strtouint32(const char *str, uint32 *result); +extern pg_strtoint_status pg_strtouint64(const char *str, uint64 *result); + extern void pg_clean_ascii(char *str); extern int pg_strip_crlf(char *str); diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h index 937ddb7ef0..02e1c9ba03 100644 --- a/src/include/utils/builtins.h +++ b/src/include/utils/builtins.h @@ -15,6 +15,7 @@ #define BUILTINS_H #include "fmgr.h" +#include "common/string.h" #include "nodes/nodes.h" #include "utils/fmgrprotos.h" @@ -43,14 +44,17 @@ extern int namestrcmp(Name name, const char *str); /* numutils.c */ extern int32 pg_atoi(const char *s, int size, int c); -extern int16 pg_strtoint16(const char *s); -extern int32 pg_strtoint32(const char *s); +extern void pg_strtoint_error(pg_strtoint_status status, + const char *input, + const char *type) pg_attribute_noreturn(); +extern int16 pg_strtoint16_check(const char *s); +extern int32 pg_strtoint32_check(const char *s); +extern int64 pg_strtoint64_check(const char *s); extern void pg_itoa(int16 i, char *a); extern void pg_ltoa(int32 l, char *a); extern void pg_lltoa(int64 ll, char *a); extern char *pg_ltostr_zeropad(char *str, int32 value, int32 minwidth); extern char *pg_ltostr(char *str, int32 value); -extern uint64 pg_strtouint64(const char *str, char **endptr, int base); /* oid.c */ extern oidvector *buildoidvector(const Oid *oids, int n); diff --git a/src/include/utils/int8.h b/src/include/utils/int8.h deleted file mode 100644 index 4836095d93..0000000000 --- a/src/include/utils/int8.h +++ /dev/null @@ -1,25 +0,0 @@ -/*------------------------------------------------------------------------- - * - * int8.h - * Declarations for operations on 64-bit integers. - * - * - * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group - * Portions Copyright (c) 1994, Regents of the University of California - * - * src/include/utils/int8.h - * - * NOTES - * These data types are supported on all 64-bit architectures, and may - * be supported through libraries on some 32-bit machines. If your machine - * is not currently supported, then please try to make it so, then post - * patches to the postgresql.org hackers mailing list. - * - *------------------------------------------------------------------------- - */ -#ifndef INT8_H -#define INT8_H - -extern bool scanint8(const char *str, bool errorOK, int64 *result); - -#endif /* INT8_H */
signature.asc
Description: PGP signature