From 2baf4efe09f65d8e8cac32fb882ab9f0fd601574 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 28 Jul 2003 19:31:32 +0000 Subject: [PATCH] Code review for recent GUC changes --- try to make it less obvious that these things were added at different times by different people ;-). Includes Aizaz Ahmed's patch to remove duplicate array in help_config.c. --- src/backend/utils/misc/guc.c | 193 ++++++++++++++++----------- src/backend/utils/misc/help_config.c | 18 +-- src/include/utils/guc.h | 50 +++---- src/include/utils/guc_tables.h | 23 ++-- 4 files changed, 152 insertions(+), 132 deletions(-) diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index a93399336f..d6ed9ba462 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -10,7 +10,7 @@ * Written by Peter Eisentraut . * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v 1.142 2003/07/28 16:22:02 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v 1.143 2003/07/28 19:31:32 tgl Exp $ * *-------------------------------------------------------------------- */ @@ -155,49 +155,6 @@ static char *timezone_string; static char *XactIsoLevel_string; -/* - * Used for pg_settings. Keep in sync with config_type enum in guc_tables.h - */ -static char *config_type_name[] = -{ - "bool", - "integer", - "real", - "string" -}; - -/* - * Used for pg_settings. Keep in sync with GucContext enum in guc.h - */ -static char *GucContextName[] = -{ - "internal", - "postmaster", - "sighup", - "backend", - "super-user", - "userlimit", - "user" -}; - -/* - * Used for pg_settings. Keep in sync with GucSource enum in guc.h - */ -static char *GucSourceName[] = -{ - "default", - "environment variable", - "configuration file", - "command line", - "userstart", - "database", - "user", - "client", - "override", - "session" -}; - - /* Macros for freeing malloc'd pointers only if appropriate to do so */ /* Some of these tests are probably redundant, but be safe ... */ #define SET_STRING_VARIABLE(rec, newval) \ @@ -239,46 +196,126 @@ static char *GucSourceName[] = /* - * The display name for each of the groupings defined in enum config_group - * This array needs to be kept in sync with enum config_group. - * This array however needs to be NULL terminated. + * Displayable names for context types (enum GucContext) + * + * Note: these strings are deliberately not localized. */ -const char *const config_group_names[] = { +const char * const GucContext_Names[] = +{ + /* PGC_INTERNAL */ "internal", + /* PGC_POSTMASTER */ "postmaster", + /* PGC_SIGHUP */ "sighup", + /* PGC_BACKEND */ "backend", + /* PGC_SUSET */ "superuser", + /* PGC_USERLIMIT */ "userlimit", + /* PGC_USERSET */ "user" +}; + +/* + * Displayable names for source types (enum GucSource) + * + * Note: these strings are deliberately not localized. + */ +const char * const GucSource_Names[] = +{ + /* PGC_S_DEFAULT */ "default", + /* PGC_S_ENV_VAR */ "environment variable", + /* PGC_S_FILE */ "configuration file", + /* PGC_S_ARGV */ "command line", + /* PGC_S_UNPRIVILEGED */ "unprivileged", + /* PGC_S_DATABASE */ "database", + /* PGC_S_USER */ "user", + /* PGC_S_CLIENT */ "client", + /* PGC_S_OVERRIDE */ "override", + /* PGC_S_SESSION */ "session" +}; + +/* + * Displayable names for the groupings defined in enum config_group + */ +const char *const config_group_names[] = +{ + /* UNGROUPED */ gettext_noop("Ungrouped"), + /* CONN_AUTH */ gettext_noop("Connections & Authentication"), + /* CONN_AUTH_SETTINGS */ gettext_noop("Connections & Authentication / Connection Settings"), + /* CONN_AUTH_SECURITY */ gettext_noop("Connections & Authentication / Security & Authentication"), + /* RESOURCES */ gettext_noop("Resource Usage"), + /* RESOURCES_MEM */ gettext_noop("Resource Usage / Memory"), + /* RESOURCES_FSM */ gettext_noop("Resource Usage / Free Space Map"), + /* RESOURCES_KERNEL */ gettext_noop("Resource Usage / Kernel Resources"), + /* WAL */ gettext_noop("Write Ahead Log"), + /* WAL_SETTINGS */ gettext_noop("Write Ahead Log / Settings"), + /* WAL_CHECKPOINTS */ gettext_noop("Write Ahead Log / Checkpoints"), + /* QUERY_TUNING */ gettext_noop("Query Tuning"), + /* QUERY_TUNING_METHOD */ gettext_noop("Query Tuning / Planner Method Enabling"), + /* QUERY_TUNING_COST */ gettext_noop("Query Tuning / Planner Cost Constants"), + /* QUERY_TUNING_GEQO */ gettext_noop("Query Tuning / Genetic Query Optimizer"), + /* QUERY_TUNING_OTHER */ gettext_noop("Query Tuning / Other Planner Options"), + /* LOGGING */ gettext_noop("Reporting & Logging"), + /* LOGGING_SYSLOG */ gettext_noop("Reporting & Logging / Syslog"), + /* LOGGING_WHEN */ gettext_noop("Reporting & Logging / When To Log"), + /* LOGGING_WHAT */ gettext_noop("Reporting & Logging / What To Log"), + /* STATS */ gettext_noop("Statistics"), + /* STATS_MONITORING */ gettext_noop("Statistics / Monitoring"), + /* STATS_COLLECTOR */ gettext_noop("Statistics / Query & Index Statistics Collector"), + /* CLIENT_CONN */ gettext_noop("Client Connection Defaults"), + /* CLIENT_CONN_STATEMENT */ gettext_noop("Client Connection Defaults / Statement Behavior"), + /* CLIENT_CONN_LOCALE */ gettext_noop("Client Connection Defaults / Locale and Formatting"), + /* CLIENT_CONN_OTHER */ gettext_noop("Client Connection Defaults / Other Defaults"), + /* LOCK_MANAGEMENT */ gettext_noop("Lock Management"), + /* COMPAT_OPTIONS */ gettext_noop("Version & Platform Compatibility"), + /* COMPAT_OPTIONS_PREVIOUS */ gettext_noop("Version & Platform Compatibility / Previous Postgres Versions"), + /* COMPAT_OPTIONS_CLIENT */ gettext_noop("Version & Platform Compatibility / Other Platforms & Clients"), + /* DEVELOPER_OPTIONS */ gettext_noop("Developer Options"), + /* help_config wants this array to be null-terminated */ NULL }; +/* + * Displayable names for GUC variable types (enum config_type) + * + * Note: these strings are deliberately not localized. + */ +const char * const config_type_names[] = +{ + /* PGC_BOOL */ "bool", + /* PGC_INT */ "integer", + /* PGC_REAL */ "real", + /* PGC_STRING */ "string" +}; + /* * Contents of GUC tables @@ -2509,9 +2546,9 @@ set_config_option(const char *name, const char *value, name))); return false; } - /* Limit non-super user changes */ + /* Limit non-superuser changes */ if (record->context == PGC_USERLIMIT && - source > PGC_S_USERSTART && + source > PGC_S_UNPRIVILEGED && newval < conf->session_val && !superuser()) { @@ -2522,10 +2559,10 @@ set_config_option(const char *name, const char *value, errhint("Must be superuser to change this value to false."))); return false; } - /* Allow admin to override non-super user setting */ + /* Allow admin to override non-superuser setting */ if (record->context == PGC_USERLIMIT && - source < PGC_S_USERSTART && - record->session_source > PGC_S_USERSTART && + source < PGC_S_UNPRIVILEGED && + record->session_source > PGC_S_UNPRIVILEGED && newval > conf->session_val && !superuser()) DoIt = DoIt_orig; @@ -2605,9 +2642,9 @@ set_config_option(const char *name, const char *value, newval, name, conf->min, conf->max))); return false; } - /* Limit non-super user changes */ + /* Limit non-superuser changes */ if (record->context == PGC_USERLIMIT && - source > PGC_S_USERSTART && + source > PGC_S_UNPRIVILEGED && conf->session_val != 0 && (newval > conf->session_val || newval == 0) && !superuser()) @@ -2619,10 +2656,10 @@ set_config_option(const char *name, const char *value, errhint("Must be superuser to increase this value or set it to zero."))); return false; } - /* Allow admin to override non-super user setting */ + /* Allow admin to override non-superuser setting */ if (record->context == PGC_USERLIMIT && - source < PGC_S_USERSTART && - record->session_source > PGC_S_USERSTART && + source < PGC_S_UNPRIVILEGED && + record->session_source > PGC_S_UNPRIVILEGED && newval < conf->session_val && !superuser()) DoIt = DoIt_orig; @@ -2702,9 +2739,9 @@ set_config_option(const char *name, const char *value, newval, name, conf->min, conf->max))); return false; } - /* Limit non-super user changes */ + /* Limit non-superuser changes */ if (record->context == PGC_USERLIMIT && - source > PGC_S_USERSTART && + source > PGC_S_UNPRIVILEGED && newval > conf->session_val && !superuser()) { @@ -2715,10 +2752,10 @@ set_config_option(const char *name, const char *value, errhint("Must be superuser to increase this value."))); return false; } - /* Allow admin to override non-super user setting */ + /* Allow admin to override non-superuser setting */ if (record->context == PGC_USERLIMIT && - source < PGC_S_USERSTART && - record->session_source > PGC_S_USERSTART && + source < PGC_S_UNPRIVILEGED && + record->session_source > PGC_S_UNPRIVILEGED && newval < conf->session_val && !superuser()) DoIt = DoIt_orig; @@ -2791,15 +2828,18 @@ set_config_option(const char *name, const char *value, return false; } - if (*conf->variable) + if (record->context == PGC_USERLIMIT && + *conf->variable) { int old_int_value, new_int_value; - /* Limit non-super user changes */ - assign_msglvl(&old_int_value, conf->reset_val, true, interactive); - assign_msglvl(&new_int_value, newval, true, interactive); - if (record->context == PGC_USERLIMIT && - source > PGC_S_USERSTART && + /* all USERLIMIT strings are message levels */ + assign_msglvl(&old_int_value, conf->reset_val, + true, interactive); + assign_msglvl(&new_int_value, newval, + true, interactive); + /* Limit non-superuser changes */ + if (source > PGC_S_UNPRIVILEGED && new_int_value > old_int_value && !superuser()) { @@ -2810,14 +2850,13 @@ set_config_option(const char *name, const char *value, errhint("Must be superuser to increase this value."))); return false; } - /* Allow admin to override non-super user setting */ - if (record->context == PGC_USERLIMIT && - source < PGC_S_USERSTART && - record->session_source > PGC_S_USERSTART && + /* Allow admin to override non-superuser setting */ + if (source < PGC_S_UNPRIVILEGED && + record->session_source > PGC_S_UNPRIVILEGED && newval < conf->session_val && !superuser()) DoIt = DoIt_orig; - } + } } else if (conf->reset_val) { @@ -3389,13 +3428,13 @@ GetConfigOptionByNum(int varnum, const char **values, bool *noshow) values[1] = _ShowOption(conf); /* context */ - values[2] = GucContextName[conf->context]; + values[2] = GucContext_Names[conf->context]; /* vartype */ - values[3] = config_type_name[conf->vartype]; + values[3] = config_type_names[conf->vartype]; /* source */ - values[4] = GucSourceName[conf->source]; + values[4] = GucSource_Names[conf->source]; /* now get the type specifc attributes */ switch (conf->vartype) diff --git a/src/backend/utils/misc/help_config.c b/src/backend/utils/misc/help_config.c index 17c3b2488d..d5d9736e59 100644 --- a/src/backend/utils/misc/help_config.c +++ b/src/backend/utils/misc/help_config.c @@ -25,7 +25,7 @@ * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/utils/misc/help_config.c,v 1.2 2003/07/09 17:57:47 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/utils/misc/help_config.c,v 1.3 2003/07/28 19:31:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -132,20 +132,6 @@ static void printGenericFoot(struct config_generic structToPrint); static void printMixedStruct(mixedStruct * structToPrint); static bool displayStruct(mixedStruct * structToDisplay); -/* - * This array contains the display names for each of the GucContexts available - * - * Note: these strings are deliberately not localized. - */ -static const char *const GucContext_names[] = { - "INTERNAL", - "POSTMASTER", - "SIGHUP", - "BACKEND", - "SUSET", - "USERLIMIT", - "USERSET" -}; /* * Reads in the the command line options and sets the state of the program @@ -406,7 +392,7 @@ printGenericHead(struct config_generic structToPrint) { printf(gettext(GENERIC_FORMAT[outFormat]), structToPrint.name, - GucContext_names[structToPrint.context], + GucContext_Names[structToPrint.context], gettext(config_group_names[structToPrint.group])); } diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index b7c1b13d0f..979b5171e8 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -7,7 +7,7 @@ * Copyright 2000-2003 by PostgreSQL Global Development Group * Written by Peter Eisentraut . * - * $Id: guc.h,v 1.37 2003/07/28 16:22:13 momjian Exp $ + * $Id: guc.h,v 1.38 2003/07/28 19:31:32 tgl Exp $ *-------------------------------------------------------------------- */ #ifndef GUC_H @@ -49,21 +49,19 @@ * we don't yet know if the user is a superuser. * * USERLIMIT options can only be manipulated in certain ways by - * non-super users. + * non-superusers. * * USERSET options can be set by anyone any time. - * - * Keep in sync with GucContextName in guc.c */ typedef enum { - PGC_INTERNAL = 0, - PGC_POSTMASTER = 1, - PGC_SIGHUP = 2, - PGC_BACKEND = 3, - PGC_SUSET = 4, - PGC_USERLIMIT = 5, - PGC_USERSET = 6 + PGC_INTERNAL, + PGC_POSTMASTER, + PGC_SIGHUP, + PGC_BACKEND, + PGC_SUSET, + PGC_USERLIMIT, + PGC_USERSET } GucContext; /* @@ -76,24 +74,21 @@ typedef enum * as the current value. Note that source == PGC_S_OVERRIDE should be * used when setting a PGC_INTERNAL option. * - * Keep in sync with GucSourceName in guc.c + * PGC_S_UNPRIVILEGED isn't actually a source value, but the dividing line + * between privileged and unprivileged sources for USERLIMIT purposes. */ typedef enum { - PGC_S_DEFAULT = 0, /* wired-in default */ - PGC_S_ENV_VAR = 1, /* postmaster environment variable */ - PGC_S_FILE = 2, /* postgresql.conf */ - PGC_S_ARGV = 3, /* postmaster command line */ - PGC_S_USERSTART=4, /* - * Settings below are controlled by users. - * This is used by PGC_USERLIMT to prevent - * non-super users from changing certain settings. - */ - PGC_S_DATABASE = 5, /* per-database setting */ - PGC_S_USER = 6, /* per-user setting */ - PGC_S_CLIENT = 7, /* from client connection request */ - PGC_S_OVERRIDE = 8, /* special case to forcibly set default */ - PGC_S_SESSION = 9 /* SET command */ + PGC_S_DEFAULT, /* wired-in default */ + PGC_S_ENV_VAR, /* postmaster environment variable */ + PGC_S_FILE, /* postgresql.conf */ + PGC_S_ARGV, /* postmaster command line */ + PGC_S_UNPRIVILEGED, /* dividing line for USERLIMIT */ + PGC_S_DATABASE, /* per-database setting */ + PGC_S_USER, /* per-user setting */ + PGC_S_CLIENT, /* from client connection request */ + PGC_S_OVERRIDE, /* special case to forcibly set default */ + PGC_S_SESSION /* SET command */ } GucSource; /* GUC vars that are actually declared in guc.c, rather than elsewhere */ @@ -117,6 +112,7 @@ extern bool Australian_timezones; extern int log_min_error_statement; extern int log_min_messages; extern int client_min_messages; +extern int log_min_duration_statement; extern void SetConfigOption(const char *name, const char *value, @@ -154,6 +150,4 @@ void write_nondefault_variables(GucContext context); void read_nondefault_variables(void); #endif -extern int log_min_duration_statement; - #endif /* GUC_H */ diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h index 564df0da25..b1abc0b107 100644 --- a/src/include/utils/guc_tables.h +++ b/src/include/utils/guc_tables.h @@ -7,7 +7,7 @@ * * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group * - * $Id: guc_tables.h,v 1.3 2003/07/28 16:22:16 momjian Exp $ + * $Id: guc_tables.h,v 1.4 2003/07/28 19:31:32 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -15,9 +15,7 @@ #define GUC_TABLES 1 /* - * Groupings to help organize all the run-time options for display. - * - * Keep this in sync with config_group_names[] in guc.c. + * Groupings to help organize all the run-time options for display */ enum config_group { @@ -55,18 +53,15 @@ enum config_group DEVELOPER_OPTIONS }; - /* * GUC supports these types of variables: - * - * Keep in sync with config_type_name in guc.c */ enum config_type { - PGC_BOOL = 0, - PGC_INT = 1, - PGC_REAL = 2, - PGC_STRING = 3 + PGC_BOOL, + PGC_INT, + PGC_REAL, + PGC_STRING }; /* @@ -171,7 +166,13 @@ struct config_string char *tentative_val; }; +/* constant tables corresponding to enums above and in guc.h */ extern const char * const config_group_names[]; +extern const char * const config_type_names[]; +extern const char * const GucContext_Names[]; +extern const char * const GucSource_Names[]; + +/* the current set of variables */ extern struct config_generic **guc_variables; extern int num_guc_variables;