Thread: Path separator
I've seen a couple of reports that the new SSL error messages on windows
look strange with paths the wrong way. For example:
root certificate file "C:\Documents and Settings\<SNIP>\Application
Data/postgresql/root.crt" does not exist.
The issue being the mix of forward and backwards slashes. Attached patch
should fix this.
Is this worth doing? Comments?
//Magnus
diff --git a/src/include/port.h b/src/include/port.h
index 0557dd2..951f5ac 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -100,6 +100,12 @@ extern BOOL AddUserToDacl(HANDLE hProcess);
#define DEVTTY "/dev/tty"
#endif
+#if defined(WIN32) || defined(__CYGWIN__)
+#define PATH_SEPARATOR "\\"
+#else
+#define PATH_SEPARATOR "/"
+#endif
+
/*
* Win32 needs double quotes at the beginning and end of system()
* strings. If not, it gets confused with multiple quoted strings.
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 0833603..9f46c99 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3964,7 +3964,7 @@ pqGetHomeDirectory(char *buf, int bufsize)
ZeroMemory(tmppath, sizeof(tmppath));
if (SHGetFolderPath(NULL, CSIDL_APPDATA, NULL, 0, tmppath) != S_OK)
return false;
- snprintf(buf, bufsize, "%s/postgresql", tmppath);
+ snprintf(buf, bufsize, "%s\\postgresql", tmppath);
return true;
#endif
}
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 7c229c3..1b14f90 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -578,7 +578,7 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
if (conn->sslcert)
strncpy(fnbuf, conn->sslcert, sizeof(fnbuf));
else
- snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_CERT_FILE);
+ snprintf(fnbuf, sizeof(fnbuf), "%s" PATH_SEPARATOR "%s", homedir, USER_CERT_FILE);
/*
* OpenSSL <= 0.9.8 lacks error stack handling, which means it's likely to
@@ -693,7 +693,7 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
else
{
/* No PGSSLKEY specified, load default file */
- snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_KEY_FILE);
+ snprintf(fnbuf, sizeof(fnbuf), "%s" PATH_SEPARATOR "%s", homedir, USER_KEY_FILE);
}
if (fnbuf[0] != '\0')
@@ -998,7 +998,7 @@ initialize_SSL(PGconn *conn)
if (conn->sslrootcert)
strncpy(fnbuf, conn->sslrootcert, sizeof(fnbuf));
else
- snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CERT_FILE);
+ snprintf(fnbuf, sizeof(fnbuf), "%s" PATH_SEPARATOR "%s", homedir, ROOT_CERT_FILE);
if (stat(fnbuf, &buf) == 0)
{
@@ -1020,7 +1020,7 @@ initialize_SSL(PGconn *conn)
if (conn->sslcrl)
strncpy(fnbuf, conn->sslcrl, sizeof(fnbuf));
else
- snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CRL_FILE);
+ snprintf(fnbuf, sizeof(fnbuf), "%s" PATH_SEPARATOR "%s", homedir, ROOT_CRL_FILE);
/* setting the flags to check against the complete CRL chain */
if (X509_STORE_load_locations(cvstore, fnbuf, NULL) == 1)
On Wed, Mar 18, 2009 at 9:24 AM, Magnus Hagander <magnus@hagander.net> wrote: > I've seen a couple of reports that the new SSL error messages on windows > look strange with paths the wrong way. For example: > > root certificate file "C:\Documents and Settings\<SNIP>\Application > Data/postgresql/root.crt" does not exist. > > The issue being the mix of forward and backwards slashes. Attached patch > should fix this. > > Is this worth doing? Comments? Yes. I've seen a couple of complaints as well, and both thought the error was actually the mixed slashes causing the problem, and *not* the missing root.crt. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Magnus Hagander <magnus@hagander.net> writes:
> I've seen a couple of reports that the new SSL error messages on windows
> look strange with paths the wrong way. For example:
> root certificate file "C:\Documents and Settings\<SNIP>\Application
> Data/postgresql/root.crt" does not exist.
> The issue being the mix of forward and backwards slashes. Attached patch
> should fix this.
> Is this worth doing? Comments?
In view of the way that canonicalize_path() works, I can't help thinking
this is going in precisely the wrong direction.
Also, don't we already have a macro someplace for the platform's
preferred path separator?
regards, tom lane
Tom Lane wrote: > Magnus Hagander <magnus@hagander.net> writes: >> I've seen a couple of reports that the new SSL error messages on windows >> look strange with paths the wrong way. For example: > >> root certificate file "C:\Documents and Settings\<SNIP>\Application >> Data/postgresql/root.crt" does not exist. > >> The issue being the mix of forward and backwards slashes. Attached patch >> should fix this. > >> Is this worth doing? Comments? > > In view of the way that canonicalize_path() works, I can't help thinking > this is going in precisely the wrong direction. In a way, yes. But canonicalize_path() runs only in the backend, and this is only in the frontend. I think the requirements on the frontend are slightly different than the backend. But the most important thing is to be consistent within the same path as we report it I think, so we could switch all to forward slashes as well if you think that's better. > > Also, don't we already have a macro someplace for the platform's > preferred path separator? I looked for a macro for it, didn't find it. It seems to be hardcoded. We have macros for SYSTEM_QUOTE for example, but not for the path separator AFAICF. I just realized we have a make_native_path() function, I had completely missed that one. So we could possibly use that instead. In the end it does the same thing //Magnus
Magnus Hagander <magnus@hagander.net> writes:
> Tom Lane wrote:
>> In view of the way that canonicalize_path() works, I can't help thinking
>> this is going in precisely the wrong direction.
> In a way, yes. But canonicalize_path() runs only in the backend, and
> this is only in the frontend. I think the requirements on the frontend
> are slightly different than the backend.
Just for the record, canonicalize_path does work in the frontend;
we have uses of it in psql and pg_ctl. But we have previously decided
not to import path.c into libpq, which is where the present issue is,
so yes there is a problem with using it. The same objection applies to
make_native_path unfortunately.
> But the most important thing is to be consistent within the same path as
> we report it I think, so we could switch all to forward slashes as well
> if you think that's better.
What I'm concerned about is the prospect that if we do this here,
we're going to end up trying to do it all over the frontend code.
(And I'm not entirely convinced that it doesn't then propagate into
the backend, too, but even just the frontend code is bad enough.)
> I just realized we have a make_native_path() function, I had completely
> missed that one. So we could possibly use that instead. In the end it
> does the same thing
I'd definitely favor using make_native_path over hand-rolled code.
But I guess what I'm suggesting is that it'd be more consistent with
our previous choices to apply canonicalize_path instead.
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?
regards, tom lane
Tom Lane wrote: > Magnus Hagander <magnus@hagander.net> writes: >> Tom Lane wrote: >>> In view of the way that canonicalize_path() works, I can't help thinking >>> this is going in precisely the wrong direction. > >> In a way, yes. But canonicalize_path() runs only in the backend, and >> this is only in the frontend. I think the requirements on the frontend >> are slightly different than the backend. > > Just for the record, canonicalize_path does work in the frontend; > we have uses of it in psql and pg_ctl. But we have previously decided > not to import path.c into libpq, which is where the present issue is, > so yes there is a problem with using it. The same objection applies to > make_native_path unfortunately. That's what I meant. And yeah, that's true about make_native_path as well. Should we reconsider this decision? >> But the most important thing is to be consistent within the same path as >> we report it I think, so we could switch all to forward slashes as well >> if you think that's better. > > What I'm concerned about is the prospect that if we do this here, > we're going to end up trying to do it all over the frontend code. > (And I'm not entirely convinced that it doesn't then propagate into > the backend, too, but even just the frontend code is bad enough.) True. >> I just realized we have a make_native_path() function, I had completely >> missed that one. So we could possibly use that instead. In the end it >> does the same thing > > I'd definitely favor using make_native_path over hand-rolled code. > But I guess what I'm suggesting is that it'd be more consistent with > our previous choices to apply canonicalize_path instead. I agree that this is probably the best way to do it. > 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. Is it worth taking a look at exactly what the sizes end up being? Shouldn't be all that much work... //Magnus
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. //Magnus
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';
+}
Magnus Hagander <magnus@hagander.net> writes:
> Tom Lane 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?
I'm more worried about the external dependencies pulled in by the
path-discovery stuff (geteuid for instance). None of that is code
that libpq needs at all.
> 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)
The normal expectation is that .o files are the unit of linking. There
might be a platform or two that is smarter, but they are not the norm.
Since I'm the one who's hot about this, I'm willing to do the work.
Do you agree that canonicalize_path and make_native_path are all that
we want to push into libpq for now? If so, I'll rename them to
pg_..._path and push them into a separate source file.
regards, tom lane
I wrote:
> Since I'm the one who's hot about this, I'm willing to do the work.
Belay that ... I'll review your patch instead, later today sometime.
regards, tom lane
Magnus Hagander <magnus@hagander.net> writes:
>> 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?
Hm, this seems to make the namespace pollution problem worse not better,
because of de-staticizing so many functions. I guess we could stick pg_
prefixes on all of them. That's a bit ugly; anybody have a better idea?
It might be that it'd be better to push a couple more of the simple
path-munging functions (like join_path_components) over into the new
file, so as to have a more logical division of responsibilities.
In my mind the two key areas here are "path syntax knowledge" and
"extracting absolute paths from environmental information". The second
part seems to be the one that doesn't belong on the frontend side.
regards, tom lane
Tom Lane wrote: > Magnus Hagander <magnus@hagander.net> writes: >>> 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? > > Hm, this seems to make the namespace pollution problem worse not better, > because of de-staticizing so many functions. I guess we could stick pg_ > prefixes on all of them. That's a bit ugly; anybody have a better idea? Not really. > It might be that it'd be better to push a couple more of the simple > path-munging functions (like join_path_components) over into the new > file, so as to have a more logical division of responsibilities. > In my mind the two key areas here are "path syntax knowledge" and > "extracting absolute paths from environmental information". The second > part seems to be the one that doesn't belong on the frontend side. What would be the gain there? To be able to re-static-ify for example skip_drive? Or just a nicer division? We should probably also consider moving get_home_path() over to the frontend one, and get rid of the copy that's in fe-connect.c. //Magnus
I assume we never came to any conclusion on this. --------------------------------------------------------------------------- Magnus Hagander wrote: > Tom Lane wrote: > > Magnus Hagander <magnus@hagander.net> writes: > >>> 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? > > > > Hm, this seems to make the namespace pollution problem worse not better, > > because of de-staticizing so many functions. I guess we could stick pg_ > > prefixes on all of them. That's a bit ugly; anybody have a better idea? > > Not really. > > > > It might be that it'd be better to push a couple more of the simple > > path-munging functions (like join_path_components) over into the new > > file, so as to have a more logical division of responsibilities. > > In my mind the two key areas here are "path syntax knowledge" and > > "extracting absolute paths from environmental information". The second > > part seems to be the one that doesn't belong on the frontend side. > > What would be the gain there? To be able to re-static-ify for example > skip_drive? Or just a nicer division? > > We should probably also consider moving get_home_path() over to the > frontend one, and get rid of the copy that's in fe-connect.c. > > > //Magnus > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.comPG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do + If your life is a hard drive,Christ can be your backup. +
yeah, we have a couple of patches that aren't good... And I kind of lost track of it waiting for feedback on my last question. It's not a must-fix, but it'd be good to get better messages eventually. //Magnus 2010/2/26 Bruce Momjian <bruce@momjian.us>: > > I assume we never came to any conclusion on this. > > --------------------------------------------------------------------------- > > Magnus Hagander wrote: >> Tom Lane wrote: >> > Magnus Hagander <magnus@hagander.net> writes: >> >>> 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? >> > >> > Hm, this seems to make the namespace pollution problem worse not better, >> > because of de-staticizing so many functions. I guess we could stick pg_ >> > prefixes on all of them. That's a bit ugly; anybody have a better idea? >> >> Not really. >> >> >> > It might be that it'd be better to push a couple more of the simple >> > path-munging functions (like join_path_components) over into the new >> > file, so as to have a more logical division of responsibilities. >> > In my mind the two key areas here are "path syntax knowledge" and >> > "extracting absolute paths from environmental information". The second >> > part seems to be the one that doesn't belong on the frontend side. >> >> What would be the gain there? To be able to re-static-ify for example >> skip_drive? Or just a nicer division? >> >> We should probably also consider moving get_home_path() over to the >> frontend one, and get rid of the copy that's in fe-connect.c. >> >> >> //Magnus >> >> >> -- >> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) >> To make changes to your subscription: >> http://www.postgresql.org/mailpref/pgsql-hackers > > -- > Bruce Momjian <bruce@momjian.us> http://momjian.us > EnterpriseDB http://enterprisedb.com > PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do > + If your life is a hard drive, Christ can be your backup. + > -- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/