Skip to content

Commit 48052de

Browse files
tglsfdctglsfdc
committed
Repair an error introduced by log_line_prefix patch: it is not acceptable
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.
1 parent 95af263 commit 48052de

File tree

6 files changed

+43
-26
lines changed

6 files changed

+43
-26
lines changed

‎src/backend/postmaster/postmaster.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
*
3838
*
3939
* IDENTIFICATION
40-
* $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.474 2005/11/03 20:02:50 alvherre Exp $
40+
* $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.475 2005/11/05 03:04:52 tgl Exp $
4141
*
4242
* NOTES
4343
*
@@ -2647,7 +2647,6 @@ BackendRun(Port *port)
26472647
/* set these to empty in case they are needed before we set them up */
26482648
port->remote_host = "";
26492649
port->remote_port = "";
2650-
port->commandTag = "";
26512650

26522651
/*
26532652
* Initialize libpq and enable reporting of ereport errors to the client.

‎src/backend/storage/lmgr/lock.c

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/storage/lmgr/lock.c,v 1.158 2005/10/15 02:49:26 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/storage/lmgr/lock.c,v 1.159 2005/11/05 03:04:52 tgl Exp $
1212
*
1313
* NOTES
1414
* Outside modules can create a lock table and acquire/release
@@ -1049,21 +1049,21 @@ WaitOnLock(LOCKMETHODID lockmethodid, LOCALLOCK *locallock,
10491049
ResourceOwner owner)
10501050
{
10511051
LockMethod lockMethodTable = LockMethods[lockmethodid];
1052-
char *new_status,
1053-
*old_status;
1054-
size_t len;
1052+
const char *old_status;
1053+
char *new_status;
1054+
int len;
10551055

10561056
Assert(lockmethodid < NumLockMethods);
10571057

10581058
LOCK_PRINT("WaitOnLock: sleeping on lock",
10591059
locallock->lock, locallock->tag.mode);
10601060

1061-
old_status = pstrdup(get_ps_display());
1062-
len = strlen(old_status);
1061+
old_status = get_ps_display(&len);
10631062
new_status = (char *) palloc(len + 8 + 1);
10641063
memcpy(new_status, old_status, len);
10651064
strcpy(new_status + len, " waiting");
10661065
set_ps_display(new_status);
1066+
new_status[len] = '\0'; /* truncate off " waiting" */
10671067

