Re: pgsql-server: Use canonicalize_path for -D, GUC paths,

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