Re: [PATCHES] pg_ctl -D canonicalization
Thanks. I can confirm that this fixes the issue. //Magnus -Original Message- From: Bruce Momjian [mailto:[EMAIL PROTECTED] Sent: den 27 oktober 2004 19:17 To: Magnus Hagander Cc: [EMAIL PROTECTED] Subject: Re: [PATCHES] pg_ctl -D canonicalization OK, attached patch applied. Your original patch didn't canonicalize the PGDATA environment variable. '-D' takes precidence but it should be accurate. I also updated the comments in canonicalize_path because it does a lot more now that just win to unix path conversion. --- ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] pg_ctl -D canonicalization
It seems pg_ctl calls canonicalize_path() only on the path as being used to access for example the pid file, and not the path that is sent along to the postmaster. Specifically, this causes failure on win32 when a path is passed with a trailing backslash, when it's inside quotes such as: pg_ctl start -D c:\program fiels\postgresql\data\ The quotes are of course necessary since there are spaces in the filename. In my specific case the trailing backslash is automatically added by windows installer. but other cases when the backslash is added can easily be seen... Attached patch makes pg_ctl call canonicalize_path() on the path as passed on the commandline as well. I have only tested this on win32, where it appears to work fine. I think it would be good on other platsforms as well which is why I didn't #ifdef it. If not, then please add #ifdefs. Uh, isn't the proper fix to fix the postmaster's handling of -D paths? Nope. The problem is that in this case, pg_ctl gets the path: c:\program files\postgresql\data\ (not unbalanced quotes). Yes, win32 commandline quoting is horrible. Anyway. then it properly quotes this for the commandline: c:\program files\postgresql\data\ and you can see where that breaks down... Another option would be simple check to simply strip the (and the trailing backslash too, probably, to make things safer at the next step). I just thought it'd be cleaner to use canonicalize_path(). But it's just the trailing quote stuff that causes the problem. (See the second step in canonicalize_path() where this has been noticed befoer, followed by the trim-trailing part which would also be nice). (Everything works if you register it as a service because then it uses pg_data and not pgdata_opt. This is just when you start things manually, which is why we missed it before...) //Magnus ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] pg_ctl -D canonicalization
OK, attached patch applied. Your original patch didn't canonicalize the
PGDATA environment variable. '-D' takes precidence but it should be
accurate.
I also updated the comments in canonicalize_path because it does a lot
more now that just win to unix path conversion.
---
Magnus Hagander wrote:
It seems pg_ctl calls canonicalize_path() only on the path as being
used to access for example the pid file, and not the path
that is sent
along to the postmaster.
Specifically, this causes failure on win32 when a path is
passed with
a trailing backslash, when it's inside quotes such as:
pg_ctl start -D c:\program fiels\postgresql\data\
The quotes are of course necessary since there are spaces in the
filename. In my specific case the trailing backslash is
automatically
added by windows installer. but other cases when the backslash is
added can easily be seen...
Attached patch makes pg_ctl call canonicalize_path() on the path as
passed on the commandline as well.
I have only tested this on win32, where it appears to work fine. I
think it would be good on other platsforms as well which is why I
didn't #ifdef it. If not, then please add #ifdefs.
Uh, isn't the proper fix to fix the postmaster's handling of -D paths?
Nope.
The problem is that in this case, pg_ctl gets the path:
c:\program files\postgresql\data\
(not unbalanced quotes). Yes, win32 commandline quoting is horrible.
Anyway. then it properly quotes this for the commandline:
c:\program files\postgresql\data\
and you can see where that breaks down...
Another option would be simple check to simply strip the (and the
trailing backslash too, probably, to make things safer at the next
step). I just thought it'd be cleaner to use canonicalize_path(). But
it's just the trailing quote stuff that causes the problem. (See the
second step in canonicalize_path() where this has been noticed befoer,
followed by the trim-trailing part which would also be nice).
(Everything works if you register it as a service because then it uses
pg_data and not pgdata_opt. This is just when you start things manually,
which is why we missed it before...)
//Magnus
---(end of broadcast)---
TIP 6: Have you searched our list archives?
http://archives.postgresql.org
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: src/bin/pg_ctl/pg_ctl.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v
retrieving revision 1.42
diff -c -c -r1.42 pg_ctl.c
*** src/bin/pg_ctl/pg_ctl.c 22 Oct 2004 00:24:18 - 1.42
--- src/bin/pg_ctl/pg_ctl.c 27 Oct 2004 17:10:58 -
***
*** 1279,1297
{
case 'D':
{
! int len =
strlen(optarg);
! char *env_var;
! env_var = xmalloc(len + 8);
! snprintf(env_var, len + 8,
PGDATA=%s, optarg);
putenv(env_var);
/*
!* Show -D for easier postmaster 'ps'
!* identification
*/
! pgdata_opt = xmalloc(len + 7);
! snprintf(pgdata_opt, len + 7, -D
\%s\ , optarg);
break;
}
case 'l':
--- 1279,1301
{
case 'D':
{
! char *pgdata_D =
xmalloc(strlen(optarg));
! char *env_var =
xmalloc(strlen(optarg) + 8);
! strcpy(pgdata_D, optarg);
! canonicalize_path(pgdata_D);
! snprintf(env_var, strlen(pgdata_D) +
8, PGDATA=%s,
!pgdata_D);
putenv(env_var);
/*
!
Re: [PATCHES] pg_ctl -D canonicalization
Magnus Hagander wrote: It seems pg_ctl calls canonicalize_path() only on the path as being used to access for example the pid file, and not the path that is sent along to the postmaster. Specifically, this causes failure on win32 when a path is passed with a trailing backslash, when it's inside quotes such as: pg_ctl start -D c:\program fiels\postgresql\data\ The quotes are of course necessary since there are spaces in the filename. In my specific case the trailing backslash is automatically added by windows installer. but other cases when the backslash is added can easily be seen... Attached patch makes pg_ctl call canonicalize_path() on the path as passed on the commandline as well. I have only tested this on win32, where it appears to work fine. I think it would be good on other platsforms as well which is why I didn't #ifdef it. If not, then please add #ifdefs. Uh, isn't the proper fix to fix the postmaster's handling of -D paths? -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
[PATCHES] pg_ctl -D canonicalization
It seems pg_ctl calls canonicalize_path() only on the path as being used to access for example the pid file, and not the path that is sent along to the postmaster. Specifically, this causes failure on win32 when a path is passed with a trailing backslash, when it's inside quotes such as: pg_ctl start -D c:\program fiels\postgresql\data\ The quotes are of course necessary since there are spaces in the filename. In my specific case the trailing backslash is automatically added by windows installer. but other cases when the backslash is added can easily be seen... Attached patch makes pg_ctl call canonicalize_path() on the path as passed on the commandline as well. I have only tested this on win32, where it appears to work fine. I think it would be good on other platsforms as well which is why I didn't #ifdef it. If not, then please add #ifdefs. //Magnus pgctl_canon.patch Description: pgctl_canon.patch ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
