*** a/src/backend/access/transam/xlog.c --- b/src/backend/access/transam/xlog.c *************** *** 5297,5303 **** readRecoveryCommandFile(void) * Since we're asking ParseConfigFp() to error out at FATAL, there's no * need to check the return value. */ ! ParseConfigFp(fd, RECOVERY_COMMAND_FILE, 0, FATAL, &head, &tail); for (item = head; item; item = item->next) { --- 5297,5303 ---- * Since we're asking ParseConfigFp() to error out at FATAL, there's no * need to check the return value. */ ! ParseConfigFp(fd, RECOVERY_COMMAND_FILE, 0, FATAL, 0, &head, &tail); for (item = head; item; item = item->next) { *** a/src/backend/commands/extension.c --- b/src/backend/commands/extension.c *************** *** 474,480 **** parse_extension_control_file(ExtensionControlFile *control, /* * Parse the file content, using GUC's file parsing code */ ! ParseConfigFp(file, filename, 0, ERROR, &head, &tail); FreeFile(file); --- 474,480 ---- /* * Parse the file content, using GUC's file parsing code */ ! ParseConfigFp(file, filename, 0, ERROR, 0, &head, &tail); FreeFile(file); *** a/src/backend/utils/misc/guc-file.l --- b/src/backend/utils/misc/guc-file.l *************** *** 43,48 **** int GUC_yylex(void); --- 43,51 ---- static char *GUC_scanstr(const char *s); + /* Maximum number of errors to report */ + #define MAX_ERROR_REPORTS 100 + %} %option 8bit *************** *** 112,135 **** ProcessConfigFile(GucContext context) char *cvc = NULL; struct config_string *cvc_struct; int i; ! Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP); ! if (context == PGC_SIGHUP) ! { ! /* ! * To avoid cluttering the log, only the postmaster bleats loudly ! * about problems with the config file. ! */ ! elevel = IsUnderPostmaster ? DEBUG2 : LOG; ! } ! else ! elevel = ERROR; ! /* Parse the file into a list of option names and values */ head = tail = NULL; ! if (!ParseConfigFile(ConfigFileName, NULL, 0, elevel, &head, &tail)) goto cleanup_list; /* --- 115,146 ---- char *cvc = NULL; struct config_string *cvc_struct; int i; + bool errors, + apply = false; ! /* ! * Config files are only processes on startup (by the postmaster) ! * and on SIGHUP (by the postmaster and backends) ! */ ! Assert((context == PGC_POSTMASTER && !IsUnderPostmaster) || ! context == PGC_SIGHUP); ! /* ! * To avoid cluttering the log, only the postmaster bleats loudly ! * about problems with the config file. ! */ ! elevel = IsUnderPostmaster ? DEBUG2 : LOG; ! /* ! * Parse the file into a list of option names and values. We continue even ! * if we hit a parse error, because we also want to report invalid values in ! * the parts we could parse. ! */ head = tail = NULL; ! errors = (ParseConfigFile(ConfigFileName, NULL, 0, elevel, 0, &head, &tail) > 0); ! /* bail out early if errors were found during the parse stage */ ! if (errors) goto cleanup_list; /* *************** *** 162,168 **** ProcessConfigFile(GucContext context) --- 173,182 ---- goto cleanup_list; if (!call_string_check_hook(cvc_struct, &cvc, &extra, PGC_S_FILE, elevel)) + { + errors = true; goto cleanup_list; + } if (extra) free(extra); } *************** *** 179,184 **** ProcessConfigFile(GucContext context) --- 193,200 ---- gconf->status &= ~GUC_IS_IN_FILE; } + apply = true; + /* * Check if all options are valid. As a side-effect, the GUC_IS_IN_FILE * flag is set on each GUC variable mentioned in the list. *************** *** 202,208 **** ProcessConfigFile(GucContext context) (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("unrecognized configuration parameter \"%s\"", item->name))); ! goto cleanup_list; } /* * 2. There is no GUC entry. If we called set_config_option then --- 218,226 ---- (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("unrecognized configuration parameter \"%s\"", item->name))); ! errors = true; ! apply = false; ! continue; } /* * 2. There is no GUC entry. If we called set_config_option then *************** *** 219,230 **** ProcessConfigFile(GucContext context) * entry. */ } ! if (!set_config_option(item->name, item->value, context, PGC_S_FILE, GUC_ACTION_SET, false)) ! goto cleanup_list; } /* * Check for variables having been removed from the config file, and * revert their reset values (and perhaps also effective values) to the --- 237,268 ---- * entry. */ } ! else if (find_option(item->name, false, elevel) == NULL) ! { ! /* For regular variables require a real GUC entry or bail out. */ ! ereport(elevel, ! (errcode(ERRCODE_UNDEFINED_OBJECT), ! errmsg("unrecognized configuration parameter \"%s\"", ! item->name))); ! errors = true; ! apply = false; ! continue; ! } ! /* Note possible problems and continue */ if (!set_config_option(item->name, item->value, context, PGC_S_FILE, GUC_ACTION_SET, false)) ! errors = true; } + /* + * Exit early if errors were detected during postmaster's startup + * or if we can't apply settings at SIGHUP due to parse errors. + */ + if (errors && context == PGC_POSTMASTER) + apply = false; + if (!apply) + goto cleanup_list; + /* * Check for variables having been removed from the config file, and * revert their reset values (and perhaps also effective values) to the *************** *** 249,254 **** ProcessConfigFile(GucContext context) --- 287,293 ---- (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed without restarting the server", gconf->name))); + errors = true; continue; } *************** *** 285,291 **** ProcessConfigFile(GucContext context) * particular should be run only after initialization is complete. * * XXX this is an unmaintainable crock, because we have to know how ! * to set (or at least what to call to set) every variable that could * potentially have PGC_S_DYNAMIC_DEFAULT or PGC_S_ENV_VAR source. * However, there's no time to redesign it for 9.1. */ --- 324,330 ---- * particular should be run only after initialization is complete. * * XXX this is an unmaintainable crock, because we have to know how ! * to set (at least what to call to set) every variable that could * potentially have PGC_S_DYNAMIC_DEFAULT or PGC_S_ENV_VAR source. * However, there's no time to redesign it for 9.1. */ *************** *** 316,321 **** ProcessConfigFile(GucContext context) --- 355,370 ---- pre_value = pstrdup(preval); } + /* + * XXX: We would like to notice the errors from set_config_option by + * examining its return value, but, unfortunately, it's not possible. + * The reason is that a false result is returned for PGC_POSTMASTER + * variables during reload, even unchanged ones. This also means that + * actual attempts to change such variables won't contribute to the + * errorcount, because the same call returns true during the check + * stage (when the changeVal argument is false), and that the invalid + * values will be reported twice during reload. + */ if (set_config_option(item->name, item->value, context, PGC_S_FILE, GUC_ACTION_SET, true)) { *************** *** 346,364 **** ProcessConfigFile(GucContext context) FreeConfigVariables(head); if (cvc) free(cvc); } /* ! * See next function for details. This one will just work with a config_file * name rather than an already opened File Descriptor */ ! bool ParseConfigFile(const char *config_file, const char *calling_file, ! int depth, int elevel, ConfigVariable **head_p, ConfigVariable **tail_p) { ! bool OK = true; FILE *fp; char abs_path[MAXPGPATH]; --- 395,433 ---- FreeConfigVariables(head); if (cvc) free(cvc); + + if (errors) + { + /* If we got errors during postmaster start, complain and bail out. */ + if (context != PGC_SIGHUP) + elevel = ERROR; + else if (!IsUnderPostmaster) + elevel = WARNING; + else + elevel = DEBUG2; + ereport(elevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("errors detected while parsing configuration files"), + apply ? + errdetail("changes were applied.") : + errdetail("changes were not applied."))); + } } /* ! * See ParseConfigFp for details. This one will just work with a config_file * name rather than an already opened File Descriptor + * + * The return convention is the same as ParseConfigFp; additionally, file not + * found or file not accessible conditions are considered a single error. */ ! int ParseConfigFile(const char *config_file, const char *calling_file, ! int depth, int elevel, int nerrors, ConfigVariable **head_p, ConfigVariable **tail_p) { ! int errorcount = 0; FILE *fp; char abs_path[MAXPGPATH]; *************** *** 373,379 **** ParseConfigFile(const char *config_file, const char *calling_file, (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("could not open configuration file \"%s\": maximum nesting depth exceeded", config_file))); ! return false; } /* --- 442,448 ---- (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("could not open configuration file \"%s\": maximum nesting depth exceeded", config_file))); ! return 1; } /* *************** *** 408,421 **** ParseConfigFile(const char *config_file, const char *calling_file, (errcode_for_file_access(), errmsg("could not open configuration file \"%s\": %m", config_file))); ! return false; } ! OK = ParseConfigFp(fp, config_file, depth, elevel, head_p, tail_p); FreeFile(fp); ! return OK; } /* --- 477,490 ---- (errcode_for_file_access(), errmsg("could not open configuration file \"%s\": %m", config_file))); ! return 1; } ! errorcount = ParseConfigFp(fp, config_file, depth, elevel, nerrors, head_p, tail_p); FreeFile(fp); ! return errorcount; } /* *************** *** 427,432 **** ParseConfigFile(const char *config_file, const char *calling_file, --- 496,502 ---- * config_file: absolute or relative path of file to read * depth: recursion depth (used only to prevent infinite recursion) * elevel: error logging level determined by ProcessConfigFile() + * nerrors: number of errors already seen while parsing previous config files * Output parameters: * head_p, tail_p: head and tail of linked list of name/value pairs * *************** *** 434,456 **** ParseConfigFile(const char *config_file, const char *calling_file, * recursion level. On exit, they contain a list of name-value pairs read * from the input file(s). * ! * Returns TRUE if successful, FALSE if an error occurred. The error has ! * already been ereport'd, it is only necessary for the caller to clean up ! * its own state and release the name/value pairs list. * * Note: if elevel >= ERROR then an error will not return control to the * caller, and internal state such as open files will not be cleaned up. * This case occurs only during postmaster or standalone-backend startup, * where an error will lead to immediate process exit anyway; so there is * no point in contorting the code so it can clean up nicely. */ ! bool ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, ! ConfigVariable **head_p, ConfigVariable **tail_p) { - bool OK = true; YY_BUFFER_STATE lex_buffer; ! int token; /* * Parse --- 504,533 ---- * recursion level. On exit, they contain a list of name-value pairs read * from the input file(s). * ! * Returns 0 if successful, or the number of parsing errors found if any ! * occurred. The error(s) have already been ereport'd, it is only necessary ! * for the caller to clean up its own state and release the name/value pairs ! * list. ! * ! * If elevel < ERROR then we don't return immediately on the first error, ! * although we do return after coming across 100 of them. This is to prevent ! * trashing out logs when parsing a completely bogus file. * * Note: if elevel >= ERROR then an error will not return control to the * caller, and internal state such as open files will not be cleaned up. * This case occurs only during postmaster or standalone-backend startup, * where an error will lead to immediate process exit anyway; so there is * no point in contorting the code so it can clean up nicely. + * + * We use nerrors to break out early in case too many errors are found. */ ! int ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, ! int nerrors, ConfigVariable **head_p, ConfigVariable **tail_p) { YY_BUFFER_STATE lex_buffer; ! int token, ! errorcount; /* * Parse *************** *** 459,464 **** ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, --- 536,542 ---- yy_switch_to_buffer(lex_buffer); ConfigFileLineno = 1; + errorcount = 0; /* This loop iterates once per logical line */ while ((token = yylex())) *************** *** 510,524 **** ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, */ unsigned int save_ConfigFileLineno = ConfigFileLineno; ! if (!ParseConfigFile(opt_value, config_file, ! depth + 1, elevel, ! head_p, tail_p)) ! { ! pfree(opt_name); ! pfree(opt_value); ! OK = false; ! goto cleanup_exit; ! } yy_switch_to_buffer(lex_buffer); ConfigFileLineno = save_ConfigFileLineno; pfree(opt_name); --- 588,595 ---- */ unsigned int save_ConfigFileLineno = ConfigFileLineno; ! errorcount += ParseConfigFile(opt_value, config_file, depth + 1, ! elevel, errorcount, head_p, tail_p); yy_switch_to_buffer(lex_buffer); ConfigFileLineno = save_ConfigFileLineno; pfree(opt_name); *************** *** 577,603 **** ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, /* break out of loop if read EOF, else loop for next line */ if (token == 0) break; } /* successful completion of parsing */ - goto cleanup_exit; - - parse_error: - if (token == GUC_EOL || token == 0) - ereport(elevel, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("syntax error in file \"%s\" line %u, near end of line", - config_file, ConfigFileLineno - 1))); - else - ereport(elevel, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("syntax error in file \"%s\" line %u, near token \"%s\"", - config_file, ConfigFileLineno, yytext))); - OK = false; - - cleanup_exit: yy_delete_buffer(lex_buffer); ! return OK; } --- 648,690 ---- /* break out of loop if read EOF, else loop for next line */ if (token == 0) break; + /* skip over parse_error if we made it this far without errors */ + continue; + + parse_error: + if (token == GUC_EOL || token == 0) + ereport(elevel, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("syntax error in file \"%s\" line %u, near end of line", + config_file, ConfigFileLineno - 1))); + else + ereport(elevel, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("syntax error in file \"%s\" line %u, near token \"%s\"", + config_file, ConfigFileLineno, yytext))); + errorcount++; + /* fast forward till the next EOL/EOF */ + while (token != 0 && token != GUC_EOL) + token = yylex(); + + /* break out of loop on EOF */ + if (token == 0) + break; + + /* avoid producing too much noise when parsing a bogus file */ + if (errorcount + nerrors >= MAX_ERROR_REPORTS) + { + ereport(elevel, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("too many errors found, stopped processing file \"%s\"", + config_file))); + break; + } } /* successful completion of parsing */ yy_delete_buffer(lex_buffer); ! return errorcount; } *** a/src/backend/utils/misc/guc.c --- b/src/backend/utils/misc/guc.c *************** *** 5053,5059 **** config_enum_get_options(struct config_enum * record, const char *prefix, * * If there is an error (non-existing option, invalid value) then an * ereport(ERROR) is thrown *unless* this is called in a context where we ! * don't want to ereport (currently, startup or SIGHUP config file reread). * In that case we write a suitable error message via ereport(LOG) and * return false. This is working around the deficiencies in the ereport * mechanism, so don't blame me. In all other cases, the function --- 5053,5059 ---- * * If there is an error (non-existing option, invalid value) then an * ereport(ERROR) is thrown *unless* this is called in a context where we ! * don't want to ereport (currently, settings defaults and config file read). * In that case we write a suitable error message via ereport(LOG) and * return false. This is working around the deficiencies in the ereport * mechanism, so don't blame me. In all other cases, the function *************** *** 5072,5078 **** set_config_option(const char *name, const char *value, bool prohibitValueChange = false; bool makeDefault; ! if (context == PGC_SIGHUP || source == PGC_S_DEFAULT) { /* * To avoid cluttering the log, only the postmaster bleats loudly --- 5072,5078 ---- bool prohibitValueChange = false; bool makeDefault; ! if (source == PGC_S_FILE || source == PGC_S_DEFAULT) { /* * To avoid cluttering the log, only the postmaster bleats loudly *** a/src/include/utils/guc.h --- b/src/include/utils/guc.h *************** *** 110,120 **** typedef struct ConfigVariable struct ConfigVariable *next; } ConfigVariable; ! extern bool ParseConfigFile(const char *config_file, const char *calling_file, ! int depth, int elevel, ConfigVariable **head_p, ConfigVariable **tail_p); ! extern bool ParseConfigFp(FILE *fp, const char *config_file, ! int depth, int elevel, ConfigVariable **head_p, ConfigVariable **tail_p); extern void FreeConfigVariables(ConfigVariable *list); --- 110,120 ---- struct ConfigVariable *next; } ConfigVariable; ! extern int ParseConfigFile(const char *config_file, const char *calling_file, ! int depth, int elevel, int nerrors, ConfigVariable **head_p, ConfigVariable **tail_p); ! extern int ParseConfigFp(FILE *fp, const char *config_file, ! int depth, int elevel, int nerrors, ConfigVariable **head_p, ConfigVariable **tail_p); extern void FreeConfigVariables(ConfigVariable *list);