Odd problem with read_pg_options ...

Started by Marc G. Fournierabout 27 years ago3 messages
#1Marc G. Fournier
scrappy@hub.org

I reported earlier a SegFault when doing an initdb, and have narrowed it
down somewhat...still probing, but figured I'd see if I'm overlooking
something obvious...

From the command line, I'm running:

echo "vacuum" | postgres -o /dev/null -F -Q -D/home/centre/marc/pgsql/data

This is the stage that the first SegFault happens in initdb.

I added some debugging, and it turns out that the 'DataDir' variable isn't
being initialized at this point:

void
read_pg_options(SIGNAL_ARGS)
{
int fd;
int n;
int verbose;
char buffer[BUF_SIZE];
char c;
char *s,
*p;

printf("before sprintf()\n");
printf("%s\n", DataDir);
sprintf(buffer, "%s/%s", DataDir, "pg_options");
printf("after sprintf()\n");
if ((fd = open(buffer, O_RDONLY)) < 0)
return;

=====================

Still looking, but uncovered a slight bug:

diff -cr postgres.c.orig postgres.c
*** postgres.c.orig     Tue Oct 13 16:47:00 1998
--- postgres.c  Tue Oct 13 16:47:33 1998
***************
*** 1052,1057 ****
--- 1052,1058 ----

case 'D': /* PGDATA directory */
DataDir = optarg;
+ break;

case 'd': /* debug level */
flagQ = false;

Further into it...if I do a setenv PGDATA, it gets around the 'bug'...back
later...

Marc G. Fournier scrappy@hub.org
Systems Administrator @ hub.org
scrappy@{postgresql|isc}.org ICQ#7615664

#2Massimo Dal Zotto
dz@cs.unitn.it
In reply to: Marc G. Fournier (#1)
Re: [HACKERS] Odd problem with read_pg_options ...

I reported earlier a SegFault when doing an initdb, and have narrowed it
down somewhat...still probing, but figured I'd see if I'm overlooking
something obvious...

echo "vacuum" | postgres -o /dev/null -F -Q -D/home/centre/marc/pgsql/data

This is the stage that the first SegFault happens in initdb.

I added some debugging, and it turns out that the 'DataDir' variable isn't
being initialized at this point:

void
read_pg_options(SIGNAL_ARGS)
{
int fd;
int n;
int verbose;
char buffer[BUF_SIZE];
char c;
char *s,
*p;

printf("before sprintf()\n");
printf("%s\n", DataDir);
sprintf(buffer, "%s/%s", DataDir, "pg_options");
printf("after sprintf()\n");
if ((fd = open(buffer, O_RDONLY)) < 0)
return;

=====================

Still looking, but uncovered a slight bug:

diff -cr postgres.c.orig postgres.c
*** postgres.c.orig     Tue Oct 13 16:47:00 1998
--- postgres.c  Tue Oct 13 16:47:33 1998
***************
*** 1052,1057 ****
--- 1052,1058 ----

case 'D': /* PGDATA directory */
DataDir = optarg;
+ break;

case 'd': /* debug level */
flagQ = false;

Further into it...if I do a setenv PGDATA, it gets around the 'bug'...back
later...

Marc G. Fournier scrappy@hub.org
Systems Administrator @ hub.org
scrappy@{postgresql|isc}.org ICQ#7615664

The problem is that read_pg_options needs DataDir to read its file but
DataDir is set after read_pg_options if postgres is called interactively.
If postgres is forked by postgres DataDir is read from the PGDATA enviromnent
variable set by the postmaster and this explains while the bug disappears.
I have written this patch but I don't like it. Any better idea?

*** src/backend/utils/init/globals.c.orig	Thu Oct 15 00:13:03 1998
--- src/backend/utils/init/globals.c	Thu Oct 15 00:13:07 1998
***************
*** 46,52 ****
  struct Port *MyProcPort;
  long		MyCancelKey;

! char *DataDir;

   /*
    * The PGDATA directory user says to use, or defaults to via environment
--- 46,52 ----
  struct Port *MyProcPort;
  long		MyCancelKey;

! char *DataDir = NULL;

   /*
    * The PGDATA directory user says to use, or defaults to via environment
*** src/backend/utils/misc/trace.c.orig	Thu Sep  3 09:00:39 1998
--- src/backend/utils/misc/trace.c	Thu Oct 15 00:18:49 1998
***************
*** 343,348 ****
--- 343,353 ----
  	char	   *s,
  			   *p;
+ 	if (!DataDir) {
+ 	    fprintf(stderr, "read_pg_options: DataDir not defined\n");
+ 	    return;
+ 	}
+ 
  	sprintf(buffer, "%s/%s", DataDir, "pg_options");
  	if ((fd = open(buffer, O_RDONLY)) < 0)
  		return;
*** src/backend/tcop/postgres.c.orig	Tue Sep  1 09:01:27 1998
--- src/backend/tcop/postgres.c	Thu Oct 15 00:23:24 1998
***************
*** 1049,1055 ****
--- 1049,1061 ----
  				break;
  			case 'D':			/* PGDATA directory */
+ 			        if (!DataDir) {
+ 				    DataDir = optarg;
+ 				    /* must be done after DataDir is defined */
+ 				    read_pg_options(0);
+ 				}
  				DataDir = optarg;
+ 				break;

case 'd': /* debug level */
flagQ = false;

--
Massimo Dal Zotto

+----------------------------------------------------------------------+
|  Massimo Dal Zotto                email:  dz@cs.unitn.it             |
|  Via Marconi, 141                 phone:  ++39-461-534251            |
|  38057 Pergine Valsugana (TN)     www:  http://www.cs.unitn.it/~dz/  |
|  Italy                            pgp:  finger dz@tango.cs.unitn.it  |
+----------------------------------------------------------------------+
#3Bruce Momjian
maillist@candle.pha.pa.us
In reply to: Massimo Dal Zotto (#2)
Re: [HACKERS] Odd problem with read_pg_options ...

The problem is that read_pg_options needs DataDir to read its file but
DataDir is set after read_pg_options if postgres is called interactively.
If postgres is forked by postgres DataDir is read from the PGDATA enviromnent
variable set by the postmaster and this explains while the bug disappears.
I have written this patch but I don't like it. Any better idea?

I have applied it. You can improve it later, if you wish.

*** src/backend/utils/init/globals.c.orig	Thu Oct 15 00:13:03 1998
--- src/backend/utils/init/globals.c	Thu Oct 15 00:13:07 1998
***************
*** 46,52 ****
struct Port *MyProcPort;
long		MyCancelKey;

! char *DataDir;

/*
* The PGDATA directory user says to use, or defaults to via environment
--- 46,52 ----
struct Port *MyProcPort;
long		MyCancelKey;

! char *DataDir = NULL;

/*
* The PGDATA directory user says to use, or defaults to via environment
*** src/backend/utils/misc/trace.c.orig	Thu Sep  3 09:00:39 1998
--- src/backend/utils/misc/trace.c	Thu Oct 15 00:18:49 1998
***************
*** 343,348 ****
--- 343,353 ----
char	   *s,
*p;
+ 	if (!DataDir) {
+ 	    fprintf(stderr, "read_pg_options: DataDir not defined\n");
+ 	    return;
+ 	}
+ 
sprintf(buffer, "%s/%s", DataDir, "pg_options");
if ((fd = open(buffer, O_RDONLY)) < 0)
return;
*** src/backend/tcop/postgres.c.orig	Tue Sep  1 09:01:27 1998
--- src/backend/tcop/postgres.c	Thu Oct 15 00:23:24 1998
***************
*** 1049,1055 ****
--- 1049,1061 ----
break;
case 'D':			/* PGDATA directory */
+ 			        if (!DataDir) {
+ 				    DataDir = optarg;
+ 				    /* must be done after DataDir is defined */
+ 				    read_pg_options(0);
+ 				}
DataDir = optarg;
+ 				break;

case 'd': /* debug level */
flagQ = false;

--
Massimo Dal Zotto

+----------------------------------------------------------------------+
|  Massimo Dal Zotto                email:  dz@cs.unitn.it             |
|  Via Marconi, 141                 phone:  ++39-461-534251            |
|  38057 Pergine Valsugana (TN)     www:  http://www.cs.unitn.it/~dz/  |
|  Italy                            pgp:  finger dz@tango.cs.unitn.it  |
+----------------------------------------------------------------------+
-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026