Further cleanup of ps_status setup code. On platforms where the

environment strings need to be moved around, do so when called from
initial startup (main.c), not in init_ps_status.  This eliminates the
former risk of invalidating saved environment-string pointers, since
no code has yet had a chance to grab any such pointers when main.c
is running.

4 files changed, 95 insertions, 88 deletions

diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index 5c4b2accd81..43d9b6433d8 100644
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -13,7 +13,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/main/main.c,v 1.46 2001/10/21 03:25:35 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/main/main.c,v 1.47 2001/10/22 19:41:38 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -99,6 +99,32 @@ main(int argc, char *argv[])
 	 * best to minimize these.
 	 */
 
+	/*
+	 * Remember the physical location of the initially given argv[] array,
+	 * since on some platforms that storage must be overwritten in order
+	 * to set the process title for ps.  Then make a copy of the argv[]
+	 * array for subsequent use, so that argument parsing doesn't get
+	 * affected if init_ps_display overwrites the original argv[].
+	 *
+	 * (NB: do NOT think to remove the copying of argv[], even though
+	 * postmaster.c finishes looking at argv[] long before we ever consider
+	 * changing the ps display.  On some platforms, getopt() keeps pointers
+	 * into the argv array, and will get horribly confused when it is
+	 * re-called to analyze a subprocess' argument string if the argv storage
+	 * has been clobbered meanwhile.)
+	 *
+	 * On some platforms, save_ps_display_args moves the environment strings
+	 * to make extra room.  Therefore this should be done as early as
+	 * possible during startup, to avoid entanglements with code that might
+	 * save a getenv() result pointer.
+	 */
+	save_ps_display_args(argc, argv);
+
+	new_argv = (char **) malloc((argc + 1) * sizeof(char *));
+	for (i = 0; i < argc; i++)
+		new_argv[i] = strdup(argv[i]);
+	new_argv[argc] = NULL;
+
 	/* Initialize NLS settings so we can give localized error messages */
 #ifdef ENABLE_NLS
 #ifdef LC_MESSAGES
@@ -169,27 +195,6 @@ main(int argc, char *argv[])
 #endif
 
 	/*
-	 * Remember the physical location of the initially given argv[] array,
-	 * since on some platforms that storage must be overwritten in order
-	 * to set the process title for ps.  Then make a copy of the argv[]
-	 * array for subsequent use, so that argument parsing doesn't get
-	 * affected if init_ps_display overwrites the original argv[].
-	 *
-	 * (NB: do NOT think to remove this copying, even though postmaster.c
-	 * finishes looking at argv[] long before we ever consider changing
-	 * the ps display.  On some platforms, getopt(3) keeps pointers into
-	 * the argv array, and will get horribly confused when it is re-called
-	 * to analyze a subprocess' argument string if the argv storage has
-	 * been clobbered meanwhile.)
-	 */
-	save_ps_display_args(argc, argv);
-
-	new_argv = (char **) malloc((argc + 1) * sizeof(char *));
-	for (i = 0; i < argc; i++)
-		new_argv[i] = strdup(argv[i]);
-	new_argv[argc] = NULL;
-
-	/*
 	 * Now dispatch to one of PostmasterMain, PostgresMain, or
 	 * BootstrapMain depending on the program name (and possibly first
 	 * argument) we were called with.  The lack of consistency here is
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 44ce0b750f6..6aae169202c 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -16,7 +16,7 @@
  *
  *	Copyright (c) 2001, PostgreSQL Global Development Group
  *
- *	$Header: /cvsroot/pgsql/src/backend/postmaster/pgstat.c,v 1.12 2001/10/21 03:25:35 tgl Exp $
+ *	$Header: /cvsroot/pgsql/src/backend/postmaster/pgstat.c,v 1.13 2001/10/22 19:41:38 tgl Exp $
  * ----------
  */
 #include "postgres.h"
@@ -1185,9 +1185,6 @@ pgstat_main(void)
 
 	/*
 	 * Identify myself via ps
-	 *
-	 * WARNING: On some platforms the environment will be moved around to
-	 * make room for the ps display string.
 	 */
 	init_ps_display("stats collector process", "", "");
 	set_ps_display("");
@@ -1470,9 +1467,6 @@ pgstat_recvbuffer(void)
 
 	/*
 	 * Identify myself via ps
-	 *
-	 * WARNING: On some platforms the environment will be moved around to
-	 * make room for the ps display string.
 	 */
 	init_ps_display("stats buffer process", "", "");
 	set_ps_display("");
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 5009185952d..08a6ce22c1d 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -37,7 +37,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v 1.250 2001/10/21 03:25:35 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v 1.251 2001/10/22 19:41:38 tgl Exp $
  *
  * NOTES
  *
@@ -2090,12 +2090,7 @@ DoBackend(Port *port)
 	}
 
 	/*
-	 * Set process parameters for ps
-	 *
-	 * WARNING: On some platforms the environment will be moved around to
-	 * make room for the ps display string. So any references to
-	 * optarg or getenv() from above will be invalid after this call.
-	 * Better use strdup or something similar.
+	 * Set process parameters for ps display.
 	 */
 	init_ps_display(port->user, port->database, remote_host);
 	set_ps_display("authentication");
