Tom Lane wrote:
> Bruce Momjian <br...@momjian.us> writes:
> > Is there a reason tablespace.c::set_short_version() uses PG_VERSION, and
> > not the simpler PG_MAJORVERSION?  initdb.c::get_short_version() does the
> > same thing.
> 
> Probably that code predates the addition of the separate PG_MAJORVERSION
> #define.  +1 for simplifying.  The change I just had to make in
> backend/catalog/Makefile reinforces the thought that MAJORVERSION is
> what we should be using in everyplace related to this type of check.

Agreed.  Fixed the attached, applied patch.

-- 
  Bruce Momjian  <br...@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/commands/tablespace.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/tablespace.c,v
retrieving revision 1.67
diff -c -c -r1.67 tablespace.c
*** src/backend/commands/tablespace.c	6 Jan 2010 01:48:09 -0000	1.67
--- src/backend/commands/tablespace.c	6 Jan 2010 23:22:40 -0000
***************
*** 82,88 ****
  
  
  static bool remove_tablespace_directories(Oid tablespaceoid, bool redo);
! static void set_short_version(const char *path);
  
  
  /*
--- 82,88 ----
  
  
  static bool remove_tablespace_directories(Oid tablespaceoid, bool redo);
! static void write_version_file(const char *path);
  
  
  /*
***************
*** 332,338 ****
  	 * (the emptiness check above will fail), and to label tablespace
  	 * directories by PG version.
  	 */
! 	set_short_version(location);
  
  	/*
  	 * All seems well, create the symlink
--- 332,338 ----
  	 * (the emptiness check above will fail), and to label tablespace
  	 * directories by PG version.
  	 */
! 	write_version_file(location);
  
  	/*
  	 * All seems well, create the symlink
***************
*** 673,718 ****
   * write out the PG_VERSION file in the specified directory
   */
  static void
! set_short_version(const char *path)
  {
- 	char	   *short_version;
- 	bool		gotdot = false;
- 	int			end;
  	char	   *fullname;
  	FILE	   *version_file;
  
- 	/* Construct short version string (should match initdb.c) */
- 	short_version = pstrdup(PG_VERSION);
- 
- 	for (end = 0; short_version[end] != '\0'; end++)
- 	{
- 		if (short_version[end] == '.')
- 		{
- 			Assert(end != 0);
- 			if (gotdot)
- 				break;
- 			else
- 				gotdot = true;
- 		}
- 		else if (short_version[end] < '0' || short_version[end] > '9')
- 		{
- 			/* gone past digits and dots */
- 			break;
- 		}
- 	}
- 	Assert(end > 0 && short_version[end - 1] != '.' && gotdot);
- 	short_version[end] = '\0';
- 
  	/* Now write the file */
  	fullname = palloc(strlen(path) + 11 + 1);
  	sprintf(fullname, "%s/PG_VERSION", path);
! 	version_file = AllocateFile(fullname, PG_BINARY_W);
! 	if (version_file == NULL)
  		ereport(ERROR,
  				(errcode_for_file_access(),
  				 errmsg("could not write to file \"%s\": %m",
  						fullname)));
! 	fprintf(version_file, "%s\n", short_version);
  	if (FreeFile(version_file))
  		ereport(ERROR,
  				(errcode_for_file_access(),
--- 673,693 ----
   * write out the PG_VERSION file in the specified directory
   */
  static void
! write_version_file(const char *path)
  {
  	char	   *fullname;
  	FILE	   *version_file;
  
  	/* Now write the file */
  	fullname = palloc(strlen(path) + 11 + 1);
  	sprintf(fullname, "%s/PG_VERSION", path);
! 
! 	if ((version_file = AllocateFile(fullname, PG_BINARY_W)) == NULL)
  		ereport(ERROR,
  				(errcode_for_file_access(),
  				 errmsg("could not write to file \"%s\": %m",
  						fullname)));
! 	fprintf(version_file, "%s\n", PG_MAJORVERSION);
  	if (FreeFile(version_file))
  		ereport(ERROR,
  				(errcode_for_file_access(),
***************
*** 720,726 ****
  						fullname)));
  
  	pfree(fullname);
- 	pfree(short_version);
  }
  
  /*
--- 695,700 ----
***************
*** 1370,1376 ****
  						 location)));
  
  		/* Create or re-create the PG_VERSION file in the target directory */
! 		set_short_version(location);
  
  		/* Create the symlink if not already present */
  		linkloc = (char *) palloc(OIDCHARS + OIDCHARS + 1);
--- 1344,1350 ----
  						 location)));
  
  		/* Create or re-create the PG_VERSION file in the target directory */
