Re: Path separator - Mailing list pgsql-hackers

Magnus Hagander wrote:
> Magnus Hagander wrote:
>>> The major stumbling block to doing either thing is not wishing to import
>>> path.c into libpq.  I think that the objection was partially code size
>>> and partially namespace pollution (path.c doesn't use pg_ or similar
>>> prefixes on its exported names).  The latter is no longer a problem on
>>> platforms that support exported-name filtering, but that isn't all of
>>> them.  Could we consider splitting path.c into two parts, that which we
>>> want in libpq and that which we don't?
>> On my system (linux), path.o is 5k. libpq.so is 157k. Is adding that
>> size *really* a problem?
>>
>> Also, is there a chance that the linker is smart enough to actually
>> remove the parts of path.o that aren't used in libpq completely, if it's
>> not exported at all? (if the size does matter)
>>
>> If it is, sure, we could split it apart. But fairly large parts of it
>> would be required by both. But I guess the number of symbols would be
>> quite a bit smaller.
>
> Answering myself here: the filesize for the "frontend only" part is
> about 2k on this system.

Long meeting, time for coding.. :-) Here's a rough patch. Is this about
what you had in mind?

//Magnus
diff --git a/src/include/port.h b/src/include/port.h
index 0557dd2..c84f0d6 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -28,8 +28,15 @@ extern char *last_dir_separator(const char *filename);
 extern char *first_path_separator(const char *pathlist);
 extern void join_path_components(char *ret_path,
                      const char *head, const char *tail);
+extern void trim_directory(char *path);
+extern void trim_trailing_separator(char *path);
 extern void canonicalize_path(char *path);
 extern void make_native_path(char *path);
+#ifdef WIN32
+extern char *skip_drive(const char *path);
+#else
+#define skip_drive(path)    (path)
+#endif
 extern bool path_contains_parent_reference(const char *path);
 extern bool path_is_prefix_of_path(const char *path1, const char *path2);
 extern const char *get_progname(const char *argv0);
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 3b9df76..02a240d 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -34,6 +34,7 @@ OBJS=    fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
     fe-protocol2.o fe-protocol3.o pqexpbuffer.o pqsignal.o fe-secure.o \
     libpq-events.o \
     md5.o ip.o wchar.o encnames.o noblock.o pgstrcasecmp.o thread.o \
+    path_fe.o \
     $(filter crypt.o getaddrinfo.o inet_aton.o open.o snprintf.o strerror.o strlcpy.o win32error.o, $(LIBOBJS))

 ifeq ($(PORTNAME), cygwin)
@@ -80,7 +81,7 @@ backend_src = $(top_srcdir)/src/backend
 # For port modules, this only happens if configure decides the module
 # is needed (see filter hack in OBJS, above).

-crypt.c getaddrinfo.c inet_aton.c noblock.c open.c pgstrcasecmp.c snprintf.c strerror.c strlcpy.c thread.c
win32error.cpgsleep.c: % : $(top_srcdir)/src/port/% 
+crypt.c getaddrinfo.c inet_aton.c noblock.c open.c path_fe.c pgstrcasecmp.c snprintf.c strerror.c strlcpy.c thread.c
win32error.cpgsleep.c: % : $(top_srcdir)/src/port/% 
     rm -f $@ && $(LN_S) $< .

 md5.c ip.c: % : $(backend_src)/libpq/%
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 8383f2a..a68baee 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -600,6 +600,7 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
         strncpy(fnbuf, conn->sslcert, sizeof(fnbuf));
     else
         snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_CERT_FILE);
+    canonicalize_path(fnbuf);

     /*
      * OpenSSL <= 0.9.8 lacks error stack handling, which means it's likely to
@@ -716,6 +717,7 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
         /* No PGSSLKEY specified, load default file */
         snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_KEY_FILE);
     }
+    canonicalize_path(fnbuf);

     if (fnbuf[0] != '\0')
     {
@@ -1016,6 +1018,7 @@ initialize_SSL(PGconn *conn)
         strncpy(fnbuf, conn->sslrootcert, sizeof(fnbuf));
     else
         snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CERT_FILE);
