Re: [pgsql-hackers-win32] Data directory with trailing [back]slash - Mailing list pgsql-patches

Oops, path patch attached.

---------------------------------------------------------------------------

Bruce Momjian wrote:
>
> I think this still is not fixed.  I think we need to add a call to
> trim_trailing_separator() in checkDataDir().  Magnus, can you confirm
> this will fix it?
>
> ---------------------------------------------------------------------------
>
> Bruce Momjian wrote:
> >
> > I am now confused about the original report.  I don't see how my fix
> > would correct the reported problem.  trim_trailing_separator() would
> > have handled d:\pgdata\ and d:\pgdata just fine.  The fix only corrects
> > d:\.
> >
> > Magnus, does current CVS fix the problem?
> >
> > ---------------------------------------------------------------------------
> >
> > Andrew Dunstan wrote:
> > >
> > >
> > > Tom Lane wrote:
> > >
> > > >"Magnus Hagander" <mha@sollentuna.net> writes:
> > > >
> > > >
> > > >>It's not possible to start the postmaster on win32 with:
> > > >>postmaster -D d:\pgdata\
> > > >>or
> > > >>postmaster -D d:/pgdata/
> > > >>
> > > >>
> > > >
> > > >Sounds like canonicalize_path() needs to be applied a bit sooner than
> > > >it is.
> > > >
> > > >BTW I think canonicalize_path() is a few bricks shy of a load yet:
> > > >I'm not sure it works well with Windows drive-letters, and it definitely
> > > >will strip significant slashes when given input like '/' or 'C:\'.
> > > >Feel free to fix those problems while at it...
> > > >
> > > >
> > >
> > > Or use the attached patch, which I think does it right.
> > >
> > > cheers
> > >
> > > andrew
> >
> >
> > >
> > > ---------------------------(end of broadcast)---------------------------
> > > TIP 9: the planner will ignore your desire to choose an index scan if your
> > >       joining column's datatypes do not match
> >
> > --
> >   Bruce Momjian                        |  http://candle.pha.pa.us
> >   pgman@candle.pha.pa.us               |  (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 2: you can get off all lists at once with the unregister command
> >     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
> >
>
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (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 9: the planner will ignore your desire to choose an index scan if your
>       joining column's datatypes do not match
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/postmaster/postmaster.c,v
retrieving revision 1.407
diff -c -c -r1.407 postmaster.c
*** src/backend/postmaster/postmaster.c    11 Jul 2004 00:18:43 -0000    1.407
--- src/backend/postmaster/postmaster.c    11 Jul 2004 21:29:16 -0000
***************
*** 372,378 ****
      InitializeGUCOptions();

      userPGDATA = getenv("PGDATA");        /* default value */
!
      opterr = 1;

      while ((opt = getopt(argc, argv, "A:a:B:b:c:D:d:Fh:ik:lm:MN:no:p:Ss-:")) != -1)
--- 372,379 ----
      InitializeGUCOptions();

      userPGDATA = getenv("PGDATA");        /* default value */
!     canonicalize_path(userPGDATA);
!
      opterr = 1;

      while ((opt = getopt(argc, argv, "A:a:B:b:c:D:d:Fh:ik:lm:MN:no:p:Ss-:")) != -1)
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/utils/misc/guc.c,v
retrieving revision 1.214
diff -c -c -r1.214 guc.c
*** src/backend/utils/misc/guc.c    11 Jul 2004 00:18:44 -0000    1.214
--- src/backend/utils/misc/guc.c    11 Jul 2004 21:29:19 -0000
***************
*** 113,118 ****
--- 113,119 ----
  static bool assign_stage_log_stats(bool newval, bool doit, GucSource source);
  static bool assign_log_stats(bool newval, bool doit, GucSource source);
  static bool assign_transaction_read_only(bool newval, bool doit, GucSource source);
+ static const char *assign_canonical_path(const char *newval, bool doit, GucSource source);

  static void ReadConfigFile(char *filename, GucContext context);

***************
*** 1470,1476 ****
                           "the specified file.")
          },
          &Dynamic_library_path,
!         "$libdir", NULL, NULL
      },

      {
--- 1471,1477 ----
                           "the specified file.")
          },
          &Dynamic_library_path,
!         "$libdir", assign_canonical_path, NULL
      },

      {
***************
*** 1556,1562 ****
              GUC_LIST_INPUT | GUC_LIST_QUOTE
          },
          &preload_libraries_string,