! 		write_version_file(location);
  
  		/* Create the symlink if not already present */
  		linkloc = (char *) palloc(OIDCHARS + OIDCHARS + 1);
Index: src/bin/initdb/initdb.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/initdb/initdb.c,v
retrieving revision 1.181
diff -c -c -r1.181 initdb.c
*** src/bin/initdb/initdb.c	2 Jan 2010 16:57:58 -0000	1.181
--- src/bin/initdb/initdb.c	6 Jan 2010 23:22:41 -0000
***************
*** 156,171 ****
  static void exit_nicely(void);
  static char *get_id(void);
  static char *get_encoding_id(char *encoding_name);
- static char *get_short_version(void);
  static int	check_data_dir(char *dir);
  static bool mkdatadir(const char *subdir);
  static void set_input(char **dest, char *filename);
  static void check_input(char *path);
! static void set_short_version(char *short_version, char *extrapath);
  static void set_null_conf(void);
  static void test_config_settings(void);
  static void setup_config(void);
! static void bootstrap_template1(char *short_version);
  static void setup_auth(void);
  static void get_set_pwd(void);
  static void setup_depend(void);
--- 156,170 ----
  static void exit_nicely(void);
  static char *get_id(void);
  static char *get_encoding_id(char *encoding_name);
  static int	check_data_dir(char *dir);
  static bool mkdatadir(const char *subdir);
  static void set_input(char **dest, char *filename);
  static void check_input(char *path);
! static void write_version_file(char *extrapath);
  static void set_null_conf(void);
  static void test_config_settings(void);
  static void setup_config(void);
! static void bootstrap_template1(void);
  static void setup_auth(void);
  static void get_set_pwd(void);
  static void setup_depend(void);
***************
*** 803,844 ****
  
  
  /*
-  * get short version of VERSION
-  */
- static char *
- get_short_version(void)
- {
- 	bool		gotdot = false;
- 	int			end;
- 	char	   *vr;
- 
- 	vr = xstrdup(PG_VERSION);
- 
- 	for (end = 0; vr[end] != '\0'; end++)
- 	{
- 		if (vr[end] == '.')
- 		{
- 			if (end == 0)
- 				return NULL;
- 			else if (gotdot)
- 				break;
- 			else
- 				gotdot = true;
- 		}
- 		else if (vr[end] < '0' || vr[end] > '9')
- 		{
- 			/* gone past digits and dots */
- 			break;
- 		}
- 	}
- 	if (end == 0 || vr[end - 1] == '.' || !gotdot)
- 		return NULL;
- 
- 	vr[end] = '\0';
- 	return vr;
- }
- 
- /*
   * make sure the directory either doesn't exist or is empty
   *
   * Returns 0 if nonexistent, 1 if exists and empty, 2 if not empty,
--- 802,807 ----
***************
*** 972,978 ****
   * if extrapath is not NULL
   */
  static void
! set_short_version(char *short_version, char *extrapath)
  {
  	FILE	   *version_file;
  	char	   *path;
--- 935,941 ----
   * if extrapath is not NULL
   */
  static void
! write_version_file(char *extrapath)
  {
  	FILE	   *version_file;
  	char	   *path;
***************
*** 987,1000 ****
  		path = pg_malloc(strlen(pg_data) + strlen(extrapath) + 13);
  		sprintf(path, "%s/%s/PG_VERSION", pg_data, extrapath);
  	}
! 	version_file = fopen(path, PG_BINARY_W);
! 	if (version_file == NULL)
  	{
  		fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"),
  				progname, path, strerror(errno));
  		exit_nicely();
  	}
