Arrange for the postmaster (and standalone backends, initdb, etc) to

chdir into PGDATA and subsequently use relative paths instead of absolute
paths to access all files under PGDATA.  This seems to give a small
performance improvement, and it should make the system more robust
against naive DBAs doing things like moving a database directory that
has a live postmaster in it.  Per recent discussion.

6 files changed, 93 insertions, 115 deletions

diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 45532539664..867f7da7e2a 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/misc.c,v 1.44 2005/06/19 21:34:02 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/misc.c,v 1.45 2005/07/04 04:51:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -145,10 +145,10 @@ pg_tablespace_databases(PG_FUNCTION_ARGS)
 		fctx = palloc(sizeof(ts_db_fctx));
 
 		/*
-		 * size = path length + tablespace dirname length + 2 dir sep
-		 * chars + oid + terminator
+		 * size = tablespace dirname length + dir sep
+		 * char + oid + terminator
 		 */
-		fctx->location = (char *) palloc(strlen(DataDir) + 11 + 10 + 1);
+		fctx->location = (char *) palloc(10 + 10 + 1);
 		if (tablespaceOid == GLOBALTABLESPACE_OID)
 		{
 			fctx->dirdesc = NULL;
@@ -158,10 +158,9 @@ pg_tablespace_databases(PG_FUNCTION_ARGS)
 		else
 		{
 			if (tablespaceOid == DEFAULTTABLESPACE_OID)
-				sprintf(fctx->location, "%s/base", DataDir);
+				sprintf(fctx->location, "base");
 			else
-				sprintf(fctx->location, "%s/pg_tblspc/%u", DataDir,
-						tablespaceOid);
+				sprintf(fctx->location, "pg_tblspc/%u", tablespaceOid);
 
 			fctx->dirdesc = AllocateDir(fctx->location);
 
diff --git a/src/backend/utils/init/flatfiles.c b/src/backend/utils/init/flatfiles.c
index 2343c01b548..e4b7154b93b 100644
--- a/src/backend/utils/init/flatfiles.c
+++ b/src/backend/utils/init/flatfiles.c
@@ -23,7 +23,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/utils/init/flatfiles.c,v 1.11 2005/06/29 20:34:15 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/init/flatfiles.c,v 1.12 2005/07/04 04:51:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -50,9 +50,9 @@
 #include "utils/syscache.h"
 
 
-/* Actual names of the flat files (within $PGDATA/global/) */
-#define DATABASE_FLAT_FILE	"pg_database"
-#define AUTH_FLAT_FILE		"pg_auth"
+/* Actual names of the flat files (within $PGDATA) */
+#define DATABASE_FLAT_FILE	"global/pg_database"
+#define AUTH_FLAT_FILE		"global/pg_auth"
 
 /* Info bits in a flatfiles 2PC record */
 #define FF_BIT_DATABASE	1
@@ -98,41 +98,29 @@ auth_file_update_needed(void)
 
 
 /*
- * database_getflatfilename --- get full pathname of database file
+ * database_getflatfilename --- get pathname of database file
  *
  * Note that result string is palloc'd, and should be freed by the caller.
+ * (This convention is not really needed anymore, since the relative path
+ * is fixed.)
  */
 char *
 database_getflatfilename(void)
 {
-	int			bufsize;
-	char	   *pfnam;
-
-	bufsize = strlen(DataDir) + strlen("/global/") +
-		strlen(DATABASE_FLAT_FILE) + 1;
-	pfnam = (char *) palloc(bufsize);
-	snprintf(pfnam, bufsize, "%s/global/%s", DataDir, DATABASE_FLAT_FILE);
-
-	return pfnam;
+	return pstrdup(DATABASE_FLAT_FILE);
 }
 
 /*
- * auth_getflatfilename --- get full pathname of auth file
+ * auth_getflatfilename --- get pathname of auth file
  *
  * Note that result string is palloc'd, and should be freed by the caller.
+ * (This convention is not really needed anymore, since the relative path
+ * is fixed.)
  */
 char *
 auth_getflatfilename(void)
 {
-	int			bufsize;
-	char	   *pfnam;
-
-	bufsize = strlen(DataDir) + strlen("/global/") +
-		strlen(AUTH_FLAT_FILE) + 1;
-	pfnam = (char *) palloc(bufsize);
-	snprintf(pfnam, bufsize, "%s/global/%s", DataDir, AUTH_FLAT_FILE);
-
-	return pfnam;
+	return pstrdup(AUTH_FLAT_FILE);
 }
 
 
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 1c9725d6298..a4fb354a4c9 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/init/globals.c,v 1.95 2004/12/31 22:01:40 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/init/globals.c,v 1.96 2005/07/04 04:51:50 tgl Exp $
  *
  * NOTES
  *	  Globals used all over the place should be declared here and not
@@ -36,13 +36,14 @@ int			MyProcPid;
 struct Port *MyProcPort;
 long		MyCancelKey;
 
+/*
+ * DataDir is the absolute path to the top level of the PGDATA directory tree.
+ * Except during early startup, this is also the server's working directory;
+ * most code therefore can simply use relative paths and not reference DataDir
+ * explicitly.
+ */
 char	   *DataDir = NULL;
 
- /*
-  * The PGDATA directory user says to use, or defaults to via environment
-  * variable.  NULL if no option given and no environment variable set
-  */
-
 char		OutputFileName[MAXPGPATH];	/* debugging output file */
 
 char		my_exec_path[MAXPGPATH];	/* full path to my executable */
@@ -56,11 +57,16 @@ char		postgres_exec_path[MAXPGPATH];		/* full path to backend */
 
 BackendId	MyBackendId = InvalidBackendId;
 
-char	   *DatabasePath = NULL;
 Oid			MyDatabaseId = InvalidOid;
 
 Oid			MyDatabaseTableSpace = InvalidOid;
 
+/*
+ * DatabasePath is the path (relative to DataDir) of my database's
+ * primary directory, ie, its directory in the default tablespace.
+ */
+char	   *DatabasePath = NULL;
+
 pid_t		PostmasterPid = 0;
 
 /*
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index be1f6e70496..ffe7db5f7e5 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.144 2005/06/28 22:16:45 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.145 2005/07/04 04:51:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -41,10 +41,11 @@
 #include "utils/syscache.h"
 
 
+#define DIRECTORY_LOCK_FILE		"postmaster.pid"
+
 ProcessingMode Mode = InitProcessing;
 
-/* Note: we rely on these to initialize as zeroes */
-static char directoryLockFile[MAXPGPATH];
+/* Note: we rely on this to initialize as zeroes */
 static char socketLockFile[MAXPGPATH];
 
 
@@ -178,15 +179,32 @@ SetDataDir(const char *dir)
 }
 
 /*
+ * Change working directory to DataDir.  Most of the postmaster and backend
+ * code assumes that we are in DataDir so it can use relative paths to access
+ * stuff in and under the data directory.  For convenience during path
+ * setup, however, we don't force the chdir to occur during SetDataDir.
+ */
+void
+ChangeToDataDir(void)
+{
+	AssertState(DataDir);
+
+	if (chdir(DataDir) < 0)
+		ereport(FATAL,
+				(errcode_for_file_access(),
+				 errmsg("could not change directory to \"%s\": %m",
+						DataDir)));
+}
+
+/*
  * If the given pathname isn't already absolute, make it so, interpreting
  * it relative to the current working directory.
  *
  * Also canonicalizes the path.  The result is always a malloc'd copy.
  *
- * Note: it is probably unwise to use this in running backends, since they
- * have chdir'd to a database-specific subdirectory; the results would not be
- * consistent across backends.  Currently this is used only during postmaster
- * or standalone-backend startup.
+ * Note: interpretation of relative-path arguments during postmaster startup
+ * should happen before doing ChangeToDataDir(), else the user will probably
+ * not like the results.
  */
 char *
 make_absolute_path(const char *path)
@@ -713,17 +731,22 @@ CreateLockFile(const char *filename, bool amPostmaster,
 	on_proc_exit(UnlinkLockFile, PointerGetDatum(strdup(filename)));
 }
 
+/*
+ * Create the data directory lockfile.
+ *
+ * When this is called, we must have already switched the working
+ * directory to DataDir, so we can just use a relative path.  This
+ * helps ensure that we are locking the directory we should be.
+ */
 void
-CreateDataDirLockFile(const char *datadir, bool amPostmaster)
+CreateDataDirLockFile(bool amPostmaster)
 {
-	char		lockfile[MAXPGPATH];
-
-	snprintf(lockfile, sizeof(lockfile), "%s/postmaster.pid", datadir);
-	CreateLockFile(lockfile, amPostmaster, true, datadir);
-	/* Save name of lockfile for RecordSharedMemoryInLockFile */
-	strcpy(directoryLockFile, lockfile);
+	CreateLockFile(DIRECTORY_LOCK_FILE, amPostmaster, true, DataDir);
 }
 
+/*
+ * Create a lockfile for the specified Unix socket file.
+ */
 void
 CreateSocketLockFile(const char *socketfile, bool amPostmaster)
 {
@@ -777,7 +800,7 @@ TouchSocketLockFile(void)
 
 /*
  * Append information about a shared memory segment to the data directory
- * lock file (if we have created one).
+ * lock file.
  *
  * This may be called multiple times in the life of a postmaster, if we
  * delete and recreate shmem due to backend crash.	Therefore, be prepared
@@ -793,20 +816,13 @@ RecordSharedMemoryInLockFile(unsigned long id1, unsigned long id2)
 	char	   *ptr;
 	char		buffer[BLCKSZ];
 
-	/*
-	 * Do nothing if we did not create a lockfile (probably because we are
-	 * running standalone).
-	 */
-	if (directoryLockFile[0] == '\0')
-		return;
-
-	fd = open(directoryLockFile, O_RDWR | PG_BINARY, 0);
+	fd = open(DIRECTORY_LOCK_FILE, O_RDWR | PG_BINARY, 0);
 	if (fd < 0)
 	{
 		ereport(LOG,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\": %m",
-						directoryLockFile)));
+						DIRECTORY_LOCK_FILE)));
 		return;
 	}
 	len = read(fd, buffer, sizeof(buffer) - 100);