!         "", NULL, NULL
      },

      {
--- 1557,1563 ----
              GUC_LIST_INPUT | GUC_LIST_QUOTE
          },
          &preload_libraries_string,
!         "", assign_canonical_path, NULL
      },

      {
***************
*** 1678,1684 ****
              NULL
          },
          &UnixSocketDir,
!         "", NULL, NULL
      },

      {
--- 1679,1685 ----
              NULL
          },
          &UnixSocketDir,
!         "", assign_canonical_path, NULL
      },

      {
***************
*** 1712,1736 ****
      {
          {"pgdata", PGC_POSTMASTER, 0, gettext_noop("Sets the location of the data directory"), NULL},
          &guc_pgdata,
!         NULL, NULL, NULL
      },

      {
          {"hba_conf", PGC_SIGHUP, 0, gettext_noop("Sets the location of the \"hba\" configuration file"), NULL},
          &guc_hbafile,
!         NULL, NULL, NULL
      },

      {
          {"ident_conf", PGC_SIGHUP, 0, gettext_noop("Sets the location of the \"ident\" configuration file"), NULL},
          &guc_identfile,
!         NULL, NULL, NULL
      },

      {
          {"external_pidfile", PGC_POSTMASTER, 0, gettext_noop("Writes the postmaster PID to the specified file"),
NULL},
          &external_pidfile,
!         NULL, NULL, NULL
      },

      /* End-of-list marker */
--- 1713,1737 ----
      {
          {"pgdata", PGC_POSTMASTER, 0, gettext_noop("Sets the location of the data directory"), NULL},
          &guc_pgdata,
!         NULL, assign_canonical_path, NULL
      },

      {
          {"hba_conf", PGC_SIGHUP, 0, gettext_noop("Sets the location of the \"hba\" configuration file"), NULL},
          &guc_hbafile,
!         NULL, assign_canonical_path, NULL
      },

      {
          {"ident_conf", PGC_SIGHUP, 0, gettext_noop("Sets the location of the \"ident\" configuration file"), NULL},
          &guc_identfile,
!         NULL, assign_canonical_path, NULL
      },

      {
          {"external_pidfile", PGC_POSTMASTER, 0, gettext_noop("Writes the postmaster PID to the specified file"),
NULL},
          &external_pidfile,
!         NULL, assign_canonical_path, NULL
      },

      /* End-of-list marker */
***************
*** 5160,5167 ****
  }

  static const char *
! assign_client_min_messages(const char *newval,
!                            bool doit, GucSource source)
  {
      return (assign_msglvl(&client_min_messages, newval, doit, source));
  }
--- 5161,5167 ----
  }

  static const char *
! assign_client_min_messages(const char *newval, bool doit, GucSource source)
  {
      return (assign_msglvl(&client_min_messages, newval, doit, source));
  }
***************
*** 5428,5433 ****
--- 5428,5448 ----
                  (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                   errmsg("cannot set transaction read only mode inside a subtransaction")));
      return true;
+ }
+
+ static const char *
+ assign_canonical_path(const char *newval, bool doit, GucSource source)
+ {
+
+     if (doit)
+     {
+         /* We have to create a new pointer to force the change */
+         char *canon_val = guc_strdup(FATAL, newval);
+         canonicalize_path(canon_val);
+         return canon_val;
+     }
+     else
+         return newval;
  }

  #include "guc-file.c"
Index: src/bin/psql/command.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/bin/psql/command.c,v
retrieving revision 1.119
diff -c -c -r1.119 command.c
*** src/bin/psql/command.c    11 Jul 2004 13:29:15 -0000    1.119
--- src/bin/psql/command.c    11 Jul 2004 21:29:22 -0000
***************
*** 375,380 ****
--- 375,381 ----
              fname = psql_scan_slash_option(scan_state,
                                             OT_NORMAL, NULL, true);
              expand_tilde(&fname);
+             canonicalize_path(fname);
              status = do_edit(fname, query_buf) ? CMD_NEWEDIT : CMD_ERROR;
              free(fname);
          }
***************
*** 777,784 ****
                      fd = popen(&fname[1], "w");
                  }
                  else
                      fd = fopen(fname, "w");
