Current GUC units code has 2 problems:
- It requires case-sensiteive representation of units ("kB").
As the main point of units is to make configuring easier,
requiring byte-exact typing makes them unnecessarily fragile.
- In attempt to preserve maximum range of values for INT64_IS_BUSTED
systems, the code is written rather non-obvious way.
Attached patch replaces per-unit custom code with lookup table,
where each unit is represented as multiplier of base unit.
And it compares unit names case-insensitivaly.
It sacrifices some range on INT64_IS_BUSTED systems, but as the only promise
we offer them is that "Postgres may compile" I don't see it as problem.
In case people like the case-sensitivity, it can be restored and the patch
applied as plain cleanup.
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 95,111 ****
#define KB_PER_MB (1024)
#define KB_PER_GB (1024*1024)
#define MS_PER_S 1000
- #define S_PER_MIN 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 MIN_PER_D (60 * 24)
- #define S_PER_D (60 * 60 * 24)
#define MS_PER_D (1000 * 60 * 60 * 24)
/* XXX these should appear in other modules' header files */
extern bool Log_disconnections;
extern int CommitDelay;
--- 95,106 ----
*************** parse_bool(const char *value, bool *resu
*** 4115,4124 ****
--- 4110,4215 ----
}
return true;
}
+ #if BLCKSZ < 1024 || XLOG_BLCKSZ < 1024
+ #error BLCKSZ or XLOG_BLCKSZ less than 1K is not supported by GUC units code.
+ #endif
+
+ /*
+ * Accepted unit abbrevations.
+ *
+ * Units are represented in terms of base unit.
+ *
+ * Base unit for memory is 1 kbyte.
+ * Base unit for time is 1 ms.
+ *
+ * If several units of same type share common prefix, longer
+ * must come first. Does not apply if different types.
+ */
+ static const GucUnit unit_list [] = {
+ /* memory units */
+ { "kb", GUC_UNIT_MEMORY, 1 },
+ { "mb", GUC_UNIT_MEMORY, KB_PER_MB },
+ { "gb", GUC_UNIT_MEMORY, KB_PER_GB },
+ /* time units */
+ { "ms", GUC_UNIT_TIME, 1 },
+ { "s", GUC_UNIT_TIME, MS_PER_S },
+ { "min", GUC_UNIT_TIME, MS_PER_MIN },
+ { "h", GUC_UNIT_TIME, MS_PER_H },
+ { "d", GUC_UNIT_TIME, MS_PER_D },
+ /* end marker */
+ { NULL },
+ };
+
+ /* hints when unit parsing fails. keep them near unit list */
+ static const char *hintmsg_time = gettext_noop("Valid units for this
parameter are \"kB\", \"MB\", and \"GB\".");
+ static const char *hintmsg_mem = gettext_noop("Valid units for this parameter
are \"ms\", \"s\", \"min\", \"h\", and \"d\".");
+
+ /* returns guc_unit struct or NULL for error */
+ static const GucUnit *
+ lookup_unit(const char *s, int flags)
+ {
+ const GucUnit *unit;
+ for (unit = unit_list; unit->abbr; unit++)
+ {
+ if ((flags & unit->typemask) == 0)
+ continue;
+ if (pg_strncasecmp(s, unit->abbr, strlen(unit->abbr)) == 0)
+ return unit;
+ }
+ return NULL;
+ }
+
+ /* returns final value */
+ static int64
+ apply_unit(const GucUnit *unit, int flags, int64 val, char **errstr)
+ {
+ /* divider to get real units from base units */
+ int div;
+ /* final result */
+ int64 res;
+
+ switch (flags & unit->typemask)
+ {
+ /* memory values */
+ case GUC_UNIT_KB:
+ div = 1;
+ break;
+ case GUC_UNIT_BLOCKS:
+ div = BLCKSZ / 1024;
+ break;
+ case GUC_UNIT_XBLOCKS:
+ div = XLOG_BLCKSZ / 1024;
+ break;
+ /* time values */
+ case GUC_UNIT_MS:
+ div = 1;
+ break;
+ case GUC_UNIT_S:
+ div = MS_PER_S;
+ break;
+ case GUC_UNIT_MIN:
+ div = MS_PER_MIN;
+ break;
+ default:
+ /* can happen only if new GUC_UNIT_* is introduced */
+ *errstr = gettext_noop("BUG: apply_unit not updated");
+ return val;
+ }
+
+ res = val * unit->multiplier / div;
+
+ #ifdef INT64_IS_BUSTED
+ /* check for overflow */
+ if (res < (val / div) * unit->multiplier)
+ *errstr = gettext_noop("unit overflow");
+ #endif
+
+ return res;
+ }
/*
* Try to parse value as an integer. The accepted formats are the
* usual decimal, octal, or hexadecimal formats, optionally followed by
* a unit name if "flags" indicates a unit is allowed.
*************** parse_bool(const char *value, bool *resu
*** 4131,4140 ****
--- 4222,4233 ----
bool
parse_int(const char *value, int *result, int flags, const char **hintmsg)
{
int64 val;
char *endptr;
+ char *errmsg = NULL;
+ const GucUnit *unit;
/* To suppress compiler warnings, always set output params */
if (result)
*result = 0;
if (hintmsg)
*************** parse_int(const char *value, int *result
*** 4159,4311 ****
endptr++;
/* Handle possible unit */
if (*endptr != '\0')
{
! /*
! * Note: the multiple-switch coding technique here is a bit
tedious,
! * but seems necessary to avoid intermediate-value overflows.
! *
! * 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)
{
! /* Set hint for use if no match or trailing garbage */
! if (hintmsg)
! *hintmsg = gettext_noop("Valid units for this
parameter are \"kB\", \"MB\", and \"GB\".");
!
! #if BLCKSZ < 1024 || BLCKSZ > (1024*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
!
! if (strncmp(endptr, "kB", 2) == 0)
! {
! endptr += 2;
! switch (flags & GUC_UNIT_MEMORY)
! {
! case GUC_UNIT_BLOCKS:
! val /= (BLCKSZ / 1024);
! break;
! case GUC_UNIT_XBLOCKS:
! val /= (XLOG_BLCKSZ / 1024);
! 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;
! }
! }
}
- else if (flags & GUC_UNIT_TIME)
- {
- /* 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\".");
! if (strncmp(endptr, "ms", 2) == 0)
! {
! 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 (strncmp(endptr, "s", 1) == 0)
! {
! 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 (strncmp(endptr, "min", 3) == 0)
! {
! endptr += 3;
! switch (flags & GUC_UNIT_TIME)
! {
! case GUC_UNIT_MS:
! val *= MS_PER_MIN;
! break;
! case GUC_UNIT_S:
! val *= S_PER_MIN;
! break;
! }
! }
! else if (strncmp(endptr, "h", 1) == 0)
! {
! 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;
! }
! }
! else if (strncmp(endptr, "d", 1) == 0)
{
! endptr += 1;
! switch (flags & GUC_UNIT_TIME)
! {
! case GUC_UNIT_MS:
! val *= MS_PER_D;
! break;
! case GUC_UNIT_S:
! val *= S_PER_D;
! break;
! case GUC_UNIT_MIN:
! val *= MIN_PER_D;
! break;
! }
}
}
/* allow whitespace after unit */
while (isspace((unsigned char) *endptr))
--- 4252,4281 ----
endptr++;
/* Handle possible unit */
if (*endptr != '\0')
{
! /* prepare hint msg */
! if (hintmsg)
{
! if (flags & GUC_UNIT_MEMORY)
! *hintmsg = hintmsg_mem;
! else if (flags & GUC_UNIT_TIME)
! *hintmsg = hintmsg_time;
}
! /* apply unit */
! unit = lookup_unit(endptr, flags);
! if (unit)
! {
! endptr += strlen(unit->abbr);
! val = apply_unit(unit, flags, val, &errmsg);
! if (errmsg)
{
! if (hintmsg)
! *hintmsg = errmsg;
! return false;
}
}
/* allow whitespace after unit */
while (isspace((unsigned char) *endptr))
*** a/src/include/utils/guc_tables.h
--- b/src/include/utils/guc_tables.h
*************** struct config_string
*** 210,219 ****
--- 210,232 ----
GucShowHook show_hook;
/* variable fields, initialized at runtime: */
char *reset_val;
};
+ /*
+ * Units are represented in terms of base unit.
+ * Base unit for memory is 1 kbyte.
+ * Base unit for time is 1 ms.
+ *
+ * typemask is either GUC_UNIT_MEMORY or GUC_UNIT_TIME.
+ */
+ typedef struct guc_unit {
+ const char *abbr;
+ int typemask;
+ unsigned int multiplier;
+ } GucUnit;
+
struct config_enum
{
struct config_generic gen;
/* constant fields, must be set correctly in initial value: */
int *variable;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
