From: | Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-committers(at)postgresql(dot)org |
Subject: | Re: pgsql-server: Use canonicalize_path for -D, GUC paths, |
Date: | 2004-07-11 23:49:19 |
Message-ID: | 200407112349.i6BNnJB06904@candle.pha.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers |
Tom Lane wrote:
> momjian(at)svr1(dot)postgresql(dot)org (Bruce Momjian) writes:
> > pgsql-server/src/backend/postmaster:
> > postmaster.c (r1.407 -> r1.408)
> > (http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/backend/postmaster/postmaster.c.diff?r1=1.407&r2=1.408)
>
> You can't do that. In the first place it will dump core if PGDATA isn't
> set, and in the second place it is not kosher to scribble on environment
> values.
OK, I added a strdup to make a copy of the environment variable. I also
moved the canonicialize_path down so it will process both PGDATA and -D.
Patch attached and applied.
> This is the wrong place to do it anyway. It is necessary, sufficient,
> and already done to do it in SetDataDir.
The problem is that checkDataDir() (called before SetDataDir()) does a
stat() on that value, and this is the failure Magnus was seeing. This
requirement is new because onlyConfigSpecified() does a stat() to test
to see if we are are pointing to a config file/dir or the data directory
before calling SetDataDir(). However, we can't remove the
canonicalize_path() from SetDataDir() because postgres and bootstrap
call that directly.
> > pgsql-server/src/backend/utils/misc:
> > guc.c (r1.214 -> r1.215)
> > (http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/backend/utils/misc/guc.c.diff?r1=1.214&r2=1.215)
>
> Could we not have FATAL here, please?
OK, changed to ERROR, but there are alot of similar calls which used
FATAL for that call in that file.
> > pgsql-server/src/bin/psql:
> > command.c (r1.119 -> r1.120)
> > (http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/bin/psql/command.c.diff?r1=1.119&r2=1.120)
>
> None of these are correct. canonicalize_path is only intended for
> directory names not file names. (I think the same problem applies
> to several of your GUC variable changes, too.)
canonicalize_path changes \ to /, and trims the trailing slash.
Filenames do not need trimming, but they might need the slash change. I
think we found that a filename with mixed / and \ will not work.
> > copy.c (r1.49 -> r1.50)
> > (http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/bin/psql/copy.c.diff?r1=1.49&r2=1.50)
>
> As above.
>
> regards, tom lane
>
--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Attachment | Content-Type | Size |
---|---|---|
unknown_filename | text/plain | 2.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2004-07-11 23:49:53 | pgsql-server: Cleanup for canonicalization fixes, from Tom. |
Previous Message | Bruce Momjian | 2004-07-11 23:31:27 | pgsql-server: Add: > * Allow moving sequences and toast tables to other |