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 -0000      1.42
--- src/bin/pg_ctl/pg_ctl.c     27 Oct 2004 17:10:58 -0000
***************
*** 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);
  
                                                /*
!                                                *      We could pass PGDATA just in 
an environment
!                                                *      variable but we do -D too for 
clearer
!                                                *      postmaster 'ps' display
                                                 */
!                                               pgdata_opt = xmalloc(strlen(pgdata_D) 
+ 7);
!                                               snprintf(pgdata_opt, strlen(pgdata_D) 
+ 7, "-D \"%s\" ",
!                                                                pgdata_D);
                                                break;
                                        }
                                case 'l':
Index: src/port/path.c
===================================================================
RCS file: /cvsroot/pgsql/src/port/path.c,v
retrieving revision 1.37
diff -c -c -r1.37 path.c
*** src/port/path.c     24 Oct 2004 22:08:19 -0000      1.37
--- src/port/path.c     27 Oct 2004 17:11:01 -0000
***************
*** 115,121 ****
  
  
  /*
!  * Make all paths look like Unix
   */
  void
  canonicalize_path(char *path)
--- 115,126 ----
  
  
  /*
!  *    Clean up path by:
!  *            o  make Win32 path use Unix slashes
!  *            o  remove trailling quote on Win32
!  *            o  remove trailling slash
!  *            o  remove trailing '.'
!  *            o  process trailing '..' ourselves
   */
  void
  canonicalize_path(char *path)
***************
*** 145,157 ****
  
        /*
         * Removing the trailing slash on a path means we never get ugly
!        * double slashes.      Also, Win32 can't stat() a directory with a
!        * trailing slash. Don't remove a leading slash, though.
         */
        trim_trailing_separator(path);
  
        /*
!        * Remove any trailing uses of "." or "..", too.
         */
        for (;;)
        {
--- 150,162 ----
  
        /*
         * Removing the trailing slash on a path means we never get ugly
!        * double trailing slashes.     Also, Win32 can't stat() a directory
!        * with a trailing slash. Don't remove a leading slash, though.
         */
        trim_trailing_separator(path);
  
        /*
!        * Remove any trailing uses of "." and process ".." ourselves
         */
        for (;;)
        {
***************
*** 165,171 ****
                else if (len >= 3 && strcmp(path + len - 3, "/..") == 0)
                {
                        trim_directory(path);
!                       trim_directory(path);
                        trim_trailing_separator(path);
                }
                else
--- 170,176 ----
                else if (len >= 3 && strcmp(path + len - 3, "/..") == 0)
                {
                        trim_directory(path);
!                       trim_directory(path);   /* remove directory above */
                        trim_trailing_separator(path);
                }
                else
---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
    (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])

Reply via email to