@@ -2443,9 +2438,6 @@ SSDataBase(int xlop)
 
 		/*
 		 * Identify myself via ps
-		 *
-		 * WARNING: On some platforms the environment will be moved around to
-		 * make room for the ps display string.
 		 */
 		switch (xlop)
 		{
diff --git a/src/backend/utils/misc/ps_status.c b/src/backend/utils/misc/ps_status.c
index 98fff4db5b4..9441aa00561 100644
--- a/src/backend/utils/misc/ps_status.c
+++ b/src/backend/utils/misc/ps_status.c
@@ -5,7 +5,7 @@
  * to contain some useful information. Mechanism differs wildly across
  * platforms.
  *
- * $Header: /cvsroot/pgsql/src/backend/utils/misc/ps_status.c,v 1.6 2001/10/21 03:25:35 tgl Exp $
+ * $Header: /cvsroot/pgsql/src/backend/utils/misc/ps_status.c,v 1.7 2001/10/22 19:41:38 tgl Exp $
  *
  * Copyright 2000 by PostgreSQL Global Development Group
  * various details abducted from various places
@@ -95,45 +95,22 @@ static char **save_argv;
 
 /*
  * Call this early in startup to save the original argc/argv values.
+ *
  * argv[] will not be overwritten by this routine, but may be overwritten
- * during init_ps_display.
+ * during init_ps_display.  Also, the physical location of the environment
+ * strings may be moved, so this should be called before any code that
+ * might try to hang onto a getenv() result.
  */
 void
 save_ps_display_args(int argc, char *argv[])
 {
 	save_argc = argc;
 	save_argv = argv;
-}
-
-/*
- * Call this once during subprocess startup to set the identification
- * values.  At this point, the original argv[] array may be overwritten.
- */
-void
-init_ps_display(const char *username, const char *dbname,
-				const char *host_info)
-{
-#ifndef PS_USE_NONE
-	Assert(username);
-	Assert(dbname);
-
-	/* no ps display for stand-alone backend */
-	if (!IsUnderPostmaster)
-		return;
-
-	/* no ps display if you didn't call save_ps_display_args() */
-	if (!save_argv)
-		return;
-
-#ifdef PS_USE_CHANGE_ARGV
-	save_argv[0] = ps_buffer;
-	save_argv[1] = NULL;
-#endif	 /* PS_USE_CHANGE_ARGV */
 
 #ifdef PS_USE_CLOBBER_ARGV
-
 	/*
-	 * If we're going to overwrite the argv area, count the space.
+	 * If we're going to overwrite the argv area, count the available
+	 * space.  Also move the environment to make additional room.
 	 */
 	{
 		char	   *end_of_area = NULL;
@@ -143,35 +120,34 @@ init_ps_display(const char *username, const char *dbname,
 		/*
 		 * check for contiguous argv strings
 		 */
-		for (i = 0; i < save_argc; i++)
-			if (i == 0 || end_of_area + 1 == save_argv[i])
-				end_of_area = save_argv[i] + strlen(save_argv[i]);
-
-		/*
-		 * check for contiguous environ strings following argv
-		 */
-		for (i = 0; end_of_area != NULL && environ[i] != NULL; i++)
-			if (end_of_area + 1 == environ[i])
-				end_of_area = environ[i] + strlen(environ[i]);
+		for (i = 0; i < argc; i++)
+		{
+			if (i == 0 || end_of_area + 1 == argv[i])
+				end_of_area = argv[i] + strlen(argv[i]);
+		}
 
-		if (end_of_area == NULL)
+		if (end_of_area == NULL) /* probably can't happen? */
 		{
 			ps_buffer = NULL;
 			ps_buffer_size = 0;
 			return;
 		}
-		else
+
+		/*
+		 * check for contiguous environ strings following argv
+		 */
+		for (i = 0; environ[i] != NULL; i++)
 		{
-			ps_buffer = save_argv[0];
-			ps_buffer_size = end_of_area - save_argv[0] - 1;
+			if (end_of_area + 1 == environ[i])
+				end_of_area = environ[i] + strlen(environ[i]);
 		}
-		save_argv[1] = NULL;
+
+		ps_buffer = argv[0];
+		ps_buffer_size = end_of_area - argv[0] - 1;
 
 		/*
 		 * move the environment out of the way
 		 */
-		for (i = 0; environ[i] != NULL; i++)
-			;
 		new_environ = malloc(sizeof(char *) * (i + 1));
 		for (i = 0; environ[i] != NULL; i++)
 			new_environ[i] = strdup(environ[i]);
@@ -179,12 +155,52 @@ init_ps_display(const char *username, const char *dbname,
 		environ = new_environ;
 	}
 #endif	 /* PS_USE_CLOBBER_ARGV */
+}
+
+/*
+ * Call this once during subprocess startup to set the identification
+ * values.  At this point, the original argv[] array may be overwritten.
+ */
+void
+init_ps_display(const char *username, const char *dbname,
+				const char *host_info)
+{
+	Assert(username);
+	Assert(dbname);
+	Assert(host_info);
+
+#ifndef PS_USE_NONE
+	/* no ps display for stand-alone backend */
+	if (!IsUnderPostmaster)
+		return;
+
+	/* no ps display if you didn't call save_ps_display_args() */
+	if (!save_argv)
+		return;
+#ifdef PS_USE_CLOBBER_ARGV
+	/* If ps_buffer is a pointer, it might still be null */
+	if (!ps_buffer)
+		return;
+#endif
+
+	/*
+	 * Overwrite argv[] to point at appropriate space, if needed
+	 */
+
+#ifdef PS_USE_CHANGE_ARGV
+	save_argv[0] = ps_buffer;
+	save_argv[1] = NULL;
+#endif	 /* PS_USE_CHANGE_ARGV */
+
+#ifdef PS_USE_CLOBBER_ARGV
+	save_argv[1] = NULL;
+#endif	 /* PS_USE_CLOBBER_ARGV */
 
 	/*
-	 * Make fixed prefix
+	 * Make fixed prefix of ps display.
 	 */
-#ifdef PS_USE_SETPROCTITLE
 
+#ifdef PS_USE_SETPROCTITLE
 	/*
 	 * apparently setproctitle() already adds a `progname:' prefix to the
 	 * ps line