10681068
awaitedLock = locallock;
10691069
awaitedOwner = owner;
@@ -1104,8 +1104,7 @@ WaitOnLock(LOCKMETHODID lockmethodid, LOCALLOCK *locallock,
11041104

11051105
awaitedLock = NULL;
11061106

1107-
set_ps_display(old_status);
1108-
pfree(old_status);
1107+
set_ps_display(new_status);
11091108
pfree(new_status);
11101109

11111110
LOCK_PRINT("WaitOnLock: wakeup on lock",

‎src/backend/utils/error/elog.c

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
*
4343
*
4444
* IDENTIFICATION
45-
* $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.166 2005/11/03 17:11:39 alvherre Exp $
45+
* $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.167 2005/11/05 03:04:52 tgl Exp $
4646
*
4747
*-------------------------------------------------------------------------
4848
*/
@@ -67,6 +67,7 @@
6767
#include "tcop/tcopprot.h"
6868
#include "utils/memutils.h"
6969
#include "utils/guc.h"
70+
#include "utils/ps_status.h"
7071

7172

7273
/* Global variables */
@@ -1484,19 +1485,26 @@ log_line_prefix(StringInfo buf)
14841485
break;
14851486
case 'i':
14861487
if (MyProcPort)
1487-
appendStringInfo(buf, "%s", MyProcPort->commandTag);
1488+
{
1489+
const char *psdisp;
1490+
int displen;
1491+
1492+
psdisp = get_ps_display(&displen);
1493+
appendStringInfo(buf, "%.*s", displen, psdisp);
1494+
}
14881495
break;
14891496
case 'r':
1490-
if (MyProcPort)
1497+
if (MyProcPort && MyProcPort->remote_host)
14911498
{
14921499
appendStringInfo(buf, "%s", MyProcPort->remote_host);
1493-
if (strlen(MyProcPort->remote_port) > 0)
1500+
if (MyProcPort->remote_port &&
1501+
MyProcPort->remote_port[0] != '\0')
14941502
appendStringInfo(buf, "(%s)",
14951503
MyProcPort->remote_port);
14961504
}
14971505
break;
14981506
case 'h':
1499-
if (MyProcPort)
1507+
if (MyProcPort && MyProcPort->remote_host)
15001508
appendStringInfo(buf, "%s", MyProcPort->remote_host);
15011509
break;
15021510
case 'q':

‎src/backend/utils/misc/ps_status.c

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* to contain some useful information. Mechanism differs wildly across
66
* platforms.
77
*
8-
* $PostgreSQL: pgsql/src/backend/utils/misc/ps_status.c,v 1.25 2005/10/15 02:49:36 momjian Exp $
8+
* $PostgreSQL: pgsql/src/backend/utils/misc/ps_status.c,v 1.26 2005/11/05 03:04:52 tgl Exp $
99
*
1010
* Copyright (c) 2000-2005, PostgreSQL Global Development Group
1111
* various details abducted from various places
@@ -307,10 +307,6 @@ init_ps_display(const char *username, const char *dbname,
307307
void
308308
set_ps_display(const char *activity)
309309
{
310-
/* save tag for possible use by elog.c */
311-
if (MyProcPort)
312-
MyProcPort->commandTag = activity;
313-
314310
#ifndef PS_USE_NONE
315311
/* no ps display for stand-alone backend */
316312
if (!IsUnderPostmaster)
@@ -365,15 +361,31 @@ set_ps_display(const char *activity)
365361

366362
/*
367363
* Returns what's currently in the ps display, in case someone needs
368-
* it. Note that only the activity part is returned.
364+
* it. Note that only the activity part is returned. On some platforms
365+
* the string will not be null-terminated, so return the effective
366+
* length into *displen.
369367
*/
370368
const char *
371-
get_ps_display(void)
369+
get_ps_display(int *displen)
372370
{
373371
#ifdef PS_USE_CLOBBER_ARGV
372+
size_t offset;
373+
374374
/* If ps_buffer is a pointer, it might still be null */
375375
if (!ps_buffer)
376+
{
377+
*displen = 0;
376378
return "";
379+
}
380+
381+
/* Remove any trailing spaces to offset the effect of PS_PADDING */
382+
offset = ps_buffer_size;
383+
while (offset > ps_buffer_fixed_size && ps_buffer[offset-1] == PS_PADDING)
384+
offset--;
385+
386+
*displen = offset - ps_buffer_fixed_size;
387+
#else
388+
*displen = strlen(ps_buffer + ps_buffer_fixed_size);
377389
#endif
378390

379391
return ps_buffer + ps_buffer_fixed_size;

‎src/include/libpq/libpq-be.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
1212
* Portions Copyright (c) 1994, Regents of the University of California
1313
*
14-
* $PostgreSQL: pgsql/src/include/libpq/libpq-be.h,v 1.52 2005/10/15 02:49:44 momjian Exp $
14+
* $PostgreSQL: pgsql/src/include/libpq/libpq-be.h,v 1.53 2005/11/05 03:04:53 tgl Exp $
1515
*
1616
*-------------------------------------------------------------------------
1717
*/
@@ -80,7 +80,6 @@ typedef struct Port
8080
* but since it gets used by elog.c in the same way as database_name and
8181
* other members of this struct, we may as well keep it here.
8282
*/
83-
const char *commandTag; /* current command tag */
8483
struct timeval session_start; /* for session duration logging */
8584

8685
/*

‎src/include/utils/ps_status.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*
55
* Declarations for backend/utils/misc/ps_status.c
66
*
7-
* $PostgreSQL: pgsql/src/include/utils/ps_status.h,v 1.25 2004/02/22 21:26:54 tgl Exp $
7+
* $PostgreSQL: pgsql/src/include/utils/ps_status.h,v 1.26 2005/11/05 03:04:53 tgl Exp $
88
*
99
*-------------------------------------------------------------------------
1010
*/
@@ -19,6 +19,6 @@ extern void init_ps_display(const char *username, const char *dbname,
1919

2020
extern void set_ps_display(const char *activity);
2121

22-
extern const char *get_ps_display(void);
22+
extern const char *get_ps_display(int *displen);
2323

2424
#endif /* PS_STATUS_H */

0 commit comments

Comments
 (0)