@@ -815,7 +831,7 @@ RecordSharedMemoryInLockFile(unsigned long id1, unsigned long id2)
 		ereport(LOG,
 				(errcode_for_file_access(),
 				 errmsg("could not read from file \"%s\": %m",
-						directoryLockFile)));
+						DIRECTORY_LOCK_FILE)));
 		close(fd);
 		return;
 	}
@@ -828,7 +844,7 @@ RecordSharedMemoryInLockFile(unsigned long id1, unsigned long id2)
 	if (ptr == NULL ||
 		(ptr = strchr(ptr + 1, '\n')) == NULL)
 	{
-		elog(LOG, "bogus data in \"%s\"", directoryLockFile);
+		elog(LOG, "bogus data in \"%s\"", DIRECTORY_LOCK_FILE);
 		close(fd);
 		return;
 	}
@@ -855,7 +871,7 @@ RecordSharedMemoryInLockFile(unsigned long id1, unsigned long id2)
 		ereport(LOG,
 				(errcode_for_file_access(),
 				 errmsg("could not write to file \"%s\": %m",
-						directoryLockFile)));
+						DIRECTORY_LOCK_FILE)));
 		close(fd);
 		return;
 	}
@@ -864,7 +880,7 @@ RecordSharedMemoryInLockFile(unsigned long id1, unsigned long id2)
 		ereport(LOG,
 				(errcode_for_file_access(),
 				 errmsg("could not write to file \"%s\": %m",
-						directoryLockFile)));
+						DIRECTORY_LOCK_FILE)));
 	}
 }
 
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 21b0650e827..18e60cab0f1 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/init/postinit.c,v 1.151 2005/06/28 19:51:23 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/init/postinit.c,v 1.152 2005/07/04 04:51:50 tgl Exp $
  *
  *
  *-------------------------------------------------------------------------
