Use GUC lexer for recovery.conf parsing.
authorRobert Haas <[email protected]>
Fri, 3 Dec 2010 13:44:15 +0000 (08:44 -0500)
committerRobert Haas <[email protected]>
Fri, 3 Dec 2010 13:56:44 +0000 (08:56 -0500)
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
src/backend/utils/misc/guc-file.l
src/backend/utils/misc/guc.c
src/include/utils/guc.h

index ede6ceb6affc30cdeb95f3cbb5b27deac5ee420f..ae9ed8fe4ceec9699d386d00667914e84190d86a 100644 (file)
@@ -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)
-   {
-       char       *tok1;
-       char       *tok2;
+    * 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);
 
-       if (!parseRecoveryCommandFileLine(cmdline, &tok1, &tok2))
-       {
-           syntaxError = true;
-           break;
-       }
-       if (tok1 == NULL)
-           continue;
-
-       if (strcmp(tok1, "restore_command") == 0)
+   for (item = head; item; item = item->next)
+   {
+       if (strcmp(item->name, "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);
 }
 
 /*
index 2986d2f25ba96f6f6ba331ff24f41dabae9b952c..8f6280656dd87d1254d9832a911db76abfb187fc 100644 (file)
@@ -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);
index 09beb5f39850ac4104b644f81769800bd355b4c3..dd19fe9e386d12aaac375f36959d2d2f72cca907 100644 (file)
@@ -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;
index ae8f2678d394d391acecca057def2327d98f8465..7143d42bdc6ccdbc694690a27ca2f48eab45ba1b 100644 (file)
@@ -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;