! 	if (fprintf(version_file, "%s\n", short_version) < 0 ||
  		fclose(version_file))
  	{
  		fprintf(stderr, _("%s: could not write file \"%s\": %s\n"),
--- 950,963 ----
  		path = pg_malloc(strlen(pg_data) + strlen(extrapath) + 13);
  		sprintf(path, "%s/%s/PG_VERSION", pg_data, extrapath);
  	}
! 
! 	if ((version_file = fopen(path, PG_BINARY_W)) == NULL)
  	{
  		fprintf(stderr, _("%s: could not open file \"%s\" for writing: %s\n"),
  				progname, path, strerror(errno));
  		exit_nicely();
  	}
! 	if (fprintf(version_file, "%s\n", PG_MAJORVERSION) < 0 ||
  		fclose(version_file))
  	{
  		fprintf(stderr, _("%s: could not write file \"%s\": %s\n"),
***************
*** 1297,1303 ****
   * run the BKI script in bootstrap mode to create template1
   */
  static void
! bootstrap_template1(char *short_version)
  {
  	PG_CMD_DECL;
  	char	  **line;
--- 1260,1266 ----
   * run the BKI script in bootstrap mode to create template1
   */
  static void
! bootstrap_template1(void)
  {
  	PG_CMD_DECL;
  	char	  **line;
***************
*** 1317,1323 ****
  	/* Check that bki file appears to be of the right version */
  
  	snprintf(headerline, sizeof(headerline), "# PostgreSQL %s\n",
! 			 short_version);
  
  	if (strcmp(headerline, *bki_lines) != 0)
  	{
--- 1280,1286 ----
  	/* Check that bki file appears to be of the right version */
  
  	snprintf(headerline, sizeof(headerline), "# PostgreSQL %s\n",
! 			 PG_MAJORVERSION);
  
  	if (strcmp(headerline, *bki_lines) != 0)
  	{
***************
*** 2480,2486 ****
  				i,
  				ret;
  	int			option_index;
- 	char	   *short_version;
  	char	   *effective_user;
  	char	   *pgdenv;			/* PGDATA value gotten from and sent to
  								 * environment */
--- 2443,2448 ----
***************
*** 2788,2799 ****
  
  	canonicalize_path(share_path);
  
- 	if ((short_version = get_short_version()) == NULL)
- 	{
- 		fprintf(stderr, _("%s: could not determine valid short version string\n"), progname);
- 		exit(1);
- 	}
- 
  	effective_user = get_id();
  	if (strlen(username) == 0)
  		username = effective_user;
--- 2750,2755 ----
***************
*** 3123,3129 ****
  	check_ok();
  
  	/* Top level PG_VERSION is checked by bootstrapper, so make it first */
! 	set_short_version(short_version, NULL);
  
  	/* Select suitable configuration settings */
  	set_null_conf();
--- 3079,3085 ----
  	check_ok();
  
  	/* Top level PG_VERSION is checked by bootstrapper, so make it first */
! 	write_version_file(NULL);
  
  	/* Select suitable configuration settings */
  	set_null_conf();
***************
*** 3133,3144 ****
  	setup_config();
  
  	/* Bootstrap template1 */
! 	bootstrap_template1(short_version);
  
  	/*
  	 * Make the per-database PG_VERSION for template1 only after init'ing it
  	 */
! 	set_short_version(short_version, "base/1");
  
  	/* Create the stuff we don't need to use bootstrap mode for */
  
--- 3089,3100 ----
  	setup_config();
  
  	/* Bootstrap template1 */
! 	bootstrap_template1();
  
  	/*
  	 * Make the per-database PG_VERSION for template1 only after init'ing it
  	 */
! 	write_version_file("base/1");
  
  	/* Create the stuff we don't need to use bootstrap mode for */
  
-- 
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