diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l index b43dec4a66..b2f04e512a 100644 --- a/src/backend/utils/misc/guc-file.l +++ b/src/backend/utils/misc/guc-file.l @@ -118,11 +118,11 @@ ProcessConfigFile(GucContext context) bool error = false; bool apply = false; int elevel; + const char *ConfFileWithError; ConfigVariable *item, - *head, - *tail; + *head, + *tail; int i; - char *ErrorConfFile = ConfigFileName; /* * Config files are processed on startup (by the postmaster only) @@ -138,6 +138,7 @@ ProcessConfigFile(GucContext context) elevel = IsUnderPostmaster ? DEBUG2 : LOG; /* Parse the main config file into a list of option names and values */ + ConfFileWithError = ConfigFileName; head = tail = NULL; if (!ParseConfigFile(ConfigFileName, NULL, true, 0, elevel, &head, &tail)) @@ -160,25 +161,23 @@ ProcessConfigFile(GucContext context) { /* Syntax error(s) detected in the file, so bail out */ error = true; - ErrorConfFile = PG_AUTOCONF_FILENAME; + ConfFileWithError = PG_AUTOCONF_FILENAME; goto cleanup_list; } } else { + /* + * If DataDir is not set, the PG_AUTOCONF_FILENAME file cannot be + * read. In this case, we don't want to accept any settings but + * data_directory from postgresql.conf, because they might be + * overwritten with settings in the PG_AUTOCONF_FILENAME file which + * will be read later. OTOH, since data_directory isn't allowed in the + * PG_AUTOCONF_FILENAME file, it will never be overwritten later. + */ ConfigVariable *prev = NULL; - /* - * Pick up only the data_directory if DataDir is not set, which - * means that the configuration file is read for the first time and - * PG_AUTOCONF_FILENAME file cannot be read yet. In this case, - * we shouldn't pick any settings except the data_directory - * from postgresql.conf because they might be overwritten - * with the settings in PG_AUTOCONF_FILENAME file which will be - * read later. OTOH, since it's ensured that data_directory doesn't - * exist in PG_AUTOCONF_FILENAME file, it will never be overwritten - * later. - */ + /* Prune all items except "data_directory" from the list */ for (item = head; item;) { ConfigVariable *ptr = item; @@ -189,15 +188,9 @@ ProcessConfigFile(GucContext context) if (prev == NULL) head = ptr->next; else - { prev->next = ptr->next; - /* - * On removing last item in list, we need to update tail - * to ensure that list will be maintianed. - */ - if (prev->next == NULL) - tail = prev; - } + if (ptr->next == NULL) + tail = prev; FreeConfigVariable(ptr); } else @@ -205,10 +198,10 @@ ProcessConfigFile(GucContext context) } /* - * Quick exit if data_directory is not present in list. + * Quick exit if data_directory is not present in file. * - * Don't remember when we last successfully loaded the config file in - * this case because that time will be set soon by subsequent load of + * We need not do any further processing, in particular we don't set + * PgReloadTime; that will be set soon by subsequent full loading of * the config file. */ if (head == NULL) @@ -263,7 +256,7 @@ ProcessConfigFile(GucContext context) item->name, item->filename, item->sourceline))); error = true; - ErrorConfFile = item->filename; + ConfFileWithError = item->filename; } } @@ -392,7 +385,7 @@ ProcessConfigFile(GucContext context) else if (scres == 0) { error = true; - ErrorConfFile = item->filename; + ConfFileWithError = item->filename; } /* else no error but variable's active value was not changed */ @@ -421,23 +414,23 @@ ProcessConfigFile(GucContext context) ereport(ERROR, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("configuration file \"%s\" contains errors", - ErrorConfFile))); + ConfFileWithError))); else if (apply) ereport(elevel, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("configuration file \"%s\" contains errors; unaffected changes were applied", - ErrorConfFile))); + ConfFileWithError))); else ereport(elevel, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("configuration file \"%s\" contains errors; no changes were applied", - ErrorConfFile))); + ConfFileWithError))); } /* * Calling FreeConfigVariables() any earlier than this can cause problems, - * because ErrorConfFile could be pointing to a string that will be freed - * here. + * because ConfFileWithError could be pointing to a string that will be + * freed here. */ FreeConfigVariables(head); } @@ -477,8 +470,11 @@ AbsoluteConfigLocation(const char *location, const char *calling_file) * Read and parse a single configuration file. This function recurses * to handle "include" directives. * - * See ParseConfigFp for details. This one merely adds opening the - * file rather than working from a caller-supplied file descriptor, + * If "strict" is true, treat failure to open the config file as an error, + * otherwise just skip the file. + * + * See ParseConfigFp for further details. This one merely adds opening the + * config file rather than working from a caller-supplied file descriptor, * and absolute-ifying the path name if necessary. */ bool @@ -516,12 +512,13 @@ ParseConfigFile(const char *config_file, const char *calling_file, bool strict, errmsg("could not open configuration file \"%s\": %m", abs_path))); OK = false; - goto cleanup; } - - ereport(LOG, - (errmsg("skipping missing configuration file \"%s\"", - abs_path))); + else + { + ereport(LOG, + (errmsg("skipping missing configuration file \"%s\"", + abs_path))); + } goto cleanup; } @@ -616,9 +613,7 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, { char *opt_name = NULL; char *opt_value = NULL; - ConfigVariable *item, - *cur_item = NULL, - *prev_item = NULL; + ConfigVariable *item; if (token == GUC_EOL) /* empty or comment line */ continue; @@ -701,41 +696,13 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, } else { - /* - * ordinary variable, append to list. For multiple items of - * same parameter, retain only which comes later. - */ + /* ordinary variable, append to list */ item = palloc(sizeof *item); item->name = opt_name; item->value = opt_value; item->filename = pstrdup(config_file); item->sourceline = ConfigFileLineno-1; item->next = NULL; - - /* Remove the existing item of same parameter from the list */ - for (cur_item = *head_p; cur_item; prev_item = cur_item, - cur_item = cur_item->next) - { - if (strcmp(item->name, cur_item->name) == 0) - { - if (prev_item == NULL) - *head_p = cur_item->next; - else - { - prev_item->next = cur_item->next; - /* - * On removing last item in list, we need to update tail - * to ensure that list will be maintianed. - */ - if (prev_item->next == NULL) - *tail_p = prev_item; - } - - FreeConfigVariable(cur_item); - break; - } - } - if (*head_p == NULL) *head_p = item; else @@ -911,21 +878,6 @@ cleanup: return status; } -/* - * Free a ConfigVariable - */ -static void -FreeConfigVariable(ConfigVariable *item) -{ - if (item != NULL) - { - pfree(item->name); - pfree(item->value); - pfree(item->filename); - pfree(item); - } -} - /* * Free a list of ConfigVariables, including the names and the values */ @@ -944,6 +896,18 @@ FreeConfigVariables(ConfigVariable *list) } } +/* + * Free a single ConfigVariable + */ +static void +FreeConfigVariable(ConfigVariable *item) +{ + pfree(item->name); + pfree(item->value); + pfree(item->filename); + pfree(item); +} + /* * scanstr diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index f6df077248..931993c25b 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -202,14 +202,6 @@ static bool check_cluster_name(char **newval, void **extra, GucSource source); static const char *show_unix_socket_permissions(void); static const char *show_log_file_mode(void); -static char *config_enum_get_options(struct config_enum * record, - const char *prefix, const char *suffix, - const char *separator); - -static bool validate_conf_option(struct config_generic * record, - const char *name, const char *value, GucSource source, - int elevel, bool freemem, void *newval, void **newextra); - /* * Options for enum values defined in this module. @@ -476,7 +468,7 @@ int tcp_keepalives_idle; int tcp_keepalives_interval; int tcp_keepalives_count; -int row_security = true; +int row_security; /* * This really belongs in pg_shmem.c, but is defined here so that it doesn't @@ -2565,7 +2557,7 @@ static struct config_int ConfigureNamesInt[] = { {"gin_pending_list_limit", PGC_USERSET, CLIENT_CONN_STATEMENT, gettext_noop("Sets the maximum size of the pending list for GIN index."), - NULL, + NULL, GUC_UNIT_KB }, &gin_pending_list_limit, @@ -3599,9 +3591,9 @@ static void ShowAllGUCConfig(DestReceiver *dest); static char *_ShowOption(struct config_generic * record, bool use_units); static bool validate_option_array_item(const char *name, const char *value, bool skipIfNoPermissions); -static void write_auto_conf_file(int fd, const char *filename, ConfigVariable **head_p); +static void write_auto_conf_file(int fd, const char *filename, ConfigVariable *head_p); static void replace_auto_config_value(ConfigVariable **head_p, ConfigVariable **tail_p, - char *config_file, char *name, char *value); + const char *name, const char *value); /* @@ -4375,11 +4367,9 @@ SelectConfigFiles(const char *userDoption, const char *progname) } /* - * Read the configuration file for the first time. This time only - * data_directory parameter is picked up to determine the data directory - * so that we can read PG_AUTOCONF_FILENAME file next time. Then don't - * forget to read the configuration file again later to pick up all the - * parameters. + * Read the configuration file for the first time. This time only the + * data_directory parameter is picked up to determine the data directory, + * so that we can read the PG_AUTOCONF_FILENAME file next time. */ ProcessConfigFile(PGC_POSTMASTER); @@ -5380,217 +5370,169 @@ config_enum_get_options(struct config_enum * record, const char *prefix, } /* - * Validates configuration parameter and value, by calling check hook functions - * depending on record's vartype. It validates if the parameter - * value given is in range of expected predefined value for that parameter. + * Parse and validate a proposed value for the specified configuration + * parameter. * - * freemem - true indicates memory for newval and newextra will be - * freed in this function, false indicates it will be freed - * by caller. - * Return value: - * 1: the value is valid - * 0: the name or value is invalid + * This does built-in checks (such as range limits for an integer parameter) + * and also calls any check hook the parameter may have. + * + * record: GUC variable's info record + * name: variable name (should match the record of course) + * value: proposed value, as a string + * source: identifies source of value (check hooks may need this) + * elevel: level to log any error reports at + * newval: on success, converted parameter value is returned here + * newextra: on success, receives any "extra" data returned by check hook + * (caller must initialize *newextra to NULL) + * + * Returns true if OK, false if not (or throws error, if elevel >= ERROR) */ static bool -validate_conf_option(struct config_generic * record, const char *name, - const char *value, GucSource source, int elevel, - bool freemem, void *newval, void **newextra) +parse_and_validate_value(struct config_generic * record, + const char *name, const char *value, + GucSource source, int elevel, + union config_var_val * newval, void **newextra) { - /* - * Validate the value for the passed record, to ensure it is in expected - * range. - */ switch (record->vartype) { - case PGC_BOOL: { struct config_bool *conf = (struct config_bool *) record; - bool tmpnewval; - if (newval == NULL) - newval = &tmpnewval; - - if (value != NULL) + if (!parse_bool(value, &newval->boolval)) { - if (!parse_bool(value, newval)) - { - ereport(elevel, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + ereport(elevel, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("parameter \"%s\" requires a Boolean value", name))); - return 0; - } - - if (!call_bool_check_hook(conf, newval, newextra, - source, elevel)) - return 0; - - if (*newextra && freemem) - free(*newextra); + return false; } + + if (!call_bool_check_hook(conf, &newval->boolval, newextra, + source, elevel)) + return false; } break; case PGC_INT: { struct config_int *conf = (struct config_int *) record; - int tmpnewval; + const char *hintmsg; - if (newval == NULL) - newval = &tmpnewval; - - if (value != NULL) + if (!parse_int(value, &newval->intval, + conf->gen.flags, &hintmsg)) { - const char *hintmsg; - - if (!parse_int(value, newval, conf->gen.flags, &hintmsg)) - { - ereport(elevel, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + ereport(elevel, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid value for parameter \"%s\": \"%s\"", name, value), - hintmsg ? errhint("%s", _(hintmsg)) : 0)); - return 0; - } - - if (*((int *) newval) < conf->min || *((int *) newval) > conf->max) - { - ereport(elevel, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("%d is outside the valid range for parameter \"%s\" (%d .. %d)", - *((int *) newval), name, conf->min, conf->max))); - return 0; - } - - if (!call_int_check_hook(conf, newval, newextra, - source, elevel)) - return 0; - - if (*newextra && freemem) - free(*newextra); + hintmsg ? errhint("%s", _(hintmsg)) : 0)); + return false; } + + if (newval->intval < conf->min || newval->intval > conf->max) + { + ereport(elevel, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("%d is outside the valid range for parameter \"%s\" (%d .. %d)", + newval->intval, name, + conf->min, conf->max))); + return false; + } + + if (!call_int_check_hook(conf, &newval->intval, newextra, + source, elevel)) + return false; } break; case PGC_REAL: { struct config_real *conf = (struct config_real *) record; - double tmpnewval; - if (newval == NULL) - newval = &tmpnewval; - - if (value != NULL) + if (!parse_real(value, &newval->realval)) { - if (!parse_real(value, newval)) - { - ereport(elevel, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + ereport(elevel, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("parameter \"%s\" requires a numeric value", name))); - return 0; - } - - if (*((double *) newval) < conf->min || *((double *) newval) > conf->max) - { - ereport(elevel, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("%g is outside the valid range for parameter \"%s\" (%g .. %g)", - *((double *) newval), name, conf->min, conf->max))); - return 0; - } - - if (!call_real_check_hook(conf, newval, newextra, - source, elevel)) - return 0; - - if (*newextra && freemem) - free(*newextra); + return false; } + + if (newval->realval < conf->min || newval->realval > conf->max) + { + ereport(elevel, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("%g is outside the valid range for parameter \"%s\" (%g .. %g)", + newval->realval, name, + conf->min, conf->max))); + return false; + } + + if (!call_real_check_hook(conf, &newval->realval, newextra, + source, elevel)) + return false; } break; case PGC_STRING: { struct config_string *conf = (struct config_string *) record; - char *tempPtr; - char **tmpnewval = newval; - if (newval == NULL) - tmpnewval = &tempPtr; + /* + * The value passed by the caller could be transient, so we + * always strdup it. + */ + newval->stringval = guc_strdup(elevel, value); + if (newval->stringval == NULL) + return false; - if (value != NULL) + /* + * The only built-in "parsing" check we have is to apply + * truncation if GUC_IS_NAME. + */ + if (conf->gen.flags & GUC_IS_NAME) + truncate_identifier(newval->stringval, + strlen(newval->stringval), + true); + + if (!call_string_check_hook(conf, &newval->stringval, newextra, + source, elevel)) { - /* - * The value passed by the caller could be transient, so - * we always strdup it. - */ - *tmpnewval = guc_strdup(elevel, value); - if (*tmpnewval == NULL) - return 0; - - /* - * The only built-in "parsing" check we have is to apply - * truncation if GUC_IS_NAME. - */ - if (conf->gen.flags & GUC_IS_NAME) - truncate_identifier(*tmpnewval, strlen(*tmpnewval), true); - - if (!call_string_check_hook(conf, tmpnewval, newextra, - source, elevel)) - { - free(*tmpnewval); - return 0; - } - - /* Free the malloc'd data if any */ - if (freemem) - { - if (*tmpnewval != NULL) - free(*tmpnewval); - if (*newextra != NULL) - free(*newextra); - } + free(newval->stringval); + newval->stringval = NULL; + return false; } } break; case PGC_ENUM: { struct config_enum *conf = (struct config_enum *) record; - int tmpnewval; - if (newval == NULL) - newval = &tmpnewval; - - if (value != NULL) + if (!config_enum_lookup_by_name(conf, value, &newval->enumval)) { - if (!config_enum_lookup_by_name(conf, value, newval)) - { - char *hintmsg; + char *hintmsg; - hintmsg = config_enum_get_options(conf, - "Available values: ", - ".", ", "); + hintmsg = config_enum_get_options(conf, + "Available values: ", + ".", ", "); - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + ereport(elevel, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid value for parameter \"%s\": \"%s\"", name, value), - hintmsg ? errhint("%s", _(hintmsg)) : 0)); + hintmsg ? errhint("%s", _(hintmsg)) : 0)); - if (hintmsg != NULL) - pfree(hintmsg); - return 0; - } - if (!call_enum_check_hook(conf, newval, newextra, - source, LOG)) - return 0; - - if (*newextra && freemem) - free(*newextra); + if (hintmsg) + pfree(hintmsg); + return false; } + + if (!call_enum_check_hook(conf, &newval->enumval, newextra, + source, elevel)) + return false; } break; } - return 1; + + return true; } @@ -5636,6 +5578,8 @@ set_config_option(const char *name, const char *value, bool is_reload) { struct config_generic *record; + union config_var_val newval_union; + void *newextra = NULL; bool prohibitValueChange = false; bool makeDefault; @@ -5649,7 +5593,9 @@ set_config_option(const char *name, const char *value, */ elevel = IsUnderPostmaster ? DEBUG3 : LOG; } - else if (source == PGC_S_GLOBAL || source == PGC_S_DATABASE || source == PGC_S_USER || + else if (source == PGC_S_GLOBAL || + source == PGC_S_DATABASE || + source == PGC_S_USER || source == PGC_S_DATABASE_USER) elevel = WARNING; else @@ -5859,14 +5805,14 @@ set_config_option(const char *name, const char *value, case PGC_BOOL: { struct config_bool *conf = (struct config_bool *) record; - bool newval; - void *newextra = NULL; + +#define newval (newval_union.boolval) if (value) { - if (!validate_conf_option(record, name, value, source, - elevel, false, &newval, - &newextra)) + if (!parse_and_validate_value(record, name, value, + source, elevel, + &newval_union, &newextra)) return 0; } else if (source == PGC_S_DEFAULT) @@ -5940,19 +5886,21 @@ set_config_option(const char *name, const char *value, if (newextra && !extra_field_used(&conf->gen, newextra)) free(newextra); break; + +#undef newval } case PGC_INT: { struct config_int *conf = (struct config_int *) record; - int newval; - void *newextra = NULL; + +#define newval (newval_union.intval) if (value) { - if (!validate_conf_option(record, name, value, source, - elevel, false, &newval, - &newextra)) + if (!parse_and_validate_value(record, name, value, + source, elevel, + &newval_union, &newextra)) return 0; } else if (source == PGC_S_DEFAULT) @@ -6026,19 +5974,21 @@ set_config_option(const char *name, const char *value, if (newextra && !extra_field_used(&conf->gen, newextra)) free(newextra); break; + +#undef newval } case PGC_REAL: { struct config_real *conf = (struct config_real *) record; - double newval; - void *newextra = NULL; + +#define newval (newval_union.realval) if (value) { - if (!validate_conf_option(record, name, value, source, - elevel, false, &newval, - &newextra)) + if (!parse_and_validate_value(record, name, value, + source, elevel, + &newval_union, &newextra)) return 0; } else if (source == PGC_S_DEFAULT) @@ -6112,19 +6062,21 @@ set_config_option(const char *name, const char *value, if (newextra && !extra_field_used(&conf->gen, newextra)) free(newextra); break; + +#undef newval } case PGC_STRING: { struct config_string *conf = (struct config_string *) record; - char *newval; - void *newextra = NULL; + +#define newval (newval_union.stringval) if (value) { - if (!validate_conf_option(record, name, value, source, - elevel, false, &newval, - &newextra)) + if (!parse_and_validate_value(record, name, value, + source, elevel, + &newval_union, &newextra)) return 0; } else if (source == PGC_S_DEFAULT) @@ -6221,19 +6173,21 @@ set_config_option(const char *name, const char *value, if (newextra && !extra_field_used(&conf->gen, newextra)) free(newextra); break; + +#undef newval } case PGC_ENUM: { struct config_enum *conf = (struct config_enum *) record; - int newval; - void *newextra = NULL; + +#define newval (newval_union.enumval) if (value) { - if (!validate_conf_option(record, name, value, source, - elevel, false, &newval, - &newextra)) + if (!parse_and_validate_value(record, name, value, + source, elevel, + &newval_union, &newextra)) return 0; } else if (source == PGC_S_DEFAULT) @@ -6307,6 +6261,8 @@ set_config_option(const char *name, const char *value, if (newextra && !extra_field_used(&conf->gen, newextra)) free(newextra); break; + +#undef newval } } @@ -6609,50 +6565,61 @@ flatten_set_variable_args(const char *name, List *args) * values before writing them. */ static void -write_auto_conf_file(int fd, const char *filename, ConfigVariable **head_p) +write_auto_conf_file(int fd, const char *filename, ConfigVariable *head) { - ConfigVariable *item; StringInfoData buf; + ConfigVariable *item; initStringInfo(&buf); + + /* Emit file header containing warning comment */ appendStringInfoString(&buf, "# Do not edit this file manually!\n"); appendStringInfoString(&buf, "# It will be overwritten by ALTER SYSTEM command.\n"); - /* - * write the file header message before contents, so that if there is no - * item it can contain message - */ - if (write(fd, buf.data, buf.len) < 0) + errno = 0; + if (write(fd, buf.data, buf.len) != buf.len) + { + /* if write didn't set errno, assume problem is no disk space */ + if (errno == 0) + errno = ENOSPC; ereport(ERROR, - (errmsg("could not write to file \"%s\": %m", filename))); - resetStringInfo(&buf); + (errcode_for_file_access(), + errmsg("could not write to file \"%s\": %m", filename))); + } - /* - * traverse the list of parameters, quote the string parameter and write - * it to file. Once all parameters are written fsync the file. - */ - - for (item = *head_p; item != NULL; item = item->next) + /* Emit each parameter, properly quoting the value */ + for (item = head; item != NULL; item = item->next) { char *escaped; - appendStringInfoString(&buf, item->name); - appendStringInfoString(&buf, " = "); + resetStringInfo(&buf); + + appendStringInfoString(&buf, item->name); + appendStringInfoString(&buf, " = '"); - appendStringInfoString(&buf, "\'"); escaped = escape_single_quotes_ascii(item->value); + if (!escaped) + ereport(ERROR, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); appendStringInfoString(&buf, escaped); free(escaped); - appendStringInfoString(&buf, "\'"); - appendStringInfoString(&buf, "\n"); + appendStringInfoString(&buf, "'\n"); - if (write(fd, buf.data, buf.len) < 0) + errno = 0; + if (write(fd, buf.data, buf.len) != buf.len) + { + /* if write didn't set errno, assume problem is no disk space */ + if (errno == 0) + errno = ENOSPC; ereport(ERROR, - (errmsg("could not write to file \"%s\": %m", filename))); - resetStringInfo(&buf); + (errcode_for_file_access(), + errmsg("could not write to file \"%s\": %m", filename))); + } } + /* fsync before considering the write to be successful */ if (pg_fsync(fd) != 0) ereport(ERROR, (errcode_for_file_access(), @@ -6661,92 +6628,76 @@ write_auto_conf_file(int fd, const char *filename, ConfigVariable **head_p) pfree(buf.data); } - /* - * This function takes list of all configuration parameters in - * PG_AUTOCONF_FILENAME and parameter to be updated as input arguments and - * replace the updated configuration parameter value in a list. If the - * parameter to be updated is new then it is appended to the list of - * parameters. + * Update the given list of configuration parameters, adding, replacing + * or deleting the entry for item "name" (delete if "value" == NULL). */ static void replace_auto_config_value(ConfigVariable **head_p, ConfigVariable **tail_p, - char *config_file, - char *name, char *value) + const char *name, const char *value) { ConfigVariable *item, *prev = NULL; - if (*head_p != NULL) + /* Search the list for an existing match (we assume there's only one) */ + for (item = *head_p; item != NULL; item = item->next) { - for (item = *head_p; item != NULL; item = item->next) + if (strcmp(item->name, name) == 0) { - if (strcmp(item->name, name) == 0) + /* found a match, replace it */ + pfree(item->value); + if (value != NULL) { - pfree(item->value); - if (value != NULL) - /* update the parameter value */ - item->value = pstrdup(value); - else - { - /* delete the configuration parameter from list */ - if (*head_p == item) - *head_p = item->next; - else - prev->next = item->next; - - if (*tail_p == item) - *tail_p = prev; - - pfree(item->name); - pfree(item->filename); - pfree(item); - } - return; + /* update the parameter value */ + item->value = pstrdup(value); } - prev = item; + else + { + /* delete the configuration parameter from list */ + if (*head_p == item) + *head_p = item->next; + else + prev->next = item->next; + if (*tail_p == item) + *tail_p = prev; + + pfree(item->name); + pfree(item->filename); + pfree(item); + } + return; } + prev = item; } + /* Not there; no work if we're trying to delete it */ if (value == NULL) return; + /* OK, append a new entry */ item = palloc(sizeof *item); item->name = pstrdup(name); item->value = pstrdup(value); - item->filename = pstrdup(config_file); + item->filename = pstrdup(""); /* new item has no location */ + item->sourceline = 0; item->next = NULL; if (*head_p == NULL) - { - item->sourceline = 1; *head_p = item; - } else - { - item->sourceline = (*tail_p)->sourceline + 1; (*tail_p)->next = item; - } - *tail_p = item; - - return; } /* - * Persist the configuration parameter value. + * Execute ALTER SYSTEM statement. * - * This function takes all previous configuration parameters - * set by ALTER SYSTEM command and the currently set ones - * and write them all to the automatic configuration file. - * It just writes an empty file incase user wants to reset - * all the parameters. + * Read the old PG_AUTOCONF_FILENAME file, merge in the new variable value, + * and write out an updated file. If the command is ALTER SYSTEM RESET ALL, + * we can skip reading the old file and just write an empty file. * - * The configuration parameters are written to a temporary - * file then renamed to the final name. - * - * An LWLock is used to serialize writing to the same file. + * An LWLock is used to serialize updates of the configuration file. * * In case of an error, we leave the original automatic * configuration file (PG_AUTOCONF_FILENAME) intact. @@ -6757,15 +6708,11 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt) char *name; char *value; bool resetall = false; - int Tmpfd = -1; - FILE *infile; - struct config_generic *record; ConfigVariable *head = NULL; ConfigVariable *tail = NULL; + volatile int Tmpfd; char AutoConfFileName[MAXPGPATH]; char AutoConfTmpFileName[MAXPGPATH]; - struct stat st; - void *newextra = NULL; if (!superuser()) ereport(ERROR, @@ -6773,7 +6720,7 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt) (errmsg("must be superuser to execute ALTER SYSTEM command")))); /* - * Validate the name and arguments [value1, value2 ... ]. + * Extract statement arguments */ name = altersysstmt->setstmt->name; @@ -6799,10 +6746,14 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt) break; } - /* If we're resetting everything, there's no need to validate anything */ + /* + * Unless it's RESET_ALL, validate the target variable and value + */ if (!resetall) { - record = find_option(name, false, LOG); + struct config_generic *record; + + record = find_option(name, false, ERROR); if (record == NULL) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), @@ -6810,8 +6761,8 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt) name))); /* - * Don't allow the parameters which can't be set in configuration - * files to be set in PG_AUTOCONF_FILENAME file. + * Don't allow parameters that can't be set in configuration files to + * be set in PG_AUTOCONF_FILENAME file. */ if ((record->context == PGC_INTERNAL) || (record->flags & GUC_DISALLOW_IN_FILE) || @@ -6821,14 +6772,24 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt) errmsg("parameter \"%s\" cannot be changed", name))); - if (!validate_conf_option(record, name, value, PGC_S_FILE, - ERROR, true, NULL, - &newextra)) - ereport(ERROR, - (errmsg("invalid value for parameter \"%s\": \"%s\"", - name, value))); - } + if (value) + { + union config_var_val newval; + void *newextra = NULL; + if (!parse_and_validate_value(record, name, value, + PGC_S_FILE, ERROR, + &newval, &newextra)) + ereport(ERROR, + (errmsg("invalid value for parameter \"%s\": \"%s\"", + name, value))); + + if (record->vartype == PGC_STRING && newval.stringval != NULL) + free(newval.stringval); + if (newextra) + free(newextra); + } + } /* * Use data directory as reference path for PG_AUTOCONF_FILENAME and its @@ -6841,62 +6802,76 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt) "tmp"); /* - * One backend is allowed to operate on file PG_AUTOCONF_FILENAME, to - * ensure that we need to update the contents of the file with - * AutoFileLock. To ensure crash safety, first the contents are written to - * a temporary file which is then renameed to PG_AUTOCONF_FILENAME. In - * case there exists a temp file from previous crash, that can be reused. + * Only one backend is allowed to operate on PG_AUTOCONF_FILENAME at a + * time. Use AutoFileLock to ensure that. We must hold the lock while + * reading the old file contents. */ - LWLockAcquire(AutoFileLock, LW_EXCLUSIVE); - Tmpfd = open(AutoConfTmpFileName, O_CREAT | O_RDWR | O_TRUNC, S_IRUSR | S_IWUSR); + /* + * If we're going to reset everything, then no need to open or parse the + * old file. We'll just write out an empty list. + */ + if (!resetall) + { + struct stat st; + + if (stat(AutoConfFileName, &st) == 0) + { + /* open old file PG_AUTOCONF_FILENAME */ + FILE *infile; + + infile = AllocateFile(AutoConfFileName, "r"); + if (infile == NULL) + ereport(ERROR, + (errmsg("could not open file \"%s\": %m", + AutoConfFileName))); + + /* parse it */ + ParseConfigFp(infile, AutoConfFileName, 0, LOG, &head, &tail); + + FreeFile(infile); + } + + /* + * Now, replace any existing entry with the new value, or add it if + * not present. + */ + replace_auto_config_value(&head, &tail, name, value); + } + + /* + * To ensure crash safety, first write the new file data to a temp file, + * then atomically rename it into place. + * + * If there is a temp file left over due to a previous crash, it's okay to + * truncate and reuse it. + */ + Tmpfd = BasicOpenFile(AutoConfTmpFileName, + O_CREAT | O_RDWR | O_TRUNC, + S_IRUSR | S_IWUSR); if (Tmpfd < 0) ereport(ERROR, (errcode_for_file_access(), errmsg("could not open file \"%s\": %m", AutoConfTmpFileName))); + /* + * Use a TRY block to clean up the file if we fail. Since we need a TRY + * block anyway, OK to use BasicOpenFile rather than OpenTransientFile. + */ PG_TRY(); { - /* - * If we're going to reset everything, then don't open the file, don't - * parse it, and don't do anything with the configuration list. Just - * write out an empty file. - */ - if (!resetall) - { - if (stat(AutoConfFileName, &st) == 0) - { - /* open file PG_AUTOCONF_FILENAME */ - infile = AllocateFile(AutoConfFileName, "r"); - if (infile == NULL) - ereport(ERROR, - (errmsg("could not open file \"%s\": %m", - AutoConfFileName))); - - /* parse it */ - ParseConfigFp(infile, AutoConfFileName, 0, LOG, &head, &tail); - - FreeFile(infile); - } - - /* - * replace with new value if the configuration parameter already - * exists OR add it as a new cofiguration parameter in the file. - */ - replace_auto_config_value(&head, &tail, AutoConfFileName, name, value); - } - /* Write and sync the new contents to the temporary file */ - write_auto_conf_file(Tmpfd, AutoConfTmpFileName, &head); + write_auto_conf_file(Tmpfd, AutoConfTmpFileName, head); + /* Close before renaming; may be required on some platforms */ close(Tmpfd); Tmpfd = -1; /* * As the rename is atomic operation, if any problem occurs after this - * at max it can loose the parameters set by last ALTER SYSTEM + * at worst it can lose the parameters set by last ALTER SYSTEM * command. */ if (rename(AutoConfTmpFileName, AutoConfFileName) < 0) @@ -6907,18 +6882,20 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt) } PG_CATCH(); { + /* Close file first, else unlink might fail on some platforms */ if (Tmpfd >= 0) close(Tmpfd); - unlink(AutoConfTmpFileName); - FreeConfigVariables(head); + /* Unlink, but ignore any error */ + (void) unlink(AutoConfTmpFileName); + PG_RE_THROW(); } PG_END_TRY(); FreeConfigVariables(head); + LWLockRelease(AutoFileLock); - return; } /* @@ -8849,7 +8826,7 @@ RestoreGUCState(void *gucstate) while (srcptr < srcend) { - int result; + int result; if ((varname = read_gucstate(&srcptr, srcend)) == NULL) break;