Repair an error introduced by log_line_prefix patch: it is not acceptable
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 5 Nov 2005 03:05:05 +0000 (03:05 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 5 Nov 2005 03:05:05 +0000 (03:05 +0000)
to assume that the string pointer passed to set_ps_display is good forever.
There's no need to anyway since ps_status.c itself saves the string, and
we already had an API (get_ps_display) to return it.
I believe this explains Jim Nasby's report of intermittent crashes in
elog.c when %i format code is in use in log_line_prefix.
While at it, repair a previously unnoticed problem: on some platforms such as
Darwin, the string returned by get_ps_display was blank-padded to the maximum
length, meaning that lock.c's attempt to append " waiting" to it never worked.

src/backend/postmaster/postmaster.c
src/backend/storage/lmgr/lock.c
src/backend/utils/error/elog.c
src/backend/utils/misc/ps_status.c
src/include/libpq/libpq-be.h
src/include/utils/ps_status.h

index 0a67a25916f648cdc455f5654405f14c48a60ef0..8b316197fc13511ed043744b8167c08f9316472f 100644 (file)
@@ -37,7 +37,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.443.4.3 2005/10/20 20:06:02 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.443.4.4 2005/11/05 03:05:04 tgl Exp $
  *
  * NOTES
  *
@@ -2638,7 +2638,6 @@ BackendRun(Port *port)
    /* set these to empty in case they are needed before we set them up */
    port->remote_host = "";
    port->remote_port = "";
-   port->commandTag = "";
 
    /*
     * Initialize libpq and enable reporting of ereport errors to the
index 4551443964d169950f243ac5a588bb4d1d41d3a6..0b611b75a51443ae0194303fcd26f8f015c02026 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/storage/lmgr/lock.c,v 1.145.4.1 2005/03/01 21:15:10 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/storage/lmgr/lock.c,v 1.145.4.2 2005/11/05 03:05:04 tgl Exp $
  *
  * NOTES
  *   Outside modules can create a lock table and acquire/release
@@ -1074,19 +1074,21 @@ WaitOnLock(LOCKMETHODID lockmethodid, LOCALLOCK *locallock,
           ResourceOwner owner)
 {
    LockMethod  lockMethodTable = LockMethods[lockmethodid];
-   char       *new_status,
-              *old_status;
+   const char *old_status;
+   char       *new_status;
+   int         len;
 
    Assert(lockmethodid < NumLockMethods);
 
    LOCK_PRINT("WaitOnLock: sleeping on lock",
               locallock->lock, locallock->tag.mode);
 
-   old_status = pstrdup(get_ps_display());
-   new_status = (char *) palloc(strlen(old_status) + 10);
-   strcpy(new_status, old_status);
-   strcat(new_status, " waiting");
+   old_status = get_ps_display(&len);
+   new_status = (char *) palloc(len + 8 + 1);
+   memcpy(new_status, old_status, len);
+   strcpy(new_status + len, " waiting");
    set_ps_display(new_status);
+   new_status[len] = '\0';     /* truncate off " waiting" */
 
    awaitedLock = locallock;
    awaitedOwner = owner;
@@ -1129,8 +1131,7 @@ WaitOnLock(LOCKMETHODID lockmethodid, LOCALLOCK *locallock,
 
    awaitedLock = NULL;
 
-   set_ps_display(old_status);
-   pfree(old_status);
+   set_ps_display(new_status);
    pfree(new_status);
 
    LOCK_PRINT("WaitOnLock: wakeup on lock",
index c919f7c2c651c0f814b74e579032d801f35f75a3..358bd982b34afd72aaf09a9fb81f91779e8cfa1e 100644 (file)
@@ -42,7 +42,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.155.4.3 2005/10/14 16:41:13 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.155.4.4 2005/11/05 03:05:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -67,6 +67,7 @@
 #include "tcop/tcopprot.h"
 #include "utils/memutils.h"
 #include "utils/guc.h"
+#include "utils/ps_status.h"
 
 
 /* Global variables */
@@ -1461,13 +1462,20 @@ log_line_prefix(StringInfo buf)
                break;
            case 'i':
                if (MyProcPort)
-                   appendStringInfo(buf, "%s", MyProcPort->commandTag);
+               {
+                   const char *psdisp;
+                   int     displen;
+
+                   psdisp = get_ps_display(&displen);
+                   appendStringInfo(buf, "%.*s", displen, psdisp);
+               }
                break;
            case 'r':
-               if (MyProcPort)
+               if (MyProcPort && MyProcPort->remote_host)
                {
                    appendStringInfo(buf, "%s", MyProcPort->remote_host);
-                   if (strlen(MyProcPort->remote_port) > 0)
+                   if (MyProcPort->remote_port &&
+                       MyProcPort->remote_port[0] != '\0')
                        appendStringInfo(buf, "(%s)",
                                         MyProcPort->remote_port);
                }
index 4cecf446ae2d6c24aa487cdd9183b5cfbb172c4f..8d34d23581576ac7acf6040ae3666b7ac57aec65 100644 (file)
@@ -5,7 +5,7 @@
  * to contain some useful information. Mechanism differs wildly across
  * platforms.
  *
- * $PostgreSQL: pgsql/src/backend/utils/misc/ps_status.c,v 1.23 2005/01/01 05:43:08 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/misc/ps_status.c,v 1.23.4.1 2005/11/05 03:05:05 tgl Exp $
  *
  * Copyright (c) 2000-2005, PostgreSQL Global Development Group
  * various details abducted from various places
@@ -308,10 +308,6 @@ init_ps_display(const char *username, const char *dbname,
 void
 set_ps_display(const char *activity)
 {
-   /* save tag for possible use by elog.c */
-   if (MyProcPort)
-       MyProcPort->commandTag = activity;
-
 #ifndef PS_USE_NONE
    /* no ps display for stand-alone backend */
    if (!IsUnderPostmaster)
@@ -367,15 +363,31 @@ set_ps_display(const char *activity)
 
 /*
  * Returns what's currently in the ps display, in case someone needs
- * it. Note that only the activity part is returned.
+ * it. Note that only the activity part is returned.  On some platforms
+ * the string will not be null-terminated, so return the effective
+ * length into *displen.
  */
 const char *
-get_ps_display(void)
+get_ps_display(int *displen)
 {
 #ifdef PS_USE_CLOBBER_ARGV
+   size_t      offset;
+
    /* If ps_buffer is a pointer, it might still be null */
    if (!ps_buffer)
+   {
+       *displen = 0;
        return "";
+   }
+
+   /* Remove any trailing spaces to offset the effect of PS_PADDING */
+   offset = ps_buffer_size;
+   while (offset > ps_buffer_fixed_size && ps_buffer[offset-1] == PS_PADDING)
+       offset--;
+
+   *displen = offset - ps_buffer_fixed_size;
+#else
+   *displen = strlen(ps_buffer + ps_buffer_fixed_size);
 #endif
 
    return ps_buffer + ps_buffer_fixed_size;
index cec679577bd880d56f126572e2bdae6a396764ac..d30a0440a67e11ff968505bc11d81e34438489cb 100644 (file)
@@ -11,7 +11,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/libpq/libpq-be.h,v 1.49 2004/12/31 22:03:32 pgsql Exp $
+ * $PostgreSQL: pgsql/src/include/libpq/libpq-be.h,v 1.49.4.1 2005/11/05 03:05:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -79,7 +79,6 @@ typedef struct Port
     * database_name and other members of this struct, we may as well keep
     * it here.
     */
-   const char *commandTag;     /* current command tag */
    struct timeval session_start;       /* for session duration logging */
 
    /*
index 6855d34a34ff62412be73af2cdc32996204f8b5e..ddf7f001a7fbc48fa1622da66ad362cc3ab6ea0e 100644 (file)
@@ -4,7 +4,7 @@
  *
  * Declarations for backend/utils/misc/ps_status.c
  *
- * $PostgreSQL: pgsql/src/include/utils/ps_status.h,v 1.25 2004/02/22 21:26:54 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/utils/ps_status.h,v 1.25.4.1 2005/11/05 03:05:05 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -19,6 +19,6 @@ extern void init_ps_display(const char *username, const char *dbname,
 
 extern void set_ps_display(const char *activity);
 
-extern const char *get_ps_display(void);
+extern const char *get_ps_display(int *displen);
 
 #endif   /* PS_STATUS_H */