+    canonicalize_path(fnbuf);

     if (stat(fnbuf, &buf) == 0)
     {
@@ -1038,6 +1041,7 @@ initialize_SSL(PGconn *conn)
                 strncpy(fnbuf, conn->sslcrl, sizeof(fnbuf));
             else
                 snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CRL_FILE);
+            canonicalize_path(fnbuf);

             /* setting the flags to check against the complete CRL chain */
             if (X509_STORE_load_locations(cvstore, fnbuf, NULL) == 1)
diff --git a/src/port/Makefile b/src/port/Makefile
index f03a17a..f515847 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -31,7 +31,7 @@ override CPPFLAGS := -I$(top_builddir)/src/port -DFRONTEND $(CPPFLAGS)
 LIBS += $(PTHREAD_LIBS)

 OBJS = $(LIBOBJS) chklocale.o copydir.o dirmod.o exec.o noblock.o path.o \
-    pgsleep.o pgstrcasecmp.o qsort.o qsort_arg.o sprompt.o thread.o
+    path_fe.o pgsleep.o pgstrcasecmp.o qsort.o qsort_arg.o sprompt.o thread.o
 ifneq (,$(filter $(PORTNAME),cygwin win32))
 OBJS += pipe.o
 endif
diff --git a/src/port/path.c b/src/port/path.c
index a841ef4..bfc0be3 100644
--- a/src/port/path.c
+++ b/src/port/path.c
@@ -46,94 +46,6 @@
 #define IS_PATH_SEP(ch) ((ch) == ';')
 #endif