!
                  if (!fd)
                  {
                      psql_error("%s: %s\n", fname, strerror(errno));
--- 778,787 ----
                      fd = popen(&fname[1], "w");
                  }
                  else
+                 {
+                     canonicalize_path(fname);
                      fd = fopen(fname, "w");
!                 }
                  if (!fd)
                  {
                      psql_error("%s: %s\n", fname, strerror(errno));
***************
*** 1122,1128 ****

      if (filename_arg)
          fname = filename_arg;
-
      else
      {
          /* make a temp file to edit */
--- 1125,1130 ----
***************
*** 1262,1267 ****
--- 1264,1270 ----
      if (!filename)
          return false;

+     canonicalize_path(filename);
      fd = fopen(filename, PG_BINARY_R);

      if (!fd)
Index: src/bin/psql/copy.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/bin/psql/copy.c,v
retrieving revision 1.49
diff -c -c -r1.49 copy.c
*** src/bin/psql/copy.c    11 Jul 2004 13:29:15 -0000    1.49
--- src/bin/psql/copy.c    11 Jul 2004 21:29:22 -0000
***************
*** 513,518 ****
--- 513,520 ----
          appendPQExpBuffer(&query, " FORCE NOT NULL %s", options->force_notnull_list);
      }

+     canonicalize_path(options->file);
+
      if (options->from)
      {
          if (options->file)
Index: src/port/path.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/port/path.c,v
retrieving revision 1.22
diff -c -c -r1.22 path.c
*** src/port/path.c    11 Jul 2004 02:59:42 -0000    1.22
--- src/port/path.c    11 Jul 2004 21:29:23 -0000
***************
*** 33,38 ****
--- 33,39 ----
  #endif

  const static char *relative_path(const char *bin_path, const char *other_path);
+ static void make_relative(const char *my_exec_path, const char *p, char *ret_path);
  static void trim_directory(char *path);
  static void trim_trailing_separator(char *path);

***************
*** 43,57 ****
          (p)++; \
  }

- /* Macro creates a relative path */
- #define MAKE_RELATIVE \
- do { \
-         StrNCpy(path, my_exec_path, MAXPGPATH); \
-         trim_directory(path); \
-         trim_directory(path); \
-         snprintf(ret_path, MAXPGPATH, "%s/%s", path, p); \
- } while (0)
-
  /*
   *    first_dir_separator
   */
--- 44,49 ----
***************
*** 140,152 ****
  void
  get_share_path(const char *my_exec_path, char *ret_path)
  {
-     char path[MAXPGPATH];
      const char *p;

      if ((p = relative_path(PGBINDIR, PGSHAREDIR)))
!         MAKE_RELATIVE;
      else
          StrNCpy(ret_path, PGSHAREDIR, MAXPGPATH);
  }


--- 132,144 ----
  void
  get_share_path(const char *my_exec_path, char *ret_path)
  {
      const char *p;

      if ((p = relative_path(PGBINDIR, PGSHAREDIR)))
!         make_relative(my_exec_path, p, ret_path);
      else
          StrNCpy(ret_path, PGSHAREDIR, MAXPGPATH);
+     canonicalize_path(ret_path);
  }


***************
*** 157,169 ****
  void
  get_etc_path(const char *my_exec_path, char *ret_path)
  {
-     char path[MAXPGPATH];
      const char *p;

      if ((p = relative_path(PGBINDIR, SYSCONFDIR)))
!         MAKE_RELATIVE;
      else
          StrNCpy(ret_path, SYSCONFDIR, MAXPGPATH);
  }


--- 149,161 ----
  void
  get_etc_path(const char *my_exec_path, char *ret_path)
  {
      const char *p;

      if ((p = relative_path(PGBINDIR, SYSCONFDIR)))
!         make_relative(my_exec_path, p, ret_path);
      else
          StrNCpy(ret_path, SYSCONFDIR, MAXPGPATH);
+     canonicalize_path(ret_path);
  }


***************
*** 174,186 ****
  void
  get_include_path(const char *my_exec_path, char *ret_path)
  {
-     char path[MAXPGPATH];
      const char *p;

      if ((p = relative_path(PGBINDIR, INCLUDEDIR)))
!         MAKE_RELATIVE;
      else
          StrNCpy(ret_path, INCLUDEDIR, MAXPGPATH);
  }


--- 166,178 ----
  void
  get_include_path(const char *my_exec_path, char *ret_path)
  {
      const char *p;

      if ((p = relative_path(PGBINDIR, INCLUDEDIR)))
!         make_relative(my_exec_path, p, ret_path);
      else
          StrNCpy(ret_path, INCLUDEDIR, MAXPGPATH);
+     canonicalize_path(ret_path);
  }


***************
*** 191,203 ****
  void
  get_pkginclude_path(const char *my_exec_path, char *ret_path)
  {
-     char path[MAXPGPATH];
      const char *p;

      if ((p = relative_path(PGBINDIR, PKGINCLUDEDIR)))
!         MAKE_RELATIVE;
      else
          StrNCpy(ret_path, PKGINCLUDEDIR, MAXPGPATH);
  }


--- 183,195 ----
  void
  get_pkginclude_path(const char *my_exec_path, char *ret_path)
  {
      const char *p;

      if ((p = relative_path(PGBINDIR, PKGINCLUDEDIR)))
!         make_relative(my_exec_path, p, ret_path);
      else
          StrNCpy(ret_path, PKGINCLUDEDIR, MAXPGPATH);
+     canonicalize_path(ret_path);
  }


***************
*** 210,222 ****
  void
  get_pkglib_path(const char *my_exec_path, char *ret_path)
  {
-     char path[MAXPGPATH];
      const char *p;

      if ((p = relative_path(PGBINDIR, PKGLIBDIR)))
!         MAKE_RELATIVE;
      else
          StrNCpy(ret_path, PKGLIBDIR, MAXPGPATH);
  }


--- 202,214 ----
  void
  get_pkglib_path(const char *my_exec_path, char *ret_path)
  {
      const char *p;

      if ((p = relative_path(PGBINDIR, PKGLIBDIR)))
!         make_relative(my_exec_path, p, ret_path);
      else
          StrNCpy(ret_path, PKGLIBDIR, MAXPGPATH);
+     canonicalize_path(ret_path);
  }


***************
*** 229,241 ****
  void
  get_locale_path(const char *my_exec_path, char *ret_path)
  {
-     char path[MAXPGPATH];
      const char *p;

      if ((p = relative_path(PGBINDIR, LOCALEDIR)))
!         MAKE_RELATIVE;
      else
          StrNCpy(ret_path, LOCALEDIR, MAXPGPATH);
  }


--- 221,233 ----
  void
  get_locale_path(const char *my_exec_path, char *ret_path)
  {
      const char *p;

      if ((p = relative_path(PGBINDIR, LOCALEDIR)))
!         make_relative(my_exec_path, p, ret_path);
      else
          StrNCpy(ret_path, LOCALEDIR, MAXPGPATH);
+     canonicalize_path(ret_path);
  }


***************
*** 270,275 ****
--- 262,268 ----
      {
          /* set for libpq to use */
          snprintf(env_path, sizeof(env_path), "PGLOCALEDIR=%s", path);
+         canonicalize_path(env_path);
          putenv(strdup(env_path));
      }
  #endif
***************
*** 280,290 ****
--- 273,298 ----

          /* set for libpq to use */
          snprintf(env_path, sizeof(env_path), "PGSYSCONFDIR=%s", path);
+         canonicalize_path(env_path);
          putenv(strdup(env_path));
      }
  }


+ /*
+  *    make_relative - adjust path to be relative to bin/
+  */
+ static void
+ make_relative(const char *my_exec_path, const char *p, char *ret_path)
+ {
+     char path[MAXPGPATH];
+
+     StrNCpy(path, my_exec_path, MAXPGPATH);
+     trim_directory(path);
+     trim_directory(path);
+     snprintf(ret_path, MAXPGPATH, "%s/%s", path, p);
+ }
+

  /*
   *    relative_path
***************
*** 391,397 ****
      char *p = path + strlen(path);

  #ifdef WIN32
!     /* Skip over network and drive specifiers for win32 */
      if (strlen(path) >= 2)
      {
          if (IS_DIR_SEP(path[0]) && IS_DIR_SEP(path[1]))
--- 399,408 ----
      char *p = path + strlen(path);

  #ifdef WIN32
!     /*
!      *    Skip over network and drive specifiers for win32.
!      *    Set 'path' to point to the last character to keep.
!      */
      if (strlen(path) >= 2)
      {
          if (IS_DIR_SEP(path[0]) && IS_DIR_SEP(path[1]))