Re: [BUGS] BUG #2195: log_min_messages crash server when in - Mailing list pgsql-patches

Applied to HEAD, 8.1.X and 8.0.X.

---------------------------------------------------------------------------

Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > The problem is that for EXEC_BACKEND, which is the default for WIN32 and
> > > what I used for testing, progname is NULL because we do not set it after
> > > the fork/exec.  Most versions of printf report "(null)" for a null
> > > argument to printf("%s"), but our port/snprintf.c version aborts, which
> > > is probably better in terms of finding bugs.
> >
> > Good catch.
> >
> > > The attached patch fixes the problem by setting progname early in
> > > main(), like we do for all other applications.
> >
> > It'd probably be best to strdup() argv[0], because of the likelihood
> > that the argv area will get clobbered at some point by ps_status.c.
> > It was OK to have a private static in postmaster.c, because the
> > postmaster doesn't ever call ps_status.c, but if we are going to expose
> > the variable in child processes it needs to be more globally valid.
> >
> > I see that get_progname strdup's its argument in some cases but not
> > all --- so simplest fix is to make it always do so.
>
> OK, new version that updates get_progname to always strdup().
>
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 359-1001
>   +  If your life is a hard drive,     |  13 Roberts Road
>   +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

> Index: src/backend/main/main.c
> ===================================================================
> RCS file: /cvsroot/pgsql/src/backend/main/main.c,v
> retrieving revision 1.99
> diff -c -c -r1.99 main.c
> *** src/backend/main/main.c    5 Jan 2006 03:01:34 -0000    1.99
> --- src/backend/main/main.c    31 Jan 2006 21:03:01 -0000
> ***************
> *** 45,51 ****
>   #include "libpq/pqsignal.h"
>   #endif
>
> !
>
>   int
>   main(int argc, char *argv[])
> --- 45,51 ----
>   #include "libpq/pqsignal.h"
>   #endif
>
> ! const char *progname;
>
>   int
>   main(int argc, char *argv[])
> ***************
> *** 77,82 ****
> --- 77,84 ----
>       char       *env_locale;
>   #endif
>
> +     progname = get_progname(argv[0]);
> +
>       /*
>        * On some platforms, unaligned memory accesses result in a kernel trap;
>        * the default kernel behavior is to emulate the memory access, but this
> ***************
> *** 246,252 ****
>        * possibly first argument) we were called with. The lack of consistency
>        * here is historical.
>        */
> !     if (strcmp(get_progname(argv[0]), "postmaster") == 0)
>       {
>           /* Called as "postmaster" */
>           exit(PostmasterMain(argc, argv));
> --- 248,254 ----
>        * possibly first argument) we were called with. The lack of consistency
>        * here is historical.
>        */
> !     if (strcmp(progname, "postmaster") == 0)
>       {
>           /* Called as "postmaster" */
>           exit(PostmasterMain(argc, argv));
> Index: src/backend/postmaster/postmaster.c
> ===================================================================
> RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
> retrieving revision 1.479
> diff -c -c -r1.479 postmaster.c
> *** src/backend/postmaster/postmaster.c    6 Jan 2006 02:58:25 -0000    1.479
> --- src/backend/postmaster/postmaster.c    31 Jan 2006 21:03:04 -0000
> ***************
> *** 171,179 ****
>    */
>   int            ReservedBackends;
>
> -
> - static const char *progname = NULL;
> -
>   /* The socket(s) we're listening to. */
>   #define MAXLISTEN    64
>   static int    ListenSocket[MAXLISTEN];
> --- 171,176 ----
> ***************
> *** 383,391 ****
>       char       *userDoption = NULL;
>       int            i;
>
> -     /* This will call exit() if strdup() fails. */
> -     progname = get_progname(argv[0]);
> -
>       MyProcPid = PostmasterPid = getpid();
>
>       IsPostmasterEnvironment = true;
> --- 380,385 ----
> Index: src/include/postmaster/postmaster.h
> ===================================================================
> RCS file: /cvsroot/pgsql/src/include/postmaster/postmaster.h,v
> retrieving revision 1.11
> diff -c -c -r1.11 postmaster.h
> *** src/include/postmaster/postmaster.h    20 Aug 2005 23:26:33 -0000    1.11
> --- src/include/postmaster/postmaster.h    31 Jan 2006 21:03:06 -0000
> ***************
> *** 34,39 ****
> --- 34,40 ----
>   extern HANDLE PostmasterHandle;
>   #endif
>
> + extern const char *progname;
>
>   extern int    PostmasterMain(int argc, char *argv[]);
>   extern void ClosePostmasterPorts(bool am_syslogger);
> Index: src/port/path.c
> ===================================================================
> RCS file: /cvsroot/pgsql/src/port/path.c,v
> retrieving revision 1.63
> diff -c -c -r1.63 path.c
> *** src/port/path.c    23 Dec 2005 22:34:22 -0000    1.63
> --- src/port/path.c    31 Jan 2006 21:03:06 -0000
> ***************
> *** 388,394 ****
>   const char *
>   get_progname(const char *argv0)
>   {
> !     const char *nodir_name;
>
>       nodir_name = last_dir_separator(argv0);
>       if (nodir_name)
> --- 388,395 ----
>   const char *
>   get_progname(const char *argv0)
>   {
> !     const char  *nodir_name;
> !     const char    *progname;
>
>       nodir_name = last_dir_separator(argv0);
>       if (nodir_name)
> ***************
> *** 396,420 ****
>       else
>           nodir_name = skip_drive(argv0);
>
> ! #if defined(__CYGWIN__) || defined(WIN32)
> !     /* strip .exe suffix, regardless of case */
> !     if (strlen(nodir_name) > sizeof(EXE) - 1 &&
> !         pg_strcasecmp(nodir_name + strlen(nodir_name) - (sizeof(EXE) - 1), EXE) == 0)
>       {
> !         char       *progname;
>
> !         progname = strdup(nodir_name);    /* leaks memory, but called only once */
> !         if (progname == NULL)
> !         {
> !             fprintf(stderr, "%s: out of memory\n", nodir_name);
> !             exit(1);            /* This could exit the postmaster */
> !         }
>           progname[strlen(progname) - (sizeof(EXE) - 1)] = '\0';
> -         nodir_name = progname;
> -     }
>   #endif
>
> !     return nodir_name;
>   }
>
>
> --- 397,421 ----
>       else
>           nodir_name = skip_drive(argv0);
>
> !     /*
> !      *    Make a copy in case argv[0] is modified by ps_status.
> !      *    Leaks memory, but called only once.
> !      */
> !     progname = strdup(nodir_name);
> !     if (progname == NULL)
>       {
> !         fprintf(stderr, "%s: out of memory\n", nodir_name);
> !         exit(1);            /* This could exit the postmaster */
> !     }
>
> ! #if defined(__CYGWIN__) || defined(WIN32)
> !     /* strip ".exe" suffix, regardless of case */
> !     if (strlen(progname) > sizeof(EXE) - 1 &&
> !         pg_strcasecmp(progname + strlen(progname) - (sizeof(EXE) - 1), EXE) == 0)
>           progname[strlen(progname) - (sizeof(EXE) - 1)] = '\0';
>   #endif
>
> !     return progname;
>   }
>
>

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
>                http://www.postgresql.org/docs/faq

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073