Skip to content

P
Postgres FD Implementation
Commit aa55d055 authored by Tom Lane's avatar Tom Lane
Browse files
Options

Provide a HINT listing the allowed unit names when a GUC variable seems to 

contain a wrong unit specification, per discussion.
In passing, fix the code to avoid unnecessary integer overflows when
converting units, and to detect overflows when they do occur.
parent 9f6aacd9
Show whitespace changes
Inline Side-by-side
Showing with 194 additions and 98 deletions
src/backend/utils/misc/guc.c
View file @ aa55d055
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
* Written by Peter Eisentraut <peter_e@gmx.net>. * Written by Peter Eisentraut <peter_e@gmx.net>.
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.400 2007/06/20 18:31:39 tgl Exp $ * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.401 2007/06/21 18:14:21 tgl Exp $
* *
*-------------------------------------------------------------------- *--------------------------------------------------------------------
*/ */
...@@ -94,8 +94,13 @@ ...@@ -94,8 +94,13 @@
#define KB_PER_GB (1024*1024) #define KB_PER_GB (1024*1024)
#define MS_PER_S 1000 #define MS_PER_S 1000
#define S_PER_MIN 60
#define MS_PER_MIN (1000 * 60) #define MS_PER_MIN (1000 * 60)
#define MIN_PER_H 60
#define S_PER_H (60 * 60)
#define MS_PER_H (1000 * 60 * 60) #define MS_PER_H (1000 * 60 * 60)
#define MIN_PER_D (60 * 24)
#define S_PER_D (60 * 60 * 24)
#define MS_PER_D (1000 * 60 * 60 * 24) #define MS_PER_D (1000 * 60 * 60 * 24)
/* XXX these should appear in other modules' header files */ /* XXX these should appear in other modules' header files */
...@@ -3783,50 +3788,71 @@ parse_bool(const char *value, bool *result) ...@@ -3783,50 +3788,71 @@ parse_bool(const char *value, bool *result)
/* /*
* Try to parse value as an integer. The accepted formats are the * Try to parse value as an integer. The accepted formats are the
* usual decimal, octal, or hexadecimal formats. If the string parses * usual decimal, octal, or hexadecimal formats, optionally followed by
* okay, return true, else false. If result is not NULL, return the * a unit name if "flags" indicates a unit is allowed.
* value there. *
* If the string parses okay, return true, else false.
* If okay and result is not NULL, return the value in *result.
* If not okay and hintmsg is not NULL, *hintmsg is set to a suitable
* HINT message, or NULL if no hint provided.
*/ */
static bool static bool
parse_int(const char *value, int *result, int flags) parse_int(const char *value, int *result, int flags, const char **hintmsg)
{ {
long val; int64 val;
char *endptr; char *endptr;
/* To suppress compiler warnings, always set output params */
if (result)
*result = 0;
if (hintmsg)
*hintmsg = NULL;
/* We assume here that int64 is at least as wide as long */
errno = 0; errno = 0;
val = strtol(value, &endptr, 0); val = strtol(value, &endptr, 0);
if ((flags & GUC_UNIT_MEMORY) && endptr != value) if (endptr == value)
return false; /* no HINT for integer syntax error */
if (errno == ERANGE || val != (int64) ((int32) val))
{ {
bool used = false; if (hintmsg)
*hintmsg = gettext_noop("Value exceeds integer range.");
return false;
}
while (*endptr == ' ') /* allow whitespace between integer and unit */
while (isspace((unsigned char) *endptr))
endptr++; endptr++;
if (strcmp(endptr, "kB") == 0) /* Handle possible unit */
{ if (*endptr != '\0')
used = true;
endptr += 2;
}
else if (strcmp(endptr, "MB") == 0)
{ {
val *= KB_PER_MB; /*
used = true; * Note: the multiple-switch coding technique here is a bit tedious,
endptr += 2; * but seems necessary to avoid intermediate-value overflows.
} *
else if (strcmp(endptr, "GB") == 0) * If INT64_IS_BUSTED (ie, it's really int32) we will fail to detect
* overflow due to units conversion, but there are few enough such
* machines that it does not seem worth trying to be smarter.
*/
if (flags & GUC_UNIT_MEMORY)
{ {
val *= KB_PER_GB; /* Set hint for use if no match or trailing garbage */
used = true; if (hintmsg)
endptr += 2; *hintmsg = gettext_noop("Valid units for this parameter are \"kB\", \"MB\", and \"GB\".");
}
#if BLCKSZ < 1024 #if BLCKSZ < 1024 || BLCKSZ > (1024*1024)
#error BLCKSZ must be >= 1024 #error BLCKSZ must be between 1KB and 1MB
#endif
#if XLOG_BLCKSZ < 1024 || XLOG_BLCKSZ > (1024*1024)
#error XLOG_BLCKSZ must be between 1KB and 1MB
#endif #endif
if (used) if (strncmp(endptr, "kB", 2) == 0)
{ {
endptr += 2;
switch (flags & GUC_UNIT_MEMORY) switch (flags & GUC_UNIT_MEMORY)
{ {
case GUC_UNIT_BLOCKS: case GUC_UNIT_BLOCKS:
...@@ -3837,70 +3863,134 @@ parse_int(const char *value, int *result, int flags) ...@@ -3837,70 +3863,134 @@ parse_int(const char *value, int *result, int flags)
break; break;
} }
} }
else if (strncmp(endptr, "MB", 2) == 0)
{
endptr += 2;
switch (flags & GUC_UNIT_MEMORY)
{
case GUC_UNIT_KB:
val *= KB_PER_MB;
break;
case GUC_UNIT_BLOCKS:
val *= KB_PER_MB / (BLCKSZ / 1024);
break;
case GUC_UNIT_XBLOCKS:
val *= KB_PER_MB / (XLOG_BLCKSZ / 1024);
break;
}
}
else if (strncmp(endptr, "GB", 2) == 0)
{
endptr += 2;
switch (flags & GUC_UNIT_MEMORY)
{
case GUC_UNIT_KB:
val *= KB_PER_GB;
break;
case GUC_UNIT_BLOCKS:
val *= KB_PER_GB / (BLCKSZ / 1024);
break;
case GUC_UNIT_XBLOCKS:
val *= KB_PER_GB / (XLOG_BLCKSZ / 1024);
break;
} }
}
if ((flags & GUC_UNIT_TIME) && endptr != value) }
else if (flags & GUC_UNIT_TIME)
{ {
bool used = false; /* Set hint for use if no match or trailing garbage */
if (hintmsg)
*hintmsg = gettext_noop("Valid units for this parameter are \"ms\", \"s\", \"min\", \"h\", and \"d\".");
while (*endptr == ' ') if (strncmp(endptr, "ms", 2) == 0)
endptr++;
if (strcmp(endptr, "ms") == 0)
{ {
used = true;
endptr += 2; endptr += 2;
switch (flags & GUC_UNIT_TIME)
{
case GUC_UNIT_S:
val /= MS_PER_S;
break;
case GUC_UNIT_MIN:
val /= MS_PER_MIN;
break;
}
} }
else if (strcmp(endptr, "s") == 0) else if (strncmp(endptr, "s", 1) == 0)
{ {
val *= MS_PER_S;
used = true;
endptr += 1; endptr += 1;
switch (flags & GUC_UNIT_TIME)
{
case GUC_UNIT_MS:
val *= MS_PER_S;
break;
case GUC_UNIT_MIN:
val /= S_PER_MIN;
break;
}
} }
else if (strcmp(endptr, "min") == 0) else if (strncmp(endptr, "min", 3) == 0)
{ {
val *= MS_PER_MIN;
used = true;
endptr += 3; endptr += 3;
} switch (flags & GUC_UNIT_TIME)
else if (strcmp(endptr, "h") == 0)
{ {
val *= MS_PER_H; case GUC_UNIT_MS:
used = true; val *= MS_PER_MIN;
endptr += 1; break;
case GUC_UNIT_S:
val *= S_PER_MIN;
break;
}
} }
else if (strcmp(endptr, "d") == 0) else if (strncmp(endptr, "h", 1) == 0)
{ {
val *= MS_PER_D;
used = true;
endptr += 1; endptr += 1;
switch (flags & GUC_UNIT_TIME)
{
case GUC_UNIT_MS:
val *= MS_PER_H;
break;
case GUC_UNIT_S:
val *= S_PER_H;
break;
case GUC_UNIT_MIN:
val *= MIN_PER_H;
break;
} }
}
if (used) else if (strncmp(endptr, "d", 1) == 0)
{ {
endptr += 1;
switch (flags & GUC_UNIT_TIME) switch (flags & GUC_UNIT_TIME)
{ {
case GUC_UNIT_MS:
val *= MS_PER_D;
break;
case GUC_UNIT_S: case GUC_UNIT_S:
val /= MS_PER_S; val *= S_PER_D;
break; break;
case GUC_UNIT_MIN: case GUC_UNIT_MIN:
val /= MS_PER_MIN; val *= MIN_PER_D;
break; break;
} }
} }
} }
if (endptr == value || *endptr != '\0' || errno == ERANGE /* allow whitespace after unit */
#ifdef HAVE_LONG_INT_64 while (isspace((unsigned char) *endptr))
/* if long > 32 bits, check for overflow of int4 */ endptr++;
|| val != (long) ((int32) val)
#endif if (*endptr != '\0')
) return false; /* appropriate hint, if any, already set */
/* Check for overflow due to units conversion */
if (val != (int64) ((int32) val))
{ {
if (result) if (hintmsg)
*result = 0; /* suppress compiler warning */ *hintmsg = gettext_noop("Value exceeds integer range.");
return false; return false;
} }
}
if (result) if (result)
*result = (int) val; *result = (int) val;
return true; return true;
...@@ -4243,12 +4333,15 @@ set_config_option(const char *name, const char *value, ...@@ -4243,12 +4333,15 @@ set_config_option(const char *name, const char *value,
if (value) if (value)
{ {
if (!parse_int(value, &newval, conf->gen.flags)) const char *hintmsg;
if (!parse_int(value, &newval, conf->gen.flags, &hintmsg))
{ {
ereport(elevel, ereport(elevel,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE), (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("parameter \"%s\" requires an integer value", errmsg("invalid value for parameter \"%s\": \"%s\"",
name))); name, value),
hintmsg ? errhint(hintmsg) : 0));
return false; return false;
} }
if (newval < conf->min || newval > conf->max) if (newval < conf->min || newval > conf->max)
...@@ -5674,21 +5767,24 @@ is_newvalue_equal(struct config_generic * record, const char *newvalue) ...@@ -5674,21 +5767,24 @@ is_newvalue_equal(struct config_generic * record, const char *newvalue)
struct config_bool *conf = (struct config_bool *) record; struct config_bool *conf = (struct config_bool *) record;
bool newval; bool newval;
return parse_bool(newvalue, &newval) && *conf->variable == newval; return parse_bool(newvalue, &newval)
&& *conf->variable == newval;
} }
case PGC_INT: case PGC_INT:
{ {
struct config_int *conf = (struct config_int *) record; struct config_int *conf = (struct config_int *) record;
int newval; int newval;
return parse_int(newvalue, &newval, record->flags) && *conf->variable == newval; return parse_int(newvalue, &newval, record->flags, NULL)
&& *conf->variable == newval;
} }
case PGC_REAL: case PGC_REAL:
{ {
struct config_real *conf = (struct config_real *) record; struct config_real *conf = (struct config_real *) record;
double newval; double newval;
return parse_real(newvalue, &newval) && *conf->variable == newval; return parse_real(newvalue, &newval)
&& *conf->variable == newval;
} }
case PGC_STRING: case PGC_STRING:
{ {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment