Further cleanup of ps_status setup code. On platforms where the
authorTom Lane
Mon, 22 Oct 2001 19:41:38 +0000 (19:41 +0000)
committerTom Lane
Mon, 22 Oct 2001 19:41:38 +0000 (19:41 +0000)
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.

src/backend/main/main.c
src/backend/postmaster/pgstat.c
src/backend/postmaster/postmaster.c
src/backend/utils/misc/ps_status.c

index 5c4b2accd818b389246ced4e1001a00bdda39399..43d9b6433d8c8272fa339aa28e0f49369d72de96 100644 (file)
@@ -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
@@ -168,27 +194,6 @@ main(int argc, char *argv[])
    setlocale(LC_MONETARY, "");
 #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
index 44ce0b750f6f48d4d817d21353131bfe8cffb927..6aae169202c6816020792e602b3be3eea24b44b2 100644 (file)
@@ -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("");
index 5009185952dd594819261ac1de418a316ca294f7..08a6ce22c1dc0cf75f9cae1ce79891d10bb2c488 100644 (file)
@@ -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)
        {
index 98fff4db5b4191312a7e95912affcdee8b11f066..9441aa0056101d5bbac2442afe850ebfa2baa556 100644 (file)
@@ -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