Alvaro Herrera <alvhe...@commandprompt.com> writes:
> the handling of relative vs absolute paths is bogus here.  I think it'd
> make more sense to have a bool "are we including"; and if that's false and
> the path is not absolute, then the file is relative to CWD; or maybe we
> make it absolute by prepending PGDATA; maybe something else?  (need to
> think of something that makes sense for both recovery.conf and extension
> control files)

Current coding in extensions prepend any control or script file with
sharepath, so that we're only dealing with absolute filename here. The
idea is that it's no business for any other part of the code to have to
know where we decide to install control and script files.

My feeling is that when !is_absolute_path(config_file) and calling_file
is NULL we should make the config_file absolute by prepending PGDATA.
Please find that done in attached v4 of the cfparser patch.

>> If that looks ok, do we want to add some documentation about the new
>> lexer capabilities?
>
> beyond extra code comments?  probably not.

Great.

>> Also, for what good reason would we want to prevent
>> people from using the include facility?
>
> Not sure about this

Ok, nothing special here.
-- 
Dimitri Fontaine
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support

*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
***************
*** 5024,5200 **** str_time(pg_time_t tnow)
  }
  
  /*
-  * 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
   */
  static void
  readRecoveryCommandFile(void)
  {
  	FILE	   *fd;
- 	char		cmdline[MAXPGPATH];
  	TimeLineID	rtli = 0;
  	bool		rtliGiven = false;
! 	bool		syntaxError = false;
  
  	fd = AllocateFile(RECOVERY_COMMAND_FILE, "r");
  	if (fd == NULL)
  	{
  		if (errno == ENOENT)
! 			return;				/* not there, so no archive recovery */
  		ereport(FATAL,
  				(errcode_for_file_access(),
  				 errmsg("could not open recovery command file \"%s\": %m",
  						RECOVERY_COMMAND_FILE)));
  	}
  
! 	/*
! 	 * Parse the file...
! 	 */
! 	while (fgets(cmdline, sizeof(cmdline), fd) != NULL)
! 	{
! 		char	   *tok1;
! 		char	   *tok2;
! 
! 		if (!parseRecoveryCommandFileLine(cmdline, &tok1, &tok2))
! 		{
! 			syntaxError = true;
! 			break;
! 		}
! 		if (tok1 == NULL)
! 			continue;
  
! 		if (strcmp(tok1, "restore_command") == 0)
  		{
! 			recoveryRestoreCommand = pstrdup(tok2);
  			ereport(DEBUG2,
  					(errmsg("restore_command = '%s'",
  							recoveryRestoreCommand)));
  		}
! 		else if (strcmp(tok1, "recovery_end_command") == 0)
  		{
! 			recoveryEndCommand = pstrdup(tok2);
  			ereport(DEBUG2,
  					(errmsg("recovery_end_command = '%s'",
  							recoveryEndCommand)));
  		}
! 		else if (strcmp(tok1, "archive_cleanup_command") == 0)
  		{
! 			archiveCleanupCommand = pstrdup(tok2);
  			ereport(DEBUG2,
  					(errmsg("archive_cleanup_command = '%s'",
  							archiveCleanupCommand)));
  		}
! 		else if (strcmp(tok1, "recovery_target_timeline") == 0)
  		{
  			rtliGiven = true;
! 			if (strcmp(tok2, "latest") == 0)
  				rtli = 0;
  			else
  			{
  				errno = 0;
! 				rtli = (TimeLineID) strtoul(tok2, NULL, 0);
  				if (errno == EINVAL || errno == ERANGE)
  					ereport(FATAL,
  							(errmsg("recovery_target_timeline is not a valid number: \"%s\"",
! 									tok2)));
  			}
  			if (rtli)
  				ereport(DEBUG2,
--- 5024,5096 ----
  }
  
  /*
   * See if there is a recovery command file (recovery.conf), and if so
   * read in parameters for archive recovery and XLOG streaming.
   *
!  * The file is parsed using the main configuration parser.
   */
  static void
  readRecoveryCommandFile(void)
  {
  	FILE	   *fd;
  	TimeLineID	rtli = 0;
  	bool		rtliGiven = false;
! 	ConfigNameValuePair item, head, tail;
  
+ 	/*
+ 	 * See if recovery.conf file is present
+ 	 */
  	fd = AllocateFile(RECOVERY_COMMAND_FILE, "r");
  	if (fd == NULL)
  	{
  		if (errno == ENOENT)
! 			return;                         /* not there, so no archive recovery */
  		ereport(FATAL,
  				(errcode_for_file_access(),
  				 errmsg("could not open recovery command file \"%s\": %m",
  						RECOVERY_COMMAND_FILE)));
  	}
+ 	FreeFile(fd);
  
! 	if (!ParseConfigFile(RECOVERY_COMMAND_FILE, NULL, 0, FATAL, &head, &tail))
! 		goto cleanup_list;
  
! 	for (item = head; item; item = item->next)
! 	{
! 		if (strcmp(item->name, "restore_command") == 0)
  		{
! 			recoveryRestoreCommand = pstrdup(item->value);
  			ereport(DEBUG2,
  					(errmsg("restore_command = '%s'",
  							recoveryRestoreCommand)));
  		}
! 		else if (strcmp(item->name, "recovery_end_command") == 0)
  		{
! 			recoveryEndCommand = pstrdup(item->value);
  			ereport(DEBUG2,
  					(errmsg("recovery_end_command = '%s'",
  							recoveryEndCommand)));
  		}
! 		else if (strcmp(item->name, "archive_cleanup_command") == 0)
  		{
! 			archiveCleanupCommand = pstrdup(item->value);
  			ereport(DEBUG2,
  					(errmsg("archive_cleanup_command = '%s'",
  							archiveCleanupCommand)));
  		}
! 		else if (strcmp(item->name, "recovery_target_timeline") == 0)
  		{
  			rtliGiven = true;
! 			if (strcmp(item->value, "latest") == 0)
  				rtli = 0;
  			else
  			{
  				errno = 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\"",
! 									item->value)));
  			}
  			if (rtli)
  				ereport(DEBUG2,
***************
*** 5203,5222 **** readRecoveryCommandFile(void)
  				ereport(DEBUG2,
  						(errmsg("recovery_target_timeline = latest")));
  		}
! 		else if (strcmp(tok1, "recovery_target_xid") == 0)
  		{
  			errno = 0;
! 			recoveryTargetXid = (TransactionId) strtoul(tok2, NULL, 0);
  			if (errno == EINVAL || errno == ERANGE)
  				ereport(FATAL,
  				 (errmsg("recovery_target_xid is not a valid number: \"%s\"",
! 						 tok2)));
  			ereport(DEBUG2,
  					(errmsg("recovery_target_xid = %u",
  							recoveryTargetXid)));
  			recoveryTarget = RECOVERY_TARGET_XID;
  		}
! 		else if (strcmp(tok1, "recovery_target_time") == 0)
  		{
  			/*
  			 * if recovery_target_xid specified, then this overrides
--- 5099,5118 ----
  				ereport(DEBUG2,
  						(errmsg("recovery_target_timeline = latest")));
  		}
! 		else if (strcmp(item->name, "recovery_target_xid") == 0)
  		{
  			errno = 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\"",
! 						 item->value)));
  			ereport(DEBUG2,
  					(errmsg("recovery_target_xid = %u",
  							recoveryTargetXid)));
  			recoveryTarget = RECOVERY_TARGET_XID;
  		}
! 		else if (strcmp(item->name, "recovery_target_time") == 0)
  		{
  			/*
  			 * if recovery_target_xid specified, then this overrides
***************
*** 5231,5274 **** readRecoveryCommandFile(void)
  			 */
  			recoveryTargetTime =
  				DatumGetTimestampTz(DirectFunctionCall3(timestamptz_in,
! 														CStringGetDatum(tok2),
  												ObjectIdGetDatum(InvalidOid),
  														Int32GetDatum(-1)));
  			ereport(DEBUG2,
  					(errmsg("recovery_target_time = '%s'",
  							timestamptz_to_str(recoveryTargetTime))));
  		}
! 		else if (strcmp(tok1, "recovery_target_inclusive") == 0)
  		{
  			/*
  			 * does nothing if a recovery_target is not also set
  			 */
! 			if (!parse_bool(tok2, &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)));
  		}
! 		else if (strcmp(tok1, "standby_mode") == 0)
  		{
! 			if (!parse_bool(tok2, &StandbyMode))
  				ereport(ERROR,
  						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
  						 errmsg("parameter \"%s\" requires a Boolean value", "standby_mode")));
  			ereport(DEBUG2,
! 					(errmsg("standby_mode = '%s'", tok2)));
  		}
! 		else if (strcmp(tok1, "primary_conninfo") == 0)
  		{
! 			PrimaryConnInfo = pstrdup(tok2);
  			ereport(DEBUG2,
  					(errmsg("primary_conninfo = '%s'",
  							PrimaryConnInfo)));
  		}
! 		else if (strcmp(tok1, "trigger_file") == 0)
  		{
! 			TriggerFile = pstrdup(tok2);
  			ereport(DEBUG2,
  					(errmsg("trigger_file = '%s'",
  							TriggerFile)));
--- 5127,5170 ----
  			 */
  			recoveryTargetTime =
  				DatumGetTimestampTz(DirectFunctionCall3(timestamptz_in,
! 														CStringGetDatum(item->value),
  												ObjectIdGetDatum(InvalidOid),
  														Int32GetDatum(-1)));
  			ereport(DEBUG2,
  					(errmsg("recovery_target_time = '%s'",
  							timestamptz_to_str(recoveryTargetTime))));
  		}
! 		else if (strcmp(item->name, "recovery_target_inclusive") == 0)
  		{
  			/*
  			 * does nothing if a recovery_target is not also set
  			 */
! 			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", item->value)));
  		}
! 		else if (strcmp(item->name, "standby_mode") == 0)
  		{
! 			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'", item->value)));
  		}
! 		else if (strcmp(item->name, "primary_conninfo") == 0)
  		{
! 			PrimaryConnInfo = pstrdup(item->value);
  			ereport(DEBUG2,
  					(errmsg("primary_conninfo = '%s'",
  							PrimaryConnInfo)));
  		}
! 		else if (strcmp(item->name, "trigger_file") == 0)
  		{
! 			TriggerFile = pstrdup(item->value);
  			ereport(DEBUG2,
  					(errmsg("trigger_file = '%s'",
  							TriggerFile)));
***************
*** 5276,5292 **** readRecoveryCommandFile(void)
  		else
  			ereport(FATAL,
  					(errmsg("unrecognized recovery parameter \"%s\"",
! 							tok1)));
  	}
  
- 	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
  	 */
--- 5172,5180 ----
  		else
  			ereport(FATAL,
  					(errmsg("unrecognized recovery parameter \"%s\"",
! 							item->name)));
  	}
  
  	/*
  	 * Check for compulsory parameters
  	 */
