From 970a18687f9b3058e89d5994a8fbf70888e79548 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Fri, 3 Dec 2010 08:44:15 -0500 Subject: [PATCH] Use GUC lexer for recovery.conf parsing. This eliminates some crufty, special-purpose code and, as a non-trivial side benefit, allows recovery.conf parameters to be unquoted. Dimitri Fontaine, with review and cleanup by Alvaro Herrera, Itagaki Takahiro, and me. --- src/backend/access/transam/xlog.c | 186 +++++++----------------------- src/backend/utils/misc/guc-file.l | 135 ++++++++++++---------- src/backend/utils/misc/guc.c | 2 +- src/include/utils/guc.h | 21 ++++ 4 files changed, 135 insertions(+), 209 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index ede6ceb6af..ae9ed8fe4c 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -5023,116 +5023,21 @@ str_time(pg_time_t tnow) return buf; } -/* - * Parse one line from recovery.conf. 'cmdline' is the raw line from the - * file. If the line is parsed successfully, returns true, false indicates - * syntax error. On success, *key_p and *value_p are set to the parameter - * name and value on the line, respectively. If the line is an empty line, - * consisting entirely of whitespace and comments, function returns true - * and *keyp_p and *value_p are set to NULL. - * - * The pointers returned in *key_p and *value_p point to an internal buffer - * that is valid only until the next call of parseRecoveryCommandFile(). - */ -static bool -parseRecoveryCommandFileLine(char *cmdline, char **key_p, char **value_p) -{ - char *ptr; - char *bufp; - char *key; - char *value; - static char *buf = NULL; - - *key_p = *value_p = NULL; - - /* - * Allocate the buffer on first use. It's used to hold both the parameter - * name and value. - */ - if (buf == NULL) - buf = malloc(MAXPGPATH + 1); - bufp = buf; - - /* Skip any whitespace at the beginning of line */ - for (ptr = cmdline; *ptr; ptr++) - { - if (!isspace((unsigned char) *ptr)) - break; - } - /* Ignore empty lines */ - if (*ptr == '\0' || *ptr == '#') - return true; - - /* Read the parameter name */ - key = bufp; - while (*ptr && !isspace((unsigned char) *ptr) && - *ptr != '=' && *ptr != '\'') - *(bufp++) = *(ptr++); - *(bufp++) = '\0'; - - /* Skip to the beginning quote of the parameter value */ - ptr = strchr(ptr, '\''); - if (!ptr) - return false; - ptr++; - - /* Read the parameter value to *bufp. Collapse any '' escapes as we go. */ - value = bufp; - for (;;) - { - if (*ptr == '\'') - { - ptr++; - if (*ptr == '\'') - *(bufp++) = '\''; - else - { - /* end of parameter */ - *bufp = '\0'; - break; - } - } - else if (*ptr == '\0') - return false; /* unterminated quoted string */ - else - *(bufp++) = *ptr; - - ptr++; - } - *(bufp++) = '\0'; - - /* Check that there's no garbage after the value */ - while (*ptr) - { - if (*ptr == '#') - break; - if (!isspace((unsigned char) *ptr)) - return false; - ptr++; - } - - /* Success! */ - *key_p = key; - *value_p = value; - return true; -} - /* * See if there is a recovery command file (recovery.conf), and if so * read in parameters for archive recovery and XLOG streaming. * - * XXX longer term intention is to expand this to - * cater for additional parameters and controls - * possibly use a flex lexer similar to the GUC one + * The file is parsed using the main configuration parser. */ static void readRecoveryCommandFile(void) { FILE *fd; - char cmdline[MAXPGPATH]; TimeLineID rtli = 0; bool rtliGiven = false; - bool syntaxError = false; + ConfigVariable *item, + *head = NULL, + *tail = NULL; fd = AllocateFile(RECOVERY_COMMAND_FILE, "r"); if (fd == NULL) @@ -5146,55 +5051,47 @@ readRecoveryCommandFile(void) } /* - * Parse the file... - */ - while (fgets(cmdline, sizeof(cmdline), fd) != NULL) + * 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) { - char *tok1; - char *tok2; - - if (!parseRecoveryCommandFileLine(cmdline, &tok1, &tok2)) + if (strcmp(item->name, "restore_command") == 0) { - syntaxError = true; - break; - } - if (tok1 == NULL) - continue; - - if (strcmp(tok1, "restore_command") == 0) - { - recoveryRestoreCommand = pstrdup(tok2); + recoveryRestoreCommand = pstrdup(item->value); ereport(DEBUG2, (errmsg("restore_command = '%s'", recoveryRestoreCommand))); } - else if (strcmp(tok1, "recovery_end_command") == 0) + else if (strcmp(item->name, "recovery_end_command") == 0) { - recoveryEndCommand = pstrdup(tok2); + recoveryEndCommand = pstrdup(item->value); ereport(DEBUG2, (errmsg("recovery_end_command = '%s'", recoveryEndCommand))); } - else if (strcmp(tok1, "archive_cleanup_command") == 0) + else if (strcmp(item->name, "archive_cleanup_command") == 0) { - archiveCleanupCommand = pstrdup(tok2); + archiveCleanupCommand = pstrdup(item->value); ereport(DEBUG2, (errmsg("archive_cleanup_command = '%s'", archiveCleanupCommand))); } - else if (strcmp(tok1, "recovery_target_timeline") == 0) + else if (strcmp(item->name, "recovery_target_timeline") == 0) { rtliGiven = true; - if (strcmp(tok2, "latest") == 0) + if (strcmp(item->value, "latest") == 0) rtli = 0; else { errno = 0; - rtli = (TimeLineID) strtoul(tok2, NULL, 0); + rtli = (TimeLineID) strtoul(item->value, NULL, 0); if (errno == EINVAL || errno == ERANGE) ereport(FATAL, (errmsg("recovery_target_timeline is not a valid number: \"%s\"", - tok2))); + item->value))); } if (rtli) ereport(DEBUG2, @@ -5203,20 +5100,20 @@ readRecoveryCommandFile(void) ereport(DEBUG2, (errmsg("recovery_target_timeline = latest"))); } - else if (strcmp(tok1, "recovery_target_xid") == 0) + else if (strcmp(item->name, "recovery_target_xid") == 0) { errno = 0; - recoveryTargetXid = (TransactionId) strtoul(tok2, NULL, 0); + recoveryTargetXid = (TransactionId) strtoul(item->value, NULL, 0); if (errno == EINVAL || errno == ERANGE) ereport(FATAL, (errmsg("recovery_target_xid is not a valid number: \"%s\"", - tok2))); + item->value))); ereport(DEBUG2, (errmsg("recovery_target_xid = %u", recoveryTargetXid))); recoveryTarget = RECOVERY_TARGET_XID; } - else if (strcmp(tok1, "recovery_target_time") == 0) + else if (strcmp(item->name, "recovery_target_time") == 0) { /* * if recovery_target_xid specified, then this overrides @@ -5231,44 +5128,44 @@ readRecoveryCommandFile(void) */ recoveryTargetTime = DatumGetTimestampTz(DirectFunctionCall3(timestamptz_in, - CStringGetDatum(tok2), + CStringGetDatum(item->value), ObjectIdGetDatum(InvalidOid), Int32GetDatum(-1))); ereport(DEBUG2, (errmsg("recovery_target_time = '%s'", timestamptz_to_str(recoveryTargetTime)))); } - else if (strcmp(tok1, "recovery_target_inclusive") == 0) + else if (strcmp(item->name, "recovery_target_inclusive") == 0) { /* * does nothing if a recovery_target is not also set */ - if (!parse_bool(tok2, &recoveryTargetInclusive)) + if (!parse_bool(item->value, &recoveryTargetInclusive)) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("parameter \"%s\" requires a Boolean value", "recovery_target_inclusive"))); ereport(DEBUG2, - (errmsg("recovery_target_inclusive = %s", tok2))); + (errmsg("recovery_target_inclusive = %s", item->value))); } - else if (strcmp(tok1, "standby_mode") == 0) + else if (strcmp(item->name, "standby_mode") == 0) { - if (!parse_bool(tok2, &StandbyMode)) + if (!parse_bool(item->value, &StandbyMode)) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("parameter \"%s\" requires a Boolean value", "standby_mode"))); ereport(DEBUG2, - (errmsg("standby_mode = '%s'", tok2))); + (errmsg("standby_mode = '%s'", item->value))); } - else if (strcmp(tok1, "primary_conninfo") == 0) + else if (strcmp(item->name, "primary_conninfo") == 0) { - PrimaryConnInfo = pstrdup(tok2); + PrimaryConnInfo = pstrdup(item->value); ereport(DEBUG2, (errmsg("primary_conninfo = '%s'", PrimaryConnInfo))); } - else if (strcmp(tok1, "trigger_file") == 0) + else if (strcmp(item->name, "trigger_file") == 0) { - TriggerFile = pstrdup(tok2); + TriggerFile = pstrdup(item->value); ereport(DEBUG2, (errmsg("trigger_file = '%s'", TriggerFile))); @@ -5276,17 +5173,9 @@ readRecoveryCommandFile(void) else ereport(FATAL, (errmsg("unrecognized recovery parameter \"%s\"", - tok1))); + item->name))); } - FreeFile(fd); - - if (syntaxError) - ereport(FATAL, - (errmsg("syntax error in recovery command file: %s", - cmdline), - errhint("Lines should have the format parameter = 'value'."))); - /* * Check for compulsory parameters */ @@ -5332,6 +5221,9 @@ readRecoveryCommandFile(void) recoveryTargetTLI = findNewestTimeLine(recoveryTargetTLI); } } + + FreeConfigVariables(head); + FreeFile(fd); } /* diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l index 2986d2f25b..8f6280656d 100644 --- a/src/backend/utils/misc/guc-file.l +++ b/src/backend/utils/misc/guc-file.l @@ -35,25 +35,11 @@ enum { GUC_ERROR = 100 }; -struct name_value_pair -{ - char *name; - char *value; - char *filename; - int sourceline; - struct name_value_pair *next; -}; - static unsigned int ConfigFileLineno; /* flex fails to supply a prototype for yylex, so provide one */ int GUC_yylex(void); -static bool ParseConfigFile(const char *config_file, const char *calling_file, - int depth, int elevel, - struct name_value_pair **head_p, - struct name_value_pair **tail_p); -static void free_name_value_list(struct name_value_pair * list); static char *GUC_scanstr(const char *s); %} @@ -118,7 +104,9 @@ void ProcessConfigFile(GucContext context) { int elevel; - struct name_value_pair *item, *head, *tail; + ConfigVariable *item, + *head, + *tail; char *cvc = NULL; struct config_string *cvc_struct; const char *envvar; @@ -351,50 +339,24 @@ ProcessConfigFile(GucContext context) PgReloadTime = GetCurrentTimestamp(); cleanup_list: - free_name_value_list(head); + FreeConfigVariables(head); if (cvc) free(cvc); } - /* - * Read and parse a single configuration file. This function recurses - * to handle "include" directives. - * - * Input parameters: - * config_file: absolute or relative path of file to read - * calling_file: absolute path of file containing the "include" directive, - * or NULL at outer level (config_file must be absolute at outer level) - * depth: recursion depth (used only to prevent infinite recursion) - * elevel: error logging level determined by ProcessConfigFile() - * Output parameters: - * head_p, tail_p: head and tail of linked list of name/value pairs - * - * *head_p and *tail_p must be initialized to NULL before calling the outer - * 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. + * See next function for details. This one will just work with a config_file + * name rather than an already opened File Descriptor */ -static bool +bool ParseConfigFile(const char *config_file, const char *calling_file, int depth, int elevel, - struct name_value_pair **head_p, - struct name_value_pair **tail_p) + ConfigVariable **head_p, + ConfigVariable **tail_p) { bool OK = true; - char abs_path[MAXPGPATH]; FILE *fp; - YY_BUFFER_STATE lex_buffer; - int token; + char abs_path[MAXPGPATH]; /* * Reject too-deep include nesting depth. This is just a safety check @@ -416,12 +378,23 @@ ParseConfigFile(const char *config_file, const char *calling_file, */ if (!is_absolute_path(config_file)) { - Assert(calling_file != NULL); - strlcpy(abs_path, calling_file, sizeof(abs_path)); - get_parent_directory(abs_path); - join_path_components(abs_path, abs_path, config_file); - canonicalize_path(abs_path); - config_file = abs_path; + if (calling_file != NULL) + { + strlcpy(abs_path, calling_file, sizeof(abs_path)); + get_parent_directory(abs_path); + join_path_components(abs_path, abs_path, config_file); + canonicalize_path(abs_path); + config_file = abs_path; + } + else + { + /* + * calling_file is NULL, we make an absolute path from $PGDATA + */ + join_path_components(abs_path, data_directory, config_file); + canonicalize_path(abs_path); + config_file = abs_path; + } } fp = AllocateFile(config_file, "r"); @@ -434,6 +407,47 @@ ParseConfigFile(const char *config_file, const char *calling_file, return false; } + OK = ParseConfigFp(fp, config_file, depth, elevel, head_p, tail_p); + + FreeFile(fp); + + return OK; +} + +/* + * Read and parse a single configuration file. This function recurses + * to handle "include" directives. + * + * Input parameters: + * fp: file pointer from AllocateFile for the configuration file to parse + * 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() + * Output parameters: + * head_p, tail_p: head and tail of linked list of name/value pairs + * + * *head_p and *tail_p must be initialized to NULL before calling the outer + * 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 */ @@ -446,7 +460,7 @@ ParseConfigFile(const char *config_file, const char *calling_file, while ((token = yylex())) { char *opt_name, *opt_value; - struct name_value_pair *item; + ConfigVariable *item; if (token == GUC_EOL) /* empty or comment line */ continue; @@ -579,23 +593,22 @@ ParseConfigFile(const char *config_file, const char *calling_file, cleanup_exit: yy_delete_buffer(lex_buffer); - FreeFile(fp); return OK; } /* - * Free a list of name/value pairs, including the names and the values + * Free a list of ConfigVariables, including the names and the values */ -static void -free_name_value_list(struct name_value_pair *list) +void +FreeConfigVariables(ConfigVariable *list) { - struct name_value_pair *item; + ConfigVariable *item; item = list; while (item) { - struct name_value_pair *next = item->next; + ConfigVariable *next = item->next; pfree(item->name); pfree(item->value); diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 09beb5f398..dd19fe9e38 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -389,6 +389,7 @@ int trace_recovery_messages = LOG; int num_temp_buffers = 1000; +char *data_directory; char *ConfigFileName; char *HbaFileName; char *IdentFileName; @@ -426,7 +427,6 @@ static char *timezone_string; static char *log_timezone_string; static char *timezone_abbreviations_string; static char *XactIsoLevel_string; -static char *data_directory; static char *custom_variable_classes; static int max_function_args; static int max_index_keys; diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index ae8f2678d3..7143d42bdc 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -96,6 +96,26 @@ typedef enum PGC_S_SESSION /* SET command */ } GucSource; +/* + * Parsing the configuation file will return a list of name-value pairs + */ +typedef struct ConfigVariable +{ + char *name; + char *value; + char *filename; + int sourceline; + 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); + /* * Enum values are made up of an array of name-value pairs */ @@ -176,6 +196,7 @@ extern int log_temp_files; extern int num_temp_buffers; +extern char *data_directory; extern char *ConfigFileName; extern char *HbaFileName; extern char *IdentFileName;