From 94daee3cb79e5e83a8b4917b226ab4a99dea5ed7 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 22 Oct 2001 19:41:38 +0000 Subject: [PATCH] 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. --- src/backend/main/main.c | 49 +++++++----- src/backend/postmaster/pgstat.c | 8 +- src/backend/postmaster/postmaster.c | 12 +-- src/backend/utils/misc/ps_status.c | 120 ++++++++++++++++------------ 4 files changed, 98 insertions(+), 91 deletions(-) diff --git a/src/backend/main/main.c b/src/backend/main/main.c index 5c4b2accd8..43d9b6433d 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 @@ -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 diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index 44ce0b750f..6aae169202 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 5009185952..08a6ce22c1 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 98fff4db5b..9441aa0056 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,14 +95,66 @@ 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; + +#ifdef PS_USE_CLOBBER_ARGV + /* + * 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; + char **new_environ; + int i; + + /* + * check for contiguous argv strings + */ + 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) /* probably can't happen? */ + { + ps_buffer = NULL; + ps_buffer_size = 0; + return; + } + + /* + * check for contiguous environ strings following argv + */ + for (i = 0; environ[i] != NULL; i++) + { + if (end_of_area + 1 == environ[i]) + end_of_area = environ[i] + strlen(environ[i]); + } + + ps_buffer = argv[0]; + ps_buffer_size = end_of_area - argv[0] - 1; + + /* + * move the environment out of the way + */ + new_environ = malloc(sizeof(char *) * (i + 1)); + for (i = 0; environ[i] != NULL; i++) + new_environ[i] = strdup(environ[i]); + new_environ[i] = NULL; + environ = new_environ; + } +#endif /* PS_USE_CLOBBER_ARGV */ } /* @@ -113,10 +165,11 @@ void init_ps_display(const char *username, const char *dbname, const char *host_info) { -#ifndef PS_USE_NONE Assert(username); Assert(dbname); + Assert(host_info); +#ifndef PS_USE_NONE /* no ps display for stand-alone backend */ if (!IsUnderPostmaster) return; @@ -124,6 +177,15 @@ init_ps_display(const char *username, const char *dbname, /* 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; @@ -131,60 +193,14 @@ init_ps_display(const char *username, const char *dbname, #endif /* PS_USE_CHANGE_ARGV */ #ifdef PS_USE_CLOBBER_ARGV - - /* - * If we're going to overwrite the argv area, count the space. - */ - { - char *end_of_area = NULL; - char **new_environ; - int i; - - /* - * 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]); - - if (end_of_area == NULL) - { - ps_buffer = NULL; - ps_buffer_size = 0; - return; - } - else - { - ps_buffer = save_argv[0]; - ps_buffer_size = end_of_area - save_argv[0] - 1; - } - save_argv[1] = NULL; - - /* - * 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]); - new_environ[i] = NULL; - environ = new_environ; - } + 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