***************
*** 5332,5337 **** readRecoveryCommandFile(void)
--- 5220,5228 ----
  			recoveryTargetTLI = findNewestTimeLine(recoveryTargetTLI);
  		}
  	}
+ 
+  cleanup_list:
+ 	free_name_value_list(head);
  }
  
  /*
*** a/src/backend/utils/misc/guc-file.l
--- b/src/backend/utils/misc/guc-file.l
***************
*** 35,59 **** 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);
  
  %}
--- 35,45 ----
***************
*** 118,124 **** void
  ProcessConfigFile(GucContext context)
  {
  	int			elevel;
! 	struct name_value_pair *item, *head, *tail;
  	char	   *cvc = NULL;
  	struct config_string *cvc_struct;
  	const char *envvar;
--- 104,110 ----
  ProcessConfigFile(GucContext context)
  {
  	int			elevel;
! 	ConfigNameValuePair item, head, tail;
  	char	   *cvc = NULL;
  	struct config_string *cvc_struct;
  	const char *envvar;
***************
*** 384,390 **** ProcessConfigFile(GucContext context)
   * 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.
   */
! static bool
  ParseConfigFile(const char *config_file, const char *calling_file,
  				int depth, int elevel,
  				struct name_value_pair **head_p,
--- 370,376 ----
   * 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
  ParseConfigFile(const char *config_file, const char *calling_file,
  				int depth, int elevel,
  				struct name_value_pair **head_p,
***************
*** 416,427 **** 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;
  	}
  
  	fp = AllocateFile(config_file, "r");
--- 402,424 ----
  	 */
  	if (!is_absolute_path(config_file))
  	{
! 		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");
***************
*** 587,593 **** cleanup_exit:
  /*
   * Free a list of name/value pairs, including the names and the values
   */
! static void
  free_name_value_list(struct name_value_pair *list)
  {
  	struct name_value_pair *item;
--- 584,590 ----
  /*
   * Free a list of name/value pairs, including the names and the values
   */
! void
  free_name_value_list(struct name_value_pair *list)
  {
  	struct name_value_pair *item;
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 403,408 **** int			tcp_keepalives_interval;
--- 403,414 ----
  int			tcp_keepalives_count;
  
  /*
+  * This is part of the dummy variables, but it needs to be exported so that
+  * guc-file.l can create $PGDATA/some.conf out of relative path.
+  */
+ char *data_directory;
+ 
+ /*
   * These variables are all dummies that don't do anything, except in some
   * cases provide the value for SHOW to display.  The real state is elsewhere
   * and is kept in sync by assign_hooks.
***************
*** 426,432 **** 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;
--- 432,437 ----
*** a/src/include/utils/guc.h
--- b/src/include/utils/guc.h
***************
*** 17,23 ****
  #include "tcop/dest.h"
  #include "utils/array.h"
  
- 
  /*
   * Certain options can only be set at certain times. The rules are
   * like this:
--- 17,22 ----
***************
*** 97,102 **** typedef enum
--- 96,122 ----
  } GucSource;
  
  /*
+  * Parsing the configuation file will return a list of name-value pairs
+  */
+ struct name_value_pair
+ {
+ 	char       *name;
+ 	char       *value;
+ 	char	   *filename;
+ 	int			sourceline;
+ 	struct name_value_pair *next;
+ } name_value_pair;
+ 
+ typedef struct name_value_pair *ConfigNameValuePair;
+ 
+ bool ParseConfigFile(const char *config_file, const char *calling_file,
+ 							int depth, int elevel,
+ 							ConfigNameValuePair *head_p,
+ 							ConfigNameValuePair *tail_p);
+ 
+ void free_name_value_list(struct name_value_pair * list);
+ 
+ /*
   * Enum values are made up of an array of name-value pairs
   */
  struct config_enum_entry
***************
*** 187,192 **** extern int	tcp_keepalives_idle;
--- 207,215 ----
  extern int	tcp_keepalives_interval;
  extern int	tcp_keepalives_count;
  
+ /* guc-file.l needs that to make absolute paths from relative ones */
+ extern char *data_directory;
+ 
  extern void SetConfigOption(const char *name, const char *value,
  				GucContext context, GucSource source);
  
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to