-static void make_relative_path(char *ret_path, const char *target_path,
-                   const char *bin_path, const char *my_exec_path);
-static void trim_directory(char *path);
-static void trim_trailing_separator(char *path);
-
-
-/*
- * skip_drive
- *
- * On Windows, a path may begin with "C:" or "//network/".    Advance over
- * this and point to the effective start of the path.
- */
-#ifdef WIN32
-
-static char *
-skip_drive(const char *path)
-{
-    if (IS_DIR_SEP(path[0]) && IS_DIR_SEP(path[1]))
-    {
-        path += 2;
-        while (*path && !IS_DIR_SEP(*path))
-            path++;
-    }
-    else if (isalpha((unsigned char) path[0]) && path[1] == ':')
-    {
-        path += 2;
-    }
-    return (char *) path;
-}
-#else
-
-#define skip_drive(path)    (path)
-#endif
-
-/*
- *    first_dir_separator
- *
- * Find the location of the first directory separator, return
- * NULL if not found.
- */
-char *
-first_dir_separator(const char *filename)
-{
-    const char *p;
-
-    for (p = skip_drive(filename); *p; p++)
-        if (IS_DIR_SEP(*p))
-            return (char *) p;
-    return NULL;
-}
-
-/*
- *    first_path_separator
- *
- * Find the location of the first path separator (i.e. ':' on
- * Unix, ';' on Windows), return NULL if not found.
- */
-char *
-first_path_separator(const char *pathlist)
-{
-    const char *p;
-
-    /* skip_drive is not needed */
-    for (p = pathlist; *p; p++)
-        if (IS_PATH_SEP(*p))
-            return (char *) p;
-    return NULL;
-}
-
-/*
- *    last_dir_separator
- *
- * Find the location of the last directory separator, return
- * NULL if not found.
- */
-char *
-last_dir_separator(const char *filename)
-{
-    const char *p,
-               *ret = NULL;
-
-    for (p = skip_drive(filename); *p; p++)
-        if (IS_DIR_SEP(*p))
-            ret = p;
-    return (char *) ret;
-}
-
-
 /*
  *    make_native_path - on WIN32, change / to \ in the path
  *
@@ -210,130 +122,6 @@ join_path_components(char *ret_path,
                  "/%s", tail);
 }

-
-/*
- *    Clean up path by:
- *        o  make Win32 path use Unix slashes
- *        o  remove trailing quote on Win32
- *        o  remove trailing slash
- *        o  remove duplicate adjacent separators
- *        o  remove trailing '.'
- *        o  process trailing '..' ourselves
- */
-void
-canonicalize_path(char *path)
-{
-    char       *p,
-               *to_p;
-    char       *spath;
-    bool        was_sep = false;
-    int            pending_strips;
-
-#ifdef WIN32
-
-    /*
-     * The Windows command processor will accept suitably quoted paths with
-     * forward slashes, but barfs badly with mixed forward and back slashes.
-     */
-    for (p = path; *p; p++)
-    {
-        if (*p == '\\')
-            *p = '/';
-    }
-
-    /*
-     * In Win32, if you do: prog.exe "a b" "\c\d\" the system will pass \c\d"
-     * as argv[2], so trim off trailing quote.
-     */
-    if (p > path && *(p - 1) == '"')
-        *(p - 1) = '/';
-#endif
-
-    /*
-     * 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 duplicate adjacent separators
-     */
-    p = path;
-#ifdef WIN32
-    /* Don't remove leading double-slash on Win32 */
-    if (*p)
-        p++;
-#endif
-    to_p = p;
-    for (; *p; p++, to_p++)
-    {
-        /* Handle many adjacent slashes, like "/a///b" */
-        while (*p == '/' && was_sep)
-            p++;
-        if (to_p != p)
-            *to_p = *p;
-        was_sep = (*p == '/');
-    }
-    *to_p = '\0';
-
-    /*
-     * Remove any trailing uses of "." and process ".." ourselves
-     *
-     * Note that "/../.." should reduce to just "/", while "../.." has to be
-     * kept as-is.    In the latter case we put back mistakenly trimmed ".."
-     * components below.  Also note that we want a Windows drive spec to be
-     * visible to trim_directory(), but it's not part of the logic that's
-     * looking at the name components; hence distinction between path and
-     * spath.
-     */
-    spath = skip_drive(path);
-    pending_strips = 0;
-    for (;;)
-    {
-        int            len = strlen(spath);
-
-        if (len >= 2 && strcmp(spath + len - 2, "/.") == 0)
-            trim_directory(path);
-        else if (strcmp(spath, ".") == 0)
-        {
-            /* Want to leave "." alone, but "./.." has to become ".." */
-            if (pending_strips > 0)
-                *spath = '\0';
-            break;
-        }
-        else if ((len >= 3 && strcmp(spath + len - 3, "/..") == 0) ||
-                 strcmp(spath, "..") == 0)
-        {
-            trim_directory(path);
-            pending_strips++;
-        }
-        else if (pending_strips > 0 && *spath != '\0')
-        {
-            /* trim a regular directory name cancelled by ".." */
-            trim_directory(path);
-            pending_strips--;
-            /* foo/.. should become ".", not empty */
-            if (*spath == '\0')
-                strcpy(spath, ".");
-        }
-        else
-            break;
-    }
-
-    if (pending_strips > 0)
-    {
-        /*
-         * We could only get here if path is now totally empty (other than a
-         * possible drive specifier on Windows). We have to put back one or
-         * more ".."'s that we took off.
-         */
-        while (--pending_strips > 0)
-            strcat(path, "../");
-        strcat(path, "..");
-    }
-}
-
 /*
  * Detect whether a path contains any parent-directory references ("..")
  *
@@ -672,53 +460,3 @@ get_parent_directory(char *path)
     trim_directory(path);
 }

-
-/*
- *    trim_directory
- *
- *    Trim trailing directory from path, that is, remove any trailing slashes,
- *    the last pathname component, and the slash just ahead of it --- but never
- *    remove a leading slash.
- */
-static void
-trim_directory(char *path)
-{
-    char       *p;
-
-    path = skip_drive(path);
-
-    if (path[0] == '\0')
-        return;
-
-    /* back up over trailing slash(es) */
-    for (p = path + strlen(path) - 1; IS_DIR_SEP(*p) && p > path; p--)
-        ;
-    /* back up over directory name */
-    for (; !IS_DIR_SEP(*p) && p > path; p--)
-        ;
-    /* if multiple slashes before directory name, remove 'em all */
-    for (; p > path && IS_DIR_SEP(*(p - 1)); p--)
-        ;
-    /* don't erase a leading slash */
-    if (p == path && IS_DIR_SEP(*p))
-        p++;
-    *p = '\0';
-}
-
-
-/*
- *    trim_trailing_separator
- *
- * trim off trailing slashes, but not a leading slash
- */
-static void
-trim_trailing_separator(char *path)
-{
-    char       *p;
-
-    path = skip_drive(path);
-    p = path + strlen(path);
-    if (p > path)
-        for (p--; p > path && IS_DIR_SEP(*p); p--)
-            *p = '\0';
-}
diff --git a/src/port/path_fe.c b/src/port/path_fe.c
new file mode 100644
index 0000000..e8ec271
--- /dev/null
+++ b/src/port/path_fe.c
@@ -0,0 +1,300 @@
+/*-------------------------------------------------------------------------
+ *
+ * path.c
+ *      portable path handling routines
+ *
+ * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ *      $PostgreSQL$
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "c.h"
+
+#include <ctype.h>
+#include <sys/stat.h>
+#ifdef WIN32
+#ifdef _WIN32_IE
+#undef _WIN32_IE
+#endif
+#define _WIN32_IE 0x0500
+#ifdef near
+#undef near
+#endif
+#define near
+#include <shlobj.h>
+#else
+#include <unistd.h>
+#endif
+
+#include "pg_config_paths.h"
+
+
+#ifndef WIN32
+#define IS_DIR_SEP(ch)    ((ch) == '/')
+#else
+#define IS_DIR_SEP(ch)    ((ch) == '/' || (ch) == '\\')
+#endif
+
+#ifndef WIN32
+#define IS_PATH_SEP(ch) ((ch) == ':')
+#else
+#define IS_PATH_SEP(ch) ((ch) == ';')
+#endif
+
+
+/*
+ * skip_drive
+ *
+ * On Windows, a path may begin with "C:" or "//network/".    Advance over
+ * this and point to the effective start of the path.
+ */
+#ifdef WIN32
+
+static char *
+skip_drive(const char *path)
+{
+    if (IS_DIR_SEP(path[0]) && IS_DIR_SEP(path[1]))
+    {
+        path += 2;
+        while (*path && !IS_DIR_SEP(*path))
+            path++;
+    }
+    else if (isalpha((unsigned char) path[0]) && path[1] == ':')
+    {
+        path += 2;
+    }
+    return (char *) path;
+}
+#endif
+
+/*
+ *    first_dir_separator
+ *
+ * Find the location of the first directory separator, return
+ * NULL if not found.
+ */
+char *
+first_dir_separator(const char *filename)
+{
+    const char *p;
+
+    for (p = skip_drive(filename); *p; p++)
+        if (IS_DIR_SEP(*p))
+            return (char *) p;
+    return NULL;
+}
+
+/*
+ *    first_path_separator
+ *
+ * Find the location of the first path separator (i.e. ':' on
+ * Unix, ';' on Windows), return NULL if not found.
+ */
+char *
+first_path_separator(const char *pathlist)
+{
+    const char *p;
+
+    /* skip_drive is not needed */
+    for (p = pathlist; *p; p++)
+        if (IS_PATH_SEP(*p))
+            return (char *) p;
+    return NULL;
+}
+
+/*
+ *    last_dir_separator
+ *
+ * Find the location of the last directory separator, return
+ * NULL if not found.
+ */
+char *
+last_dir_separator(const char *filename)
+{
+    const char *p,
+               *ret = NULL;
+
+    for (p = skip_drive(filename); *p; p++)
+        if (IS_DIR_SEP(*p))
+            ret = p;
+    return (char *) ret;
+}
+
+
+/*
+ *    Clean up path by:
+ *        o  make Win32 path use Unix slashes
+ *        o  remove trailing quote on Win32
+ *        o  remove trailing slash
+ *        o  remove duplicate adjacent separators
+ *        o  remove trailing '.'
+ *        o  process trailing '..' ourselves
+ */
+void
+canonicalize_path(char *path)
+{
+    char       *p,
+               *to_p;
+    char       *spath;
+    bool        was_sep = false;
+    int            pending_strips;
+
+#ifdef WIN32
+
+    /*
+     * The Windows command processor will accept suitably quoted paths with
+     * forward slashes, but barfs badly with mixed forward and back slashes.
+     */
+    for (p = path; *p; p++)
+    {
+        if (*p == '\\')
+            *p = '/';
+    }
+
+    /*
+     * In Win32, if you do: prog.exe "a b" "\c\d\" the system will pass \c\d"
+     * as argv[2], so trim off trailing quote.
+     */
+    if (p > path && *(p - 1) == '"')
+        *(p - 1) = '/';
+#endif
+
+    /*
+     * 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 duplicate adjacent separators
+     */
+    p = path;
+#ifdef WIN32
+    /* Don't remove leading double-slash on Win32 */
+    if (*p)
+        p++;
+#endif
+    to_p = p;
+    for (; *p; p++, to_p++)
+    {
+        /* Handle many adjacent slashes, like "/a///b" */
+        while (*p == '/' && was_sep)
+            p++;
+        if (to_p != p)
+            *to_p = *p;
+        was_sep = (*p == '/');
+    }
+    *to_p = '\0';
+
+    /*
+     * Remove any trailing uses of "." and process ".." ourselves
+     *
+     * Note that "/../.." should reduce to just "/", while "../.." has to be
+     * kept as-is.    In the latter case we put back mistakenly trimmed ".."
+     * components below.  Also note that we want a Windows drive spec to be
+     * visible to trim_directory(), but it's not part of the logic that's
+     * looking at the name components; hence distinction between path and
+     * spath.
+     */
+    spath = skip_drive(path);
+    pending_strips = 0;
+    for (;;)
+    {
+        int            len = strlen(spath);
+
+        if (len >= 2 && strcmp(spath + len - 2, "/.") == 0)
+            trim_directory(path);
+        else if (strcmp(spath, ".") == 0)
+        {
+            /* Want to leave "." alone, but "./.." has to become ".." */
+            if (pending_strips > 0)
+                *spath = '\0';
+            break;
+        }
+        else if ((len >= 3 && strcmp(spath + len - 3, "/..") == 0) ||
+                 strcmp(spath, "..") == 0)
+        {
+            trim_directory(path);
+            pending_strips++;
+        }
+        else if (pending_strips > 0 && *spath != '\0')
+        {
+            /* trim a regular directory name cancelled by ".." */
+            trim_directory(path);
+            pending_strips--;
+            /* foo/.. should become ".", not empty */
+            if (*spath == '\0')
+                strcpy(spath, ".");
+        }
+        else
+            break;
+    }
+
+    if (pending_strips > 0)
+    {
+        /*
+         * We could only get here if path is now totally empty (other than a
+         * possible drive specifier on Windows). We have to put back one or
+         * more ".."'s that we took off.
+         */
+        while (--pending_strips > 0)
+            strcat(path, "../");
+        strcat(path, "..");
+    }
+}
+
+/*
+ *    trim_directory
+ *
+ *    Trim trailing directory from path, that is, remove any trailing slashes,
+ *    the last pathname component, and the slash just ahead of it --- but never
+ *    remove a leading slash.
+ */
+void
+trim_directory(char *path)
+{
+    char       *p;
+
+    path = skip_drive(path);
+
+    if (path[0] == '\0')
+        return;
+
+    /* back up over trailing slash(es) */
+    for (p = path + strlen(path) - 1; IS_DIR_SEP(*p) && p > path; p--)
+        ;
+    /* back up over directory name */
+    for (; !IS_DIR_SEP(*p) && p > path; p--)
+        ;
+    /* if multiple slashes before directory name, remove 'em all */
+    for (; p > path && IS_DIR_SEP(*(p - 1)); p--)
+        ;
+    /* don't erase a leading slash */
+    if (p == path && IS_DIR_SEP(*p))
+        p++;
+    *p = '\0';
+}
+
+
+/*
+ *    trim_trailing_separator
+ *
+ * trim off trailing slashes, but not a leading slash
+ */
+void
+trim_trailing_separator(char *path)
+{
+    char       *p;
+
+    path = skip_drive(path);
+    p = path + strlen(path);
+    if (p > path)
+        for (p--; p > path && IS_DIR_SEP(*p); p--)
+            *p = '\0';
+}