@@ -338,12 +338,6 @@ InitPostgres(const char *dbname, const char *username)
 
 		ValidatePgVersion(fullpath);
 
-		if (chdir(fullpath) == -1)
-			ereport(FATAL,
-					(errcode_for_file_access(),
-					 errmsg("could not change directory to \"%s\": %m",
-							fullpath)));
-
 		SetDatabasePath(fullpath);
 	}
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 84d8085503a..0ea335770c3 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -10,7 +10,7 @@
  * Written by Peter Eisentraut <peter_e@gmx.net>.
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.271 2005/06/28 05:09:02 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.272 2005/07/04 04:51:51 tgl Exp $
  *
  *--------------------------------------------------------------------
  */
@@ -74,6 +74,7 @@
 
 #ifdef EXEC_BACKEND
 #define CONFIG_EXEC_PARAMS "global/config_exec_params"
+#define CONFIG_EXEC_PARAMS_NEW "global/config_exec_params.new"
 #endif
 
 /* XXX these should appear in other modules' header files */
@@ -2619,7 +2620,9 @@ SelectConfigFiles(const char *userDoption, const char *progname)
 	 * Reflect the final DataDir value back into the data_directory GUC var.
 	 * (If you are wondering why we don't just make them a single variable,
 	 * it's because the EXEC_BACKEND case needs DataDir to be transmitted to
-	 * child backends specially.)
+	 * child backends specially.  XXX is that still true?  Given that we
+	 * now chdir to DataDir, EXEC_BACKEND can read the config file without
+	 * knowing DataDir in advance.)
 	 */
 	SetConfigOption("data_directory", DataDir, PGC_POSTMASTER, PGC_S_OVERRIDE);
 
