Обсуждение: Re: [HACKERS] Found small issue with OUT params

Поиск
Список
Период
Сортировка

Re: [HACKERS] Found small issue with OUT params

От
Bruce Momjian
Дата:
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);
 }

Re: Making pgxs builds work with a relocated installation

От
Tom Lane
Дата:
[ 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

Re: [HACKERS] Found small issue with OUT params

От
Bruce Momjian
Дата:
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

Сайт использует файлы cookie для корректной работы и повышения удобства. Нажимая кнопку «Принять» или продолжая пользоваться сайтом, вы соглашаетесь на их использование в соответствии с Политикой в отношении обработки cookie ООО «ППГ», в том числе на передачу данных из файлов cookie сторонним статистическим и рекламным службам. Вы можете управлять настройками cookie через параметры вашего браузера