Thread: Re: [HACKERS] Found small issue with OUT params
Oh, good, I missed that patch.
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.
---------------------------------------------------------------------------
>
>
>
> -----Original Message-----
> From: pgsql-hackers-owner@postgresql.org on behalf of Bruce Momjian
> Sent: Sat 10/1/2005 1:16 AM
> To: Jim C. Nasby
> Cc: Joshua D. Drake; Tony Caduto; pgsql-hackers@postgresql.org
> Subject: Re: [HACKERS] Found small issue with OUT params
>
> > fix pgxs for spaces in file names
>
> I posted a patch for that.
>
> http://archives.postgresql.org/pgsql-patches/2005-09/msg00137.php
>
> Regards, Dave
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
> http://archives.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
Index: src/bin/pg_config/pg_config.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/pg_config/pg_config.c,v
retrieving revision 1.13
diff -u -r1.13 pg_config.c
--- src/bin/pg_config/pg_config.c 27 Sep 2005 17:39:33 -0000 1.13
+++ src/bin/pg_config/pg_config.c 28 Sep 2005 12:13:13 -0000
@@ -31,6 +31,63 @@
/*
+ * This function cleans up the paths for use with either cmd.exe or Msys
+ * on Windows. We need them to use double backslashes and filenames without
+ * spaces (for which a short filename is the safest equivalent) eg:
+ * C:\\Progra~1\\
+ *
+ * This can fail in 2 ways - if the path doesn't exist, or short names are
+ * disabled. In the first case, don't return any path. In the second case,
+ * we leave the path in the long form. In this case, it does still seem to
+ * fix elements containing spaces which is all we actually need.
+ */
+static void
+cleanup_path(char *path)
+{
+#ifdef WIN32
+ int x=0, y=0;
+ char temp[MAXPGPATH];
+
+ if (GetShortPathName(path, path, MAXPGPATH - 1) == 0)
+ {
+ /* Ignore ERROR_INVALID_PARAMETER as it almost certainly
+ * means that short names are disabled
+ */
+ if (GetLastError() != ERROR_INVALID_PARAMETER)
+ {
+ path[0] = '\0';
+ return;
+ }
+ }
+
+
+ /* Replace '\' with '\\'. */
+ for (x = 0; x < strlen(path); x++)
+ {
+ if (path[x] == '/' || path[x] == '\\')
+ {
+ temp[y] = '\\';
+ y++;
+ temp[y] = '\\';
+ }
+ else
+ {
+ temp[y] = path[x];
+ }
+
+ y++;
+
+ /* Bail out if we're too close to MAXPGPATH */
+ if (y >= MAXPGPATH - 2)
+ break;
+ }
+ temp[y] = '\0';
+
+ strncpy(path, temp, MAXPGPATH - 1);
+#endif
+}
+
+
+/*
* For each piece of information known to pg_config, we define a subroutine
* to print it. This is probably overkill, but it avoids code duplication
* and accidentally omitting items from the "all" display.
@@ -39,138 +96,152 @@
static void
show_bindir(bool all)
{
- char path[MAXPGPATH];
- char *lastsep;
+ char path[MAXPGPATH];
+ char *lastsep;
if (all)
printf("BINDIR = ");
/* assume we are located in the bindir */
strcpy(path, mypath);
lastsep = strrchr(path, '/');
+
if (lastsep)
*lastsep = '\0';
+
+ cleanup_path(path);
printf("%s\n", path);
}
static void
show_docdir(bool all)
{
- char path[MAXPGPATH];
+ char path[MAXPGPATH];
if (all)
printf("DOCDIR = ");
get_doc_path(mypath, path);
+ cleanup_path(path);
printf("%s\n", path);
}
static void
show_includedir(bool all)
{
- char path[MAXPGPATH];
+ char path[MAXPGPATH];
if (all)
printf("INCLUDEDIR = ");
get_include_path(mypath, path);
+ cleanup_path(path);
printf("%s\n", path);
}
static void
show_pkgincludedir(bool all)
{
- char path[MAXPGPATH];
+ char path[MAXPGPATH];
if (all)
printf("PKGINCLUDEDIR = ");
get_pkginclude_path(mypath, path);
+ cleanup_path(path);
printf("%s\n", path);
}
static void
show_includedir_server(bool all)
{
- char path[MAXPGPATH];
+ char path[MAXPGPATH];
if (all)
printf("INCLUDEDIR-SERVER = ");
get_includeserver_path(mypath, path);
+ cleanup_path(path);
printf("%s\n", path);
}
static void
show_libdir(bool all)
{
- char path[MAXPGPATH];
+ char path[MAXPGPATH];
if (all)
printf("LIBDIR = ");
get_lib_path(mypath, path);
+ cleanup_path(path);
printf("%s\n", path);
}
static void
show_pkglibdir(bool all)
{
- char path[MAXPGPATH];
+ char path[MAXPGPATH];
if (all)
printf("PKGLIBDIR = ");
get_pkglib_path(mypath, path);
+ cleanup_path(path);
printf("%s\n", path);
}
static void
show_localedir(bool all)
{
- char path[MAXPGPATH];
+ char path[MAXPGPATH];
if (all)
printf("LOCALEDIR = ");
get_locale_path(mypath, path);
+ cleanup_path(path);
printf("%s\n", path);
}
static void
show_mandir(bool all)
{
- char path[MAXPGPATH];
+ char path[MAXPGPATH];
if (all)
printf("MANDIR = ");
get_man_path(mypath, path);
+ cleanup_path(path);
printf("%s\n", path);
}
static void
show_sharedir(bool all)
{
- char path[MAXPGPATH];
+ char path[MAXPGPATH];
if (all)
printf("SHAREDIR = ");
get_share_path(mypath, path);
+ cleanup_path(path);
printf("%s\n", path);
}
static void
show_sysconfdir(bool all)
{
- char path[MAXPGPATH];
+ char path[MAXPGPATH];
if (all)
printf("SYSCONFDIR = ");
get_etc_path(mypath, path);
+ cleanup_path(path);
printf("%s\n", path);
}
static void
show_pgxs(bool all)
{
- char path[MAXPGPATH];
+ char path[MAXPGPATH];
if (all)
printf("PGXS = ");
get_pkglib_path(mypath, path);
strncat(path, "/pgxs/src/makefiles/pgxs.mk", MAXPGPATH - 1);
+ cleanup_path(path);
printf("%s\n", path);
}
[ correcting the completely offtopic Subject ] Bruce Momjian <pgman@candle.pha.pa.us> writes: > Your patch has been added to the PostgreSQL unapplied patches list at: > http://momjian.postgresql.org/cgi-bin/pgpatches > It will be applied as soon as one of the PostgreSQL committers reviews > and approves it. Before going down that path, I wanted to review the makefiles and see whether quoting all occurrences of the path variables would be a workable thing or not. That would be more desirable, if it's not a maintenance nightmare, because it'd solve the problem everywhere instead of only for Windows --- and there is a real risk on, say, Darwin. One idea that comes to mind is to just quote the variables at point of creation --- that is, write something like override pkglibdir := "$(pkglibdir)" in Makefile.global. I haven't tried this to see if it'd work or not ... any comments? Would single or double quotes be most desirable? regards, tom lane
Patch applied. Thanks. --------------------------------------------------------------------------- Bruce Momjian wrote: > > Oh, good, I missed that patch. > > Your patch has been added to the PostgreSQL unapplied patches list at: > > http://momjian.postgresql.org/cgi-bin/pgpatches > > It will be applied as soon as one of the PostgreSQL committers reviews > and approves it. > > --------------------------------------------------------------------------- > > > > > > > > > > -----Original Message----- > > From: pgsql-hackers-owner@postgresql.org on behalf of Bruce Momjian > > Sent: Sat 10/1/2005 1:16 AM > > To: Jim C. Nasby > > Cc: Joshua D. Drake; Tony Caduto; pgsql-hackers@postgresql.org > > Subject: Re: [HACKERS] Found small issue with OUT params > > > > > fix pgxs for spaces in file names > > > > I posted a patch for that. > > > > http://archives.postgresql.org/pgsql-patches/2005-09/msg00137.php > > > > Regards, Dave > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 4: Have you searched our list archives? > > > > http://archives.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 > Index: src/bin/pg_config/pg_config.c > =================================================================== > RCS file: /projects/cvsroot/pgsql/src/bin/pg_config/pg_config.c,v > retrieving revision 1.13 > diff -u -r1.13 pg_config.c > --- src/bin/pg_config/pg_config.c 27 Sep 2005 17:39:33 -0000 1.13 > +++ src/bin/pg_config/pg_config.c 28 Sep 2005 12:13:13 -0000 > @@ -31,6 +31,63 @@ > > > /* > + * This function cleans up the paths for use with either cmd.exe or Msys > + * on Windows. We need them to use double backslashes and filenames without > + * spaces (for which a short filename is the safest equivalent) eg: > + * C:\\Progra~1\\ > + * > + * This can fail in 2 ways - if the path doesn't exist, or short names are > + * disabled. In the first case, don't return any path. In the second case, > + * we leave the path in the long form. In this case, it does still seem to > + * fix elements containing spaces which is all we actually need. > + */ > +static void > +cleanup_path(char *path) > +{ > +#ifdef WIN32 > + int x=0, y=0; > + char temp[MAXPGPATH]; > + > + if (GetShortPathName(path, path, MAXPGPATH - 1) == 0) > + { > + /* Ignore ERROR_INVALID_PARAMETER as it almost certainly > + * means that short names are disabled > + */ > + if (GetLastError() != ERROR_INVALID_PARAMETER) > + { > + path[0] = '\0'; > + return; > + } > + } > + > + > + /* Replace '\' with '\\'. */ > + for (x = 0; x < strlen(path); x++) > + { > + if (path[x] == '/' || path[x] == '\\') > + { > + temp[y] = '\\'; > + y++; > + temp[y] = '\\'; > + } > + else > + { > + temp[y] = path[x]; > + } > + > + y++; > + > + /* Bail out if we're too close to MAXPGPATH */ > + if (y >= MAXPGPATH - 2) > + break; > + } > + temp[y] = '\0'; > + > + strncpy(path, temp, MAXPGPATH - 1); > +#endif > +} > + > + > +/* > * For each piece of information known to pg_config, we define a subroutine > * to print it. This is probably overkill, but it avoids code duplication > * and accidentally omitting items from the "all" display. > @@ -39,138 +96,152 @@ > static void > show_bindir(bool all) > { > - char path[MAXPGPATH]; > - char *lastsep; > + char path[MAXPGPATH]; > + char *lastsep; > > if (all) > printf("BINDIR = "); > /* assume we are located in the bindir */ > strcpy(path, mypath); > lastsep = strrchr(path, '/'); > + > if (lastsep) > *lastsep = '\0'; > + > + cleanup_path(path); > printf("%s\n", path); > } > > static void > show_docdir(bool all) > { > - char path[MAXPGPATH]; > + char path[MAXPGPATH]; > > if (all) > printf("DOCDIR = "); > get_doc_path(mypath, path); > + cleanup_path(path); > printf("%s\n", path); > } > > static void > show_includedir(bool all) > { > - char path[MAXPGPATH]; > + char path[MAXPGPATH]; > > if (all) > printf("INCLUDEDIR = "); > get_include_path(mypath, path); > + cleanup_path(path); > printf("%s\n", path); > } > > static void > show_pkgincludedir(bool all) > { > - char path[MAXPGPATH]; > + char path[MAXPGPATH]; > > if (all) > printf("PKGINCLUDEDIR = "); > get_pkginclude_path(mypath, path); > + cleanup_path(path); > printf("%s\n", path); > } > > static void > show_includedir_server(bool all) > { > - char path[MAXPGPATH]; > + char path[MAXPGPATH]; > > if (all) > printf("INCLUDEDIR-SERVER = "); > get_includeserver_path(mypath, path); > + cleanup_path(path); > printf("%s\n", path); > } > > static void > show_libdir(bool all) > { > - char path[MAXPGPATH]; > + char path[MAXPGPATH]; > > if (all) > printf("LIBDIR = "); > get_lib_path(mypath, path); > + cleanup_path(path); > printf("%s\n", path); > } > > static void > show_pkglibdir(bool all) > { > - char path[MAXPGPATH]; > + char path[MAXPGPATH]; > > if (all) > printf("PKGLIBDIR = "); > get_pkglib_path(mypath, path); > + cleanup_path(path); > printf("%s\n", path); > } > > static void > show_localedir(bool all) > { > - char path[MAXPGPATH]; > + char path[MAXPGPATH]; > > if (all) > printf("LOCALEDIR = "); > get_locale_path(mypath, path); > + cleanup_path(path); > printf("%s\n", path); > } > > static void > show_mandir(bool all) > { > - char path[MAXPGPATH]; > + char path[MAXPGPATH]; > > if (all) > printf("MANDIR = "); > get_man_path(mypath, path); > + cleanup_path(path); > printf("%s\n", path); > } > > static void > show_sharedir(bool all) > { > - char path[MAXPGPATH]; > + char path[MAXPGPATH]; > > if (all) > printf("SHAREDIR = "); > get_share_path(mypath, path); > + cleanup_path(path); > printf("%s\n", path); > } > > static void > show_sysconfdir(bool all) > { > - char path[MAXPGPATH]; > + char path[MAXPGPATH]; > > if (all) > printf("SYSCONFDIR = "); > get_etc_path(mypath, path); > + cleanup_path(path); > printf("%s\n", path); > } > > static void > show_pgxs(bool all) > { > - char path[MAXPGPATH]; > + char path[MAXPGPATH]; > > if (all) > printf("PGXS = "); > get_pkglib_path(mypath, path); > strncat(path, "/pgxs/src/makefiles/pgxs.mk", MAXPGPATH - 1); > + cleanup_path(path); > printf("%s\n", path); > } > > ---------------------------(end of broadcast)--------------------------- > TIP 1: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly -- 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