@@ -4849,6 +4852,7 @@ _ShowOption(struct config_generic * record)
 
 
 #ifdef EXEC_BACKEND
+
 /*
  *	This routine dumps out all non-default GUC options into a binary
  *	file that is read by all exec'ed backends.  The format is:
@@ -4861,42 +4865,23 @@ void
 write_nondefault_variables(GucContext context)
 {
 	int			i;
-	char	   *new_filename,
-			   *filename;
 	int			elevel;
 	FILE	   *fp;
 
 	Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
-	Assert(DataDir);
 
 	elevel = (context == PGC_SIGHUP) ? LOG : ERROR;
 
 	/*
 	 * Open file
 	 */
-	new_filename = guc_malloc(elevel, strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) +
-							  strlen(".new") + 2);
-	if (new_filename == NULL)
-		return;
-
-	filename = guc_malloc(elevel, strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) + 2);
-	if (filename == NULL)
-	{
-		free(new_filename);
-		return;
-	}
-
-	sprintf(new_filename, "%s/" CONFIG_EXEC_PARAMS ".new", DataDir);
-	sprintf(filename, "%s/" CONFIG_EXEC_PARAMS, DataDir);
-
-	fp = AllocateFile(new_filename, "w");
+	fp = AllocateFile(CONFIG_EXEC_PARAMS_NEW, "w");
 	if (!fp)
 	{
-		free(new_filename);
-		free(filename);
 		ereport(elevel,
 				(errcode_for_file_access(),
-				 errmsg("could not write to file \"%s\": %m", CONFIG_EXEC_PARAMS)));
+				 errmsg("could not write to file \"%s\": %m",
+						CONFIG_EXEC_PARAMS_NEW)));
 		return;
 	}
 
@@ -4956,11 +4941,10 @@ write_nondefault_variables(GucContext context)
 
 	if (FreeFile(fp))
 	{
-		free(new_filename);
-		free(filename);
 		ereport(elevel,
 				(errcode_for_file_access(),
-				 errmsg("could not write to file \"%s\": %m", CONFIG_EXEC_PARAMS)));
+				 errmsg("could not write to file \"%s\": %m",
+						CONFIG_EXEC_PARAMS_NEW)));
 		return;
 	}
 
@@ -4968,9 +4952,7 @@ write_nondefault_variables(GucContext context)
 	 * Put new file in place.  This could delay on Win32, but we don't
 	 * hold any exclusive locks.
 	 */
-	rename(new_filename, filename);
-	free(new_filename);
-	free(filename);
+	rename(CONFIG_EXEC_PARAMS_NEW, CONFIG_EXEC_PARAMS);
 }
 
 
@@ -5014,29 +4996,23 @@ read_string_with_null(FILE *fp)
 void
 read_nondefault_variables(void)
 {
-	char	   *filename;
 	FILE	   *fp;
 	char	   *varname,
 			   *varvalue;
 	int			varsource;
 
-	Assert(DataDir);
-
 	/*
 	 * Open file
 	 */
-	filename = guc_malloc(FATAL, strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) + 2);
-	sprintf(filename, "%s/" CONFIG_EXEC_PARAMS, DataDir);
-
-	fp = AllocateFile(filename, "r");
+	fp = AllocateFile(CONFIG_EXEC_PARAMS, "r");
 	if (!fp)
 	{
-		free(filename);
 		/* File not found is fine */
 		if (errno != ENOENT)
 			ereport(FATAL,
 					(errcode_for_file_access(),
-					 errmsg("could not read from file \"%s\": %m", CONFIG_EXEC_PARAMS)));
+					 errmsg("could not read from file \"%s\": %m",
+							CONFIG_EXEC_PARAMS)));
 		return;
 	}
 
@@ -5061,10 +5037,9 @@ read_nondefault_variables(void)
 	}
 
 	FreeFile(fp);
-	free(filename);
-	return;
 }
-#endif
+
+#endif /* EXEC_BACKEND */
 
 
 /*