Fix initdb for path with whitespace and at char
Hi,
On win32, initdb fails if it's path includes a space and at ('@')
character. E.g.
C:\>"C:\Program Files\user@company\Postgres\9.3\bin\initdb.exe" -D "c:\baz"
'C:\Program' is not recognized as an internal or external command,
operable program or batch file.
Here's a patch that fixes initdb by enclosing the command string in
extra double quotes being passed to popen() (similar to system() call).
Thanks,
Nikhil
Attachments:
0001-Fix-initdb-for-path-with-whitespace-and-at-char.patchapplication/octet-stream; name=0001-Fix-initdb-for-path-with-whitespace-and-at-char.patchDownload
From 55903d487b0fd191cb23420ba2675c470f4567f3 Mon Sep 17 00:00:00 2001
From: Nikhil R Deshpande <nikhail@gmail.com>
Date: Fri, 25 Apr 2014 20:35:27 -0700
Subject: [PATCH] Fix initdb for path with whitespace and at char
initdb code invokes postgres backend to boostrap the database dir.
On win32, it has special consideration for handling paths with
whitespace and special chars (e.g. '@') when invoking system()
call (see comment for SYSTEMQUOTE in src/include/port.h).
But not for popen() call (which also invokes a shell specified by
COMSPEC env var, e.g. cmd.exe), this double-quoting is missing.
Executable path is already surrounded by double-quotes, but the
entire command line needs to be enclosed in double quotes too.
This patch adds those double quotes on win32 for entire command line
wherever backend_exec is invoked through popen(). Without this patch,
initdb fails to run when initdb path as both whitespaces and '@' char.
E.g. following command fails without this patch:
C:\>"C:\Program Files\user@company\Postgres\9.3\bin\initdb.exe" -D "c:\baz"
'C:\Program' is not recognized as an internal or external command,
operable program or batch file.
If initdb path has just whitespace or '@' char, but not both, it works fine.
---
src/bin/initdb/initdb.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 5fc7291..a0ef27b 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1444,7 +1444,7 @@ bootstrap_template1(void)
unsetenv("PGCLIENTENCODING");
snprintf(cmd, sizeof(cmd),
- "\"%s\" --boot -x1 %s %s %s",
+ SYSTEMQUOTE "\"%s\" --boot -x1 %s %s %s" SYSTEMQUOTE,
backend_exec,
data_checksums ? "-k" : "",
boot_options, talkargs);
@@ -1485,7 +1485,7 @@ setup_auth(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
@@ -1563,7 +1563,7 @@ get_set_pwd(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
@@ -1663,7 +1663,7 @@ setup_depend(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
@@ -1696,7 +1696,7 @@ setup_sysviews(void)
* We use -j here to avoid backslashing stuff in system_views.sql
*/
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s -j template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s -j template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
@@ -1727,7 +1727,7 @@ setup_description(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
@@ -1834,7 +1834,7 @@ setup_collation(void)
#if defined(HAVE_LOCALE_T) && !defined(WIN32)
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
@@ -1973,7 +1973,7 @@ setup_conversion(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
@@ -2011,7 +2011,7 @@ setup_dictionary(void)
* We use -j here to avoid backslashing stuff
*/
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s -j template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s -j template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
@@ -2062,7 +2062,7 @@ setup_privileges(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
@@ -2125,7 +2125,7 @@ setup_schema(void)
* We use -j here to avoid backslashing stuff in information_schema.sql
*/
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s -j template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s -j template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
@@ -2142,7 +2142,7 @@ setup_schema(void)
PG_CMD_CLOSE;
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
@@ -2176,7 +2176,7 @@ load_plpgsql(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
@@ -2201,7 +2201,7 @@ vacuum_db(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
@@ -2257,7 +2257,7 @@ make_template0(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
@@ -2289,7 +2289,7 @@ make_postgres(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- "\"%s\" %s template1 >%s",
+ SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
backend_exec, backend_options,
DEVNULL);
--
1.7.11.msysgit.1
On 04/29/2014 09:14 PM, Nikhil Deshpande wrote:
On win32, initdb fails if it's path includes a space and at ('@')
character. E.g.C:\>"C:\Program Files\user@company\Postgres\9.3\bin\initdb.exe" -D "c:\baz"
'C:\Program' is not recognized as an internal or external command,
operable program or batch file.Here's a patch that fixes initdb by enclosing the command string in
extra double quotes being passed to popen() (similar to system() call).
This looks correct to me. popen() requires SYSTEMQUOTEs on Windows, like
system() does. We already use SYSTEMQUOTEs in some popen() calls, like
in pg_ctl, but initdb is missing them. get_bin_version function in
pg_upgrade is also missing it, as is the popen() call in COPY TO/FROM
PROGRAM command.
Is there any situation where would *not* want to wrap the command in
SYSTEMQUOTEs? If there isn't, ISTM it would be better to create a
wrapper function, pgwin32_popen(), which adds the quotes instead of
sprinkling them into all callers. Even if we go around and fix all of
the callers now, we're bound to forget it again in the future.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Heikki Linnakangas <hlinnakangas@vmware.com> writes:
This looks correct to me. popen() requires SYSTEMQUOTEs on Windows, like
system() does. We already use SYSTEMQUOTEs in some popen() calls, like
in pg_ctl, but initdb is missing them. get_bin_version function in
pg_upgrade is also missing it, as is the popen() call in COPY TO/FROM
PROGRAM command.
Yuck.
Is there any situation where would *not* want to wrap the command in
SYSTEMQUOTEs? If there isn't, ISTM it would be better to create a
wrapper function, pgwin32_popen(), which adds the quotes instead of
sprinkling them into all callers. Even if we go around and fix all of
the callers now, we're bound to forget it again in the future.
We might forget to use the wrapper function too, if it has a nonstandard
name, no? A better idea would be to redefine popen() and system() on
Windows. It looks like we're already using a #define to redefine popen().
This approach would let us get rid of nonstandard notation for this
problem, instead of adding more.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Apr 30, 2014 at 3:57 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Heikki Linnakangas <hlinnakangas@vmware.com> writes:
This looks correct to me. popen() requires SYSTEMQUOTEs on Windows, like
system() does.
It seems right now SYSTEMQUOTE is used before popen both for
Windows and non-Windows, ex. adjust_data_dir() in pg_ctl.c
We might forget to use the wrapper function too, if it has a nonstandard
name, no? A better idea would be to redefine popen() and system() on
Windows. It looks like we're already using a #define to redefine popen().
Won't defining variant of popen just for Windows to add SYSTEMQUOTE
effect such (where currently it is used for both win and non-winows)
usage? Also, I think we might want to remove use of SYSTEMQUOTE
before popen calls where ever it is currently used to avoid usage of the
same two times.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Amit Kapila <amit.kapila16@gmail.com> writes:
On Wed, Apr 30, 2014 at 3:57 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
We might forget to use the wrapper function too, if it has a nonstandard
name, no? A better idea would be to redefine popen() and system() on
Windows. It looks like we're already using a #define to redefine popen().
Won't defining variant of popen just for Windows to add SYSTEMQUOTE
effect such (where currently it is used for both win and non-winows)
usage? Also, I think we might want to remove use of SYSTEMQUOTE
before popen calls where ever it is currently used to avoid usage of the
same two times.
Well, yeah: the point would be to remove SYSTEMQUOTE altogether,
probably. Certainly the idea I'm suggesting is that we don't need any
Windows-specific notation at the call sites.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/30/2014 07:39 AM, Amit Kapila wrote:
On Wed, Apr 30, 2014 at 3:57 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Heikki Linnakangas <hlinnakangas@vmware.com> writes:
This looks correct to me. popen() requires SYSTEMQUOTEs on Windows, like
system() does.It seems right now SYSTEMQUOTE is used before popen both for
Windows and non-Windows, ex. adjust_data_dir() in pg_ctl.c
Yeah, but it's defined to an empty string on non-Windows platforms.
We might forget to use the wrapper function too, if it has a nonstandard
name, no? A better idea would be to redefine popen() and system() on
Windows. It looks like we're already using a #define to redefine popen().
Right, that's exactly what I meant by a wrapper function.
Won't defining variant of popen just for Windows to add SYSTEMQUOTE
effect such (where currently it is used for both win and non-winows)
usage? Also, I think we might want to remove use of SYSTEMQUOTE
before popen calls where ever it is currently used to avoid usage of the
same two times.
Yep.
I'll write up a patch to do that for git master. For back-branches, I
just added the missing SYSTEMQUOTEs. There are a couple of places where
changing the behavior might break existing installations. In particular,
in the stable branches it's probably best to not add the SYSTEMQUOTEs to
things like archive_command, restore_command, and COPY TO/FROM PROGRAM,
where the command is specified by the user. Because people might already
have added the extra quotes to the command to make it work, and if we
suddenly start adding yet another pair of quotes, it will stop working.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I committed the non-invasive fixes to backbranches (and master too, just
to keep it in sync), but the attached is what I came up with for master.
There are a couple of places in the code where we have #ifdef WIN32 code
that uses CreateProcess with "CMD /C ..." directly. I believe those are
currently (ie. before this patch) wrong for cygwin builds. SYSTEMQUOTE
is defined as:
#if defined(WIN32) && !defined(__CYGWIN__)
#define SYSTEMQUOTE "\""
#else
#define SYSTEMQUOTE ""
#endif
I presume the !CYGWIN part is because cygwin version of system() and
popen() don't require the extra quoting, because cygwin does that for
us. But when we use CreateProcess directly, e.g like this in pg_ctl.c:
snprintf(cmd, MAXPGPATH, "CMD /C " SYSTEMQUOTE "\"\"%s\" %s%s <
\"%s\" 2>&1\"" SYSTEMQUOTE,
exec_path, pgdata_opt, post_opts, DEVNULL);
if (!CreateRestrictedProcess(cmd, &pi, false))
return GetLastError();
we would need the extra quotes, but SYSTEMQUOTE doesn't provide them
with cygwin.
Andrew: you have a cygwin installation, don't you? Could you test if
"pg_ctl start" works when the binaries are installed to a path that
contains both a space and an @ sign, like "C:\white
space\at@sign\install". I suspect it doesn't, but the attached patch
fixes it.
- Heikki
Attachments:
0001-Replace-SYSTEMQUOTEs-with-Windows-specific-wrapper-f.patchtext/x-diff; name=0001-Replace-SYSTEMQUOTEs-with-Windows-specific-wrapper-f.patchDownload
>From b00134385de28361194e7ba0a050343bb581e058 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Wed, 30 Apr 2014 10:23:14 +0300
Subject: [PATCH] Replace SYSTEMQUOTEs with Windows-specific wrapper functions.
It's easy to forget using SYSTEMQUOTEs whe constructing command strings
for system() or popen(). We are currently missing it e.g. in COPY TO/FROM
PROGRAM calls. Even if we fix all the places missing it now, it is bound
to be forgotten again in the future. To eliminate the need for that, add
wrapper functions that do the the extra quoting for you, and get rid of
SYSTEMQUOTEs in all the callers.
diff --git a/configure.in b/configure.in
index fc9c52f..52357a6 100644
--- a/configure.in
+++ b/configure.in
@@ -1353,6 +1353,7 @@ if test "$PORTNAME" = "win32"; then
AC_REPLACE_FUNCS(gettimeofday)
AC_LIBOBJ(kill)
AC_LIBOBJ(open)
+ AC_LIBOBJ(system)
AC_LIBOBJ(win32env)
AC_LIBOBJ(win32error)
AC_LIBOBJ(win32setlocale)
diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
index 56e912d..d22b6d3 100644
--- a/contrib/pg_upgrade/check.c
+++ b/contrib/pg_upgrade/check.c
@@ -970,7 +970,7 @@ get_bin_version(ClusterInfo *cluster)
int pre_dot,
post_dot;
- snprintf(cmd, sizeof(cmd), SYSTEMQUOTE "\"%s/pg_ctl\" --version" SYSTEMQUOTE, cluster->bindir);
+ snprintf(cmd, sizeof(cmd), "\"%s/pg_ctl\" --version", cluster->bindir);
if ((output = popen(cmd, "r")) == NULL ||
fgets(cmd_output, sizeof(cmd_output), output) == NULL)
diff --git a/contrib/pg_upgrade/controldata.c b/contrib/pg_upgrade/controldata.c
index fa0a005..476c6be 100644
--- a/contrib/pg_upgrade/controldata.c
+++ b/contrib/pg_upgrade/controldata.c
@@ -110,7 +110,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
pg_putenv("LC_ALL", NULL);
pg_putenv("LC_MESSAGES", "C");
- snprintf(cmd, sizeof(cmd), SYSTEMQUOTE "\"%s/%s \"%s\"" SYSTEMQUOTE,
+ snprintf(cmd, sizeof(cmd), "\"%s/%s \"%s\"",
cluster->bindir,
live_check ? "pg_controldata\"" : "pg_resetxlog\" -n",
cluster->pgdata);
diff --git a/contrib/pg_upgrade/exec.c b/contrib/pg_upgrade/exec.c
index 7f01301..91e66e6 100644
--- a/contrib/pg_upgrade/exec.c
+++ b/contrib/pg_upgrade/exec.c
@@ -59,14 +59,14 @@ static DWORD mainThreadId = 0;
mainThreadId = GetCurrentThreadId();
#endif
- written = strlcpy(cmd, SYSTEMQUOTE, sizeof(cmd));
+ written = 0;
va_start(ap, fmt);
written += vsnprintf(cmd + written, MAXCMDLEN - written, fmt, ap);
va_end(ap);
if (written >= MAXCMDLEN)
pg_fatal("command too long\n");
written += snprintf(cmd + written, MAXCMDLEN - written,
- " >> \"%s\" 2>&1" SYSTEMQUOTE, log_file);
+ " >> \"%s\" 2>&1", log_file);
if (written >= MAXCMDLEN)
pg_fatal("command too long\n");
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index b53fa8b..83b7f6e 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1130,11 +1130,11 @@ test_config_settings(void)
test_buffs = MIN_BUFS_FOR_CONNS(test_conns);
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" --boot -x0 %s "
+ "\"%s\" --boot -x0 %s "
"-c max_connections=%d "
"-c shared_buffers=%d "
"-c dynamic_shared_memory_type=none "
- "< \"%s\" > \"%s\" 2>&1" SYSTEMQUOTE,
+ "< \"%s\" > \"%s\" 2>&1",
backend_exec, boot_options,
test_conns, test_buffs,
DEVNULL, DEVNULL);
@@ -1165,11 +1165,11 @@ test_config_settings(void)
}
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" --boot -x0 %s "
+ "\"%s\" --boot -x0 %s "
"-c max_connections=%d "
"-c shared_buffers=%d "
"-c dynamic_shared_memory_type=none "
- "< \"%s\" > \"%s\" 2>&1" SYSTEMQUOTE,
+ "< \"%s\" > \"%s\" 2>&1",
backend_exec, boot_options,
n_connections, test_buffs,
DEVNULL, DEVNULL);
@@ -1503,7 +1503,7 @@ bootstrap_template1(void)
unsetenv("PGCLIENTENCODING");
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" --boot -x1 %s %s %s" SYSTEMQUOTE,
+ "\"%s\" --boot -x1 %s %s %s",
backend_exec,
data_checksums ? "-k" : "",
boot_options, talkargs);
@@ -1544,7 +1544,7 @@ setup_auth(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s template1 >%s",
backend_exec, backend_options,
DEVNULL);
@@ -1622,7 +1622,7 @@ get_set_pwd(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s template1 >%s",
backend_exec, backend_options,
DEVNULL);
@@ -1722,7 +1722,7 @@ setup_depend(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s template1 >%s",
backend_exec, backend_options,
DEVNULL);
@@ -1755,7 +1755,7 @@ setup_sysviews(void)
* We use -j here to avoid backslashing stuff in system_views.sql
*/
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s -j template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s -j template1 >%s",
backend_exec, backend_options,
DEVNULL);
@@ -1786,7 +1786,7 @@ setup_description(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s template1 >%s",
backend_exec, backend_options,
DEVNULL);
@@ -1893,7 +1893,7 @@ setup_collation(void)
#if defined(HAVE_LOCALE_T) && !defined(WIN32)
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s template1 >%s",
backend_exec, backend_options,
DEVNULL);
@@ -2038,7 +2038,7 @@ setup_conversion(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s template1 >%s",
backend_exec, backend_options,
DEVNULL);
@@ -2076,7 +2076,7 @@ setup_dictionary(void)
* We use -j here to avoid backslashing stuff
*/
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s -j template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s -j template1 >%s",
backend_exec, backend_options,
DEVNULL);
@@ -2127,7 +2127,7 @@ setup_privileges(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s template1 >%s",
backend_exec, backend_options,
DEVNULL);
@@ -2190,7 +2190,7 @@ setup_schema(void)
* We use -j here to avoid backslashing stuff in information_schema.sql
*/
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s -j template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s -j template1 >%s",
backend_exec, backend_options,
DEVNULL);
@@ -2207,7 +2207,7 @@ setup_schema(void)
PG_CMD_CLOSE;
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s template1 >%s",
backend_exec, backend_options,
DEVNULL);
@@ -2241,7 +2241,7 @@ load_plpgsql(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s template1 >%s",
backend_exec, backend_options,
DEVNULL);
@@ -2266,7 +2266,7 @@ vacuum_db(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s template1 >%s",
backend_exec, backend_options,
DEVNULL);
@@ -2322,7 +2322,7 @@ make_template0(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s template1 >%s",
backend_exec, backend_options,
DEVNULL);
@@ -2354,7 +2354,7 @@ make_postgres(void)
fflush(stdout);
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" %s template1 >%s" SYSTEMQUOTE,
+ "\"%s\" %s template1 >%s",
backend_exec, backend_options,
DEVNULL);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index fc87e7d..473d653 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -435,11 +435,11 @@ start_postmaster(void)
* the PID without having to rely on reading it back from the pidfile.
*/
if (log_file != NULL)
- snprintf(cmd, MAXPGPATH, SYSTEMQUOTE "\"%s\" %s%s < \"%s\" >> \"%s\" 2>&1 &" SYSTEMQUOTE,
+ snprintf(cmd, MAXPGPATH, "\"%s\" %s%s < \"%s\" >> \"%s\" 2>&1 &",
exec_path, pgdata_opt, post_opts,
DEVNULL, log_file);
else
- snprintf(cmd, MAXPGPATH, SYSTEMQUOTE "\"%s\" %s%s < \"%s\" 2>&1 &" SYSTEMQUOTE,
+ snprintf(cmd, MAXPGPATH, "\"%s\" %s%s < \"%s\" 2>&1 &",
exec_path, pgdata_opt, post_opts, DEVNULL);
return system(cmd);
@@ -453,10 +453,10 @@ start_postmaster(void)
PROCESS_INFORMATION pi;
if (log_file != NULL)
- snprintf(cmd, MAXPGPATH, "CMD /C " SYSTEMQUOTE "\"%s\" %s%s < \"%s\" >> \"%s\" 2>&1" SYSTEMQUOTE,
+ snprintf(cmd, MAXPGPATH, "CMD /C \"\"%s\" %s%s < \"%s\" >> \"%s\" 2>&1\"",
exec_path, pgdata_opt, post_opts, DEVNULL, log_file);
else
- snprintf(cmd, MAXPGPATH, "CMD /C " SYSTEMQUOTE "\"%s\" %s%s < \"%s\" 2>&1" SYSTEMQUOTE,
+ snprintf(cmd, MAXPGPATH, "CMD /C \"\"%s\" %s%s < \"%s\" 2>&1\"",
exec_path, pgdata_opt, post_opts, DEVNULL);
if (!CreateRestrictedProcess(cmd, &pi, false))
@@ -814,10 +814,10 @@ do_init(void)
post_opts = "";
if (!silent_mode)
- snprintf(cmd, MAXPGPATH, SYSTEMQUOTE "\"%s\" %s%s" SYSTEMQUOTE,
+ snprintf(cmd, MAXPGPATH, "\"%s\" %s%s",
exec_path, pgdata_opt, post_opts);
else
- snprintf(cmd, MAXPGPATH, SYSTEMQUOTE "\"%s\" %s%s > \"%s\"" SYSTEMQUOTE,
+ snprintf(cmd, MAXPGPATH, "\"%s\" %s%s > \"%s\"",
exec_path, pgdata_opt, post_opts, DEVNULL);
if (system(cmd) != 0)
@@ -2035,7 +2035,7 @@ adjust_data_dir(void)
my_exec_path = pg_strdup(exec_path);
/* it's important for -C to be the first option, see main.c */
- snprintf(cmd, MAXPGPATH, SYSTEMQUOTE "\"%s\" -C data_directory %s%s" SYSTEMQUOTE,
+ snprintf(cmd, MAXPGPATH, "\"%s\" -C data_directory %s%s",
my_exec_path,
pgdata_opt ? pgdata_opt : "",
post_opts ? post_opts : "");
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 47fe6cc..208e49b 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1666,7 +1666,7 @@ runPgDump(const char *dbname)
PQExpBuffer cmd = createPQExpBuffer();
int ret;
- appendPQExpBuffer(cmd, SYSTEMQUOTE "\"%s\" %s", pg_dump_bin,
+ appendPQExpBuffer(cmd, "\"%s\" %s", pg_dump_bin,
pgdumpopts->data);
/*
@@ -1687,8 +1687,6 @@ runPgDump(const char *dbname)
doShellQuoting(cmd, connstrbuf->data);
- appendPQExpBufferStr(cmd, SYSTEMQUOTE);
-
if (verbose)
fprintf(stderr, _("%s: running \"%s\"\n"), progname, cmd->data);
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index fd64ba8..dabcd68 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1936,10 +1936,10 @@ editFile(const char *fname, int lineno)
editorName, fname);
#else
if (lineno > 0)
- sys = psprintf(SYSTEMQUOTE "\"%s\" %s%d \"%s\"" SYSTEMQUOTE,
+ sys = psprintf("\"%s\" %s%d \"%s\"",
editorName, editor_lineno_arg, lineno, fname);
else
- sys = psprintf(SYSTEMQUOTE "\"%s\" \"%s\"" SYSTEMQUOTE,
+ sys = psprintf("\"%s\" \"%s\"",
editorName, fname);
#endif
result = system(sys);
@@ -2643,7 +2643,7 @@ do_shell(const char *command)
#ifndef WIN32
sys = psprintf("exec %s", shellName);
#else
- sys = psprintf(SYSTEMQUOTE "\"%s\"" SYSTEMQUOTE, shellName);
+ sys = psprintf("\"%s\"", shellName);
#endif
result = system(sys);
free(sys);
diff --git a/src/include/port.h b/src/include/port.h
index 0698685..21c8a05 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -115,37 +115,6 @@ extern BOOL AddUserToTokenDacl(HANDLE hToken);
#define DEVNULL "/dev/null"
#endif
-/*
- * Win32 needs double quotes at the beginning and end of system()
- * strings. If not, it gets confused with multiple quoted strings.
- * It also requires double-quotes around the executable name and
- * any files used for redirection. Other args can use single-quotes.
- *
- * Generated using Win32 "CMD /?":
- *
- * 1. If all of the following conditions are met, then quote characters
- * on the command line are preserved:
- *
- * - no /S switch
- * - exactly two quote characters
- * - no special characters between the two quote characters, where special
- * is one of: &<>()@^|
- * - there are one or more whitespace characters between the two quote
- * characters
- * - the string between the two quote characters is the name of an
- * executable file.
- *
- * 2. Otherwise, old behavior is to see if the first character is a quote
- * character and if so, strip the leading character and remove the last
- * quote character on the command line, preserving any text after the last
- * quote character.
- */
-#if defined(WIN32) && !defined(__CYGWIN__)
-#define SYSTEMQUOTE "\""
-#else
-#define SYSTEMQUOTE ""
-#endif
-
/* Portable delay handling */
extern void pg_usleep(long microsec);
@@ -332,12 +301,16 @@ extern FILE *pgwin32_fopen(const char *, const char *);
#define fopen(a,b) pgwin32_fopen(a,b)
#endif
-#ifndef popen
-#define popen(a,b) _popen(a,b)
-#endif
-#ifndef pclose
+/*
+ * system() and popen() replacements to enclose the command in an extra
+ * pair of quotes.
+ */
+extern int pgwin32_system(const char *command);
+extern FILE *pgwin32_popen(const char *command, const char *type);
+
+#define system(a) pgwin32_system(a)
+#define popen(a,b) pgwin32_popen(a,b)
#define pclose(a) _pclose(a)
-#endif
/* New versions of MingW have gettimeofday, old mingw and msvc don't */
#ifndef HAVE_GETTIMEOFDAY
diff --git a/src/interfaces/ecpg/test/pg_regress_ecpg.c b/src/interfaces/ecpg/test/pg_regress_ecpg.c
index cb79b61..e9bedb5 100644
--- a/src/interfaces/ecpg/test/pg_regress_ecpg.c
+++ b/src/interfaces/ecpg/test/pg_regress_ecpg.c
@@ -137,7 +137,7 @@ ecpg_start_test(const char *testname,
snprintf(inprg, sizeof(inprg), "%s/%s", inputdir, testname);
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "\"%s\" >\"%s\" 2>\"%s\"" SYSTEMQUOTE,
+ "\"%s\" >\"%s\" 2>\"%s\"",
inprg,
outfile_stdout,
outfile_stderr);
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 7f2d901..2b770d0 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -38,7 +38,7 @@ OBJS= fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o \
OBJS += chklocale.o inet_net_ntop.o noblock.o pgstrcasecmp.o pqsignal.o \
thread.o
# libpgport C files that are needed if identified by configure
-OBJS += $(filter crypt.o getaddrinfo.o getpeereid.o inet_aton.o open.o snprintf.o strerror.o strlcpy.o win32error.o win32setlocale.o, $(LIBOBJS))
+OBJS += $(filter crypt.o getaddrinfo.o getpeereid.o inet_aton.o open.o system.o snprintf.o strerror.o strlcpy.o win32error.o win32setlocale.o, $(LIBOBJS))
# backend/libpq
OBJS += ip.o md5.o
# utils/mb
@@ -89,7 +89,7 @@ backend_src = $(top_srcdir)/src/backend
# For some libpgport modules, this only happens if configure decides
# the module is needed (see filter hack in OBJS, above).
-chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c inet_net_ntop.c noblock.c open.c pgsleep.c pgstrcasecmp.c pqsignal.c snprintf.c strerror.c strlcpy.c thread.c win32error.c win32setlocale.c: % : $(top_srcdir)/src/port/%
+chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c inet_net_ntop.c noblock.c open.c system.c pgsleep.c pgstrcasecmp.c pqsignal.c snprintf.c strerror.c strlcpy.c thread.c win32error.c win32setlocale.c: % : $(top_srcdir)/src/port/%
rm -f $@ && $(LN_S) $< .
ip.c md5.c: % : $(backend_src)/libpq/%
@@ -150,7 +150,7 @@ clean distclean: clean-lib
# Might be left over from a Win32 client-only build
rm -f pg_config_paths.h
rm -f inet_net_ntop.c noblock.c pgstrcasecmp.c pqsignal.c thread.c
- rm -f chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c open.c snprintf.c strerror.c strlcpy.c win32error.c win32setlocale.c
+ rm -f chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c open.c system.c snprintf.c strerror.c strlcpy.c win32error.c win32setlocale.c
rm -f pgsleep.c
rm -f md5.c ip.c
rm -f encnames.c wchar.c
diff --git a/src/interfaces/libpq/bcc32.mak b/src/interfaces/libpq/bcc32.mak
index 8f5cd8d..78102fa 100644
--- a/src/interfaces/libpq/bcc32.mak
+++ b/src/interfaces/libpq/bcc32.mak
@@ -106,6 +106,7 @@ CLEAN :
-@erase "$(INTDIR)\dirmod.obj"
-@erase "$(INTDIR)\pgsleep.obj"
-@erase "$(INTDIR)\open.obj"
+ -@erase "$(INTDIR)\system.obj"
-@erase "$(INTDIR)\win32error.obj"
-@erase "$(OUTDIR)\$(OUTFILENAME).lib"
-@erase "$(OUTDIR)\$(OUTFILENAME)dll.lib"
@@ -149,6 +150,7 @@ LIB32_OBJS= \
"$(INTDIR)\dirmod.obj" \
"$(INTDIR)\pgsleep.obj" \
"$(INTDIR)\open.obj" \
+ "$(INTDIR)\system.obj" \
"$(INTDIR)\win32error.obj" \
"$(INTDIR)\pthread-win32.obj"
@@ -295,6 +297,11 @@ LINK32_FLAGS = -Gn -L$(BCB)\lib;$(INTDIR); -x -Tpd -v
$(CPP_PROJ) /I"." ..\..\port\open.c
<<
+"$(INTDIR)\system.obj" : ..\..\port\system.c
+ $(CPP) @<<
+ $(CPP_PROJ) /I"." ..\..\port\system.c
+<<
+
"$(INTDIR)\win32error.obj" : ..\..\port\win32error.c
$(CPP) @<<
$(CPP_PROJ) /I"." ..\..\port\win32error.c
diff --git a/src/interfaces/libpq/win32.mak b/src/interfaces/libpq/win32.mak
index ee1884f..23e09e9 100644
--- a/src/interfaces/libpq/win32.mak
+++ b/src/interfaces/libpq/win32.mak
@@ -113,6 +113,7 @@ CLEAN :
-@erase "$(INTDIR)\dirmod.obj"
-@erase "$(INTDIR)\pgsleep.obj"
-@erase "$(INTDIR)\open.obj"
+ -@erase "$(INTDIR)\system.obj"
-@erase "$(INTDIR)\win32error.obj"
-@erase "$(INTDIR)\win32setlocale.obj"
-@erase "$(OUTDIR)\$(OUTFILENAME).lib"
@@ -159,6 +160,7 @@ LIB32_OBJS= \
"$(INTDIR)\dirmod.obj" \
"$(INTDIR)\pgsleep.obj" \
"$(INTDIR)\open.obj" \
+ "$(INTDIR)\system.obj" \
"$(INTDIR)\win32error.obj" \
"$(INTDIR)\win32setlocale.obj" \
"$(INTDIR)\pthread-win32.obj"
@@ -335,6 +337,11 @@ LINK32_OBJS= \
$(CPP_PROJ) /I"." ..\..\port\open.c
<<
+"$(INTDIR)\system.obj" : ..\..\port\system.c
+ $(CPP) @<<
+ $(CPP_PROJ) /I"." ..\..\port\system.c
+<<
+
"$(INTDIR)\win32error.obj" : ..\..\port\win32error.c
$(CPP) @<<
$(CPP_PROJ) /I"." ..\..\port\win32error.c
diff --git a/src/port/system.c b/src/port/system.c
new file mode 100644
index 0000000..702ff8c
--- /dev/null
+++ b/src/port/system.c
@@ -0,0 +1,92 @@
+/*-------------------------------------------------------------------------
+ *
+ * system.c
+ * Win32 system() and popen() replacements
+ *
+ *
+ * Win32 needs double quotes at the beginning and end of system()
+ * strings. If not, it gets confused with multiple quoted strings.
+ * It also requires double-quotes around the executable name and
+ * any files used for redirection. Other args can use single-quotes.
+ *
+ * Generated using Win32 "CMD /?":
+ *
+ * 1. If all of the following conditions are met, then quote characters
+ * on the command line are preserved:
+ *
+ * - no /S switch
+ * - exactly two quote characters
+ * - no special characters between the two quote characters, where special
+ * is one of: &<>()@^|
+ * - there are one or more whitespace characters between the two quote
+ * characters
+ * - the string between the two quote characters is the name of an
+ * executable file.
+ *
+ * 2. Otherwise, old behavior is to see if the first character is a quote
+ * character and if so, strip the leading character and remove the last
+ * quote character on the command line, preserving any text after the last
+ * quote character.
+ *
+ * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group
+ *
+ * src/port/system.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#if defined(WIN32) && !defined(__CYGWIN__)
+
+#ifndef FRONTEND
+#include "postgres.h"
+#else
+#include "postgres_fe.h"
+#endif
+
+#include <windows.h>
+#include <fcntl.h>
+#include <assert.h>
+
+#undef system
+#undef popen
+
+int
+pgwin32_system(const char *command)
+{
+ size_t cmdlen = strlen(command);
+ char *buf;
+ int res;
+
+ buf = malloc(cmdlen + 2 + 1);
+ buf[0] = '"';
+ memcpy(&buf[1], command, cmdlen);
+ buf[cmdlen + 1] = '"';
+ buf[cmdlen + 2] = '\0';
+
+ res = system(buf);
+ free(buf);
+
+ return res;
+}
+
+
+FILE *
+pgwin32_popen(const char *command, const char *type)
+{
+ size_t cmdlen = strlen(command);
+ char *buf;
+ FILE *res;
+
+ buf = malloc(cmdlen + 2 + 1);
+ buf[0] = '"';
+ memcpy(&buf[1], command, cmdlen);
+ buf[cmdlen + 1] = '"';
+ buf[cmdlen + 2] = '\0';
+
+ res = _popen(buf, type);
+ free(buf);
+
+ return res;
+}
+
+#endif
diff --git a/src/test/isolation/isolation_main.c b/src/test/isolation/isolation_main.c
index 64c4175..c8d431f 100644
--- a/src/test/isolation/isolation_main.c
+++ b/src/test/isolation/isolation_main.c
@@ -77,7 +77,7 @@ isolation_start_test(const char *testname,
"%s ", launcher);
snprintf(psql_cmd + offset, sizeof(psql_cmd) - offset,
- SYSTEMQUOTE "\"%s\" \"dbname=%s\" < \"%s\" > \"%s\" 2>&1" SYSTEMQUOTE,
+ "\"%s\" \"dbname=%s\" < \"%s\" > \"%s\" 2>&1",
isolation_exec,
dblist->str,
infile,
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 07dd803..c41cf7e 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -293,7 +293,7 @@ stop_postmaster(void)
fflush(stderr);
snprintf(buf, sizeof(buf),
- SYSTEMQUOTE "\"%s/pg_ctl\" stop -D \"%s/data\" -s -m fast" SYSTEMQUOTE,
+ "\"%s/pg_ctl\" stop -D \"%s/data\" -s -m fast",
bindir, temp_install);
r = system(buf);
if (r != 0)
@@ -904,7 +904,7 @@ psql_command(const char *database, const char *query,...)
/* And now we can build and execute the shell command */
snprintf(psql_cmd, sizeof(psql_cmd),
- SYSTEMQUOTE "\"%s%spsql\" -X -c \"%s\" \"%s\"" SYSTEMQUOTE,
+ "\"%s%spsql\" -X -c \"%s\" \"%s\"",
psqldir ? psqldir : "",
psqldir ? "/" : "",
query_escaped,
@@ -1033,7 +1033,7 @@ spawn_process(const char *cmdline)
exit(2);
}
- cmdline2 = psprintf("cmd /c %s", cmdline);
+ cmdline2 = psprintf("cmd /c \"%s\"", cmdline);
#ifndef __CYGWIN__
AddUserToTokenDacl(restrictedToken);
@@ -1251,7 +1251,7 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul
/* OK, run the diff */
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "diff %s \"%s\" \"%s\" > \"%s\"" SYSTEMQUOTE,
+ "diff %s \"%s\" \"%s\" > \"%s\"",
basic_diff_opts, expectfile, resultsfile, diff);
/* Is the diff file empty? */
@@ -1284,7 +1284,7 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul
}
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "diff %s \"%s\" \"%s\" > \"%s\"" SYSTEMQUOTE,
+ "diff %s \"%s\" \"%s\" > \"%s\"",
basic_diff_opts, alt_expectfile, resultsfile, diff);
if (run_diff(cmd, diff) == 0)
@@ -1312,7 +1312,7 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul
if (platform_expectfile)
{
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "diff %s \"%s\" \"%s\" > \"%s\"" SYSTEMQUOTE,
+ "diff %s \"%s\" \"%s\" > \"%s\"",
basic_diff_opts, default_expectfile, resultsfile, diff);
if (run_diff(cmd, diff) == 0)
@@ -1336,7 +1336,7 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul
* append to the diffs summary file.
*/
snprintf(cmd, sizeof(cmd),
- SYSTEMQUOTE "diff %s \"%s\" \"%s\" >> \"%s\"" SYSTEMQUOTE,
+ "diff %s \"%s\" \"%s\" >> \"%s\"",
pretty_diff_opts, best_expect_file, resultsfile, difffilename);
run_diff(cmd, difffilename);
@@ -2121,11 +2121,11 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
/* "make install" */
#ifndef WIN32_ONLY_COMPILER
snprintf(buf, sizeof(buf),
- SYSTEMQUOTE "\"%s\" -C \"%s\" DESTDIR=\"%s/install\" install > \"%s/log/install.log\" 2>&1" SYSTEMQUOTE,
+ "\"%s\" -C \"%s\" DESTDIR=\"%s/install\" install > \"%s/log/install.log\" 2>&1",
makeprog, top_builddir, temp_install, outputdir);
#else
snprintf(buf, sizeof(buf),
- SYSTEMQUOTE "perl \"%s/src/tools/msvc/install.pl\" \"%s/install\" >\"%s/log/install.log\" 2>&1" SYSTEMQUOTE,
+ "perl \"%s/src/tools/msvc/install.pl\" \"%s/install\" >\"%s/log/install.log\" 2>&1",
top_builddir, temp_install, outputdir);
#endif
if (system(buf))
@@ -2138,7 +2138,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
{
#ifndef WIN32_ONLY_COMPILER
snprintf(buf, sizeof(buf),
- SYSTEMQUOTE "\"%s\" -C \"%s/%s\" DESTDIR=\"%s/install\" install >> \"%s/log/install.log\" 2>&1" SYSTEMQUOTE,
+ "\"%s\" -C \"%s/%s\" DESTDIR=\"%s/install\" install >> \"%s/log/install.log\" 2>&1",
makeprog, top_builddir, sl->str, temp_install, outputdir);
#else
fprintf(stderr, _("\n%s: --extra-install option not supported on this platform\n"), progname);
@@ -2155,7 +2155,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
/* initdb */
header(_("initializing database system"));
snprintf(buf, sizeof(buf),
- SYSTEMQUOTE "\"%s/initdb\" -D \"%s/data\" -L \"%s\" --noclean --nosync%s%s > \"%s/log/initdb.log\" 2>&1" SYSTEMQUOTE,
+ "\"%s/initdb\" -D \"%s/data\" -L \"%s\" --noclean --nosync%s%s > \"%s/log/initdb.log\" 2>&1",
bindir, temp_install, datadir,
debug ? " --debug" : "",
nolocale ? " --no-locale" : "",
@@ -2206,7 +2206,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
* Check if there is a postmaster running already.
*/
snprintf(buf2, sizeof(buf2),
- SYSTEMQUOTE "\"%s/psql\" -X postgres <%s 2>%s" SYSTEMQUOTE,
+ "\"%s/psql\" -X postgres <%s 2>%s",
bindir, DEVNULL, DEVNULL);
for (i = 0; i < 16; i++)
@@ -2238,7 +2238,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
*/
header(_("starting postmaster"));
snprintf(buf, sizeof(buf),
- SYSTEMQUOTE "\"%s/postgres\" -D \"%s/data\" -F%s -c \"listen_addresses=%s\" > \"%s/log/postmaster.log\" 2>&1" SYSTEMQUOTE,
+ "\"%s/postgres\" -D \"%s/data\" -F%s -c \"listen_addresses=%s\" > \"%s/log/postmaster.log\" 2>&1",
bindir, temp_install,
debug ? " -d 5" : "",
hostname ? hostname : "",
diff --git a/src/test/regress/pg_regress_main.c b/src/test/regress/pg_regress_main.c
index a4f66b8..90327b0 100644
--- a/src/test/regress/pg_regress_main.c
+++ b/src/test/regress/pg_regress_main.c
@@ -64,7 +64,7 @@ psql_start_test(const char *testname,
"%s ", launcher);
snprintf(psql_cmd + offset, sizeof(psql_cmd) - offset,
- SYSTEMQUOTE "\"%s%spsql\" -X -a -q -d \"%s\" < \"%s\" > \"%s\" 2>&1" SYSTEMQUOTE,
+ "\"%s%spsql\" -X -a -q -d \"%s\" < \"%s\" > \"%s\" 2>&1",
psqldir ? psqldir : "",
psqldir ? "/" : "",
dblist->str,
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index d06d6ad..1254d89 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -69,7 +69,7 @@ sub mkvcbuild
srandom.c getaddrinfo.c gettimeofday.c inet_net_ntop.c kill.c open.c
erand48.c snprintf.c strlcat.c strlcpy.c dirmod.c noblock.c path.c
pgcheckdir.c pg_crc.c pgmkdirp.c pgsleep.c pgstrcasecmp.c pqsignal.c
- qsort.c qsort_arg.c quotes.c
+ qsort.c qsort_arg.c quotes.c system.c
sprompt.c tar.c thread.c getopt.c getopt_long.c dirent.c
win32env.c win32error.c win32setlocale.c);
--
1.9.2
On 04/30/2014 06:31 AM, Heikki Linnakangas wrote:
Andrew: you have a cygwin installation, don't you? Could you test if
"pg_ctl start" works when the binaries are installed to a path that
contains both a space and an @ sign, like "C:\white
space\at@sign\install". I suspect it doesn't, but the attached patch
fixes it.
I'll see what I can do.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Heikki Linnakangas <hlinnakangas@vmware.com> writes:
I committed the non-invasive fixes to backbranches (and master too, just
to keep it in sync), but the attached is what I came up with for master.
The malloc's in the new system.c file should be pg_malloc, or else have
custom defenses against out-of-memory (possibly returning ENOMEM to
the caller would be best?). Also, it seems like a good idea to save and
restore errno across the ending free() calls. I don't know if Windows'
version of free() can change errno, but we've definitely found that to
be possible on other platforms.
Looks good otherwise.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/30/2014 11:58 AM, Andrew Dunstan wrote:
On 04/30/2014 06:31 AM, Heikki Linnakangas wrote:
Andrew: you have a cygwin installation, don't you? Could you test if
"pg_ctl start" works when the binaries are installed to a path that
contains both a space and an @ sign, like "C:\white
space\at@sign\install". I suspect it doesn't, but the attached patch
fixes it.I'll see what I can do.
I tried git master like this:
foo\ bar/a\@b/bin/initdb.exe data
foo\ bar/a\@b/bin/pg_ctl.exe -D data/ -w start
and didn't encounter a problem.
Platform is Windows 8 Pro 64 bit, with latest 32 bit Cygwin.
[ ... ] It looks like possibly the only time this will actually matter
on Cygwin is when starting a service. Just running "pg_ctl start" from
the command line works fine. But starting the service doesn't.
I'll try the patch when I get a chance.
cheers
andrew
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/30/2014 03:03 PM, Andrew Dunstan wrote:
On 04/30/2014 11:58 AM, Andrew Dunstan wrote:
On 04/30/2014 06:31 AM, Heikki Linnakangas wrote:
Andrew: you have a cygwin installation, don't you? Could you test if
"pg_ctl start" works when the binaries are installed to a path that
contains both a space and an @ sign, like "C:\white
space\at@sign\install". I suspect it doesn't, but the attached patch
fixes it.I'll see what I can do.
I tried git master like this:
foo\ bar/a\@b/bin/initdb.exe data
foo\ bar/a\@b/bin/pg_ctl.exe -D data/ -w startand didn't encounter a problem.
Platform is Windows 8 Pro 64 bit, with latest 32 bit Cygwin.
[ ... ] It looks like possibly the only time this will actually matter
on Cygwin is when starting a service. Just running "pg_ctl start"
from the command line works fine. But starting the service doesn't.I'll try the patch when I get a chance.
No, there is something horribly wrong with the Cygwin service code. I
don't have time now to sort it out. I suspect it's been broken for a
very long time. I'm not even sure why we have it. Cygwin contains a
wrapper (cygrunsrv) for setting up cygwin executables as services, so
this is probably worse than redundant.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Apr 30, 2014 at 4:01 PM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:
I committed the non-invasive fixes to backbranches (and master too, just to
keep it in sync), but the attached is what I came up with for master.There are a couple of places in the code where we have #ifdef WIN32 code
that uses CreateProcess with "CMD /C ..." directly.
1. Do we need similar handling for CreatePipe case where it directly uses
executable path such as in function pipe_read_line()?
Currently the caller of pipe_read_line() calls canonicalize_path() to change
win32 specific path, is that sufficient or do we need SYSTEMQUOTE type
of handling for it.
2.
system.c
#include <assert.h>
Do we really need inclusion of assert.h or this is for future use?
3. One more observation is that currently install.bat doesn't work
for such paths:
install.bat "e:\PostgreSQL\master\install 1\ins@1"
1\ins@1""=="" was unexpected at this time.
4. Similar to Andrew, I also could not reproduce this problem on my
Windows system (Windows 7 64 bit)
e:\>"e:\PostgreSQL\master\install 1\ins@1\bin\initdb.exe" -D "e:
\PostgreSQL\master\Data 1"
e:\>"e:\PostgreSQL\master\install 1\ins@1\bin\pg_ctl.exe" start -D "e:
\PostgreSQL\master\Data 1"
Above commands work fine.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 05/01/2014 07:55 AM, Amit Kapila wrote:
On Wed, Apr 30, 2014 at 4:01 PM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:I committed the non-invasive fixes to backbranches (and master too, just to
keep it in sync), but the attached is what I came up with for master.There are a couple of places in the code where we have #ifdef WIN32 code
that uses CreateProcess with "CMD /C ..." directly.1. Do we need similar handling for CreatePipe case where it directly uses
executable path such as in function pipe_read_line()?
Currently the caller of pipe_read_line() calls canonicalize_path() to change
win32 specific path, is that sufficient or do we need SYSTEMQUOTE type
of handling for it.
No, SYSTEMQUOTE style handling is not required with CreateProcess.
find_other_exec, which is the only caller of pipe_read_line, adds one
pair of double-quotes around the executable's path, which is enough for
CreateProcess.
2.
system.c
#include <assert.h>
Do we really need inclusion of assert.h or this is for future use?
You're right, that's not needed.
3. One more observation is that currently install.bat doesn't work
for such paths:
install.bat "e:\PostgreSQL\master\install 1\ins@1"
1\ins@1""=="" was unexpected at this time.
Yeah, I noticed that. I haven't looked into what it would take to fix
that, but for now you can just install to a path that doesn't contain
whitespace, and move it from there. install.bat is only used by
developers / packagers, so that's not a big deal.
4. Similar to Andrew, I also could not reproduce this problem on my
Windows system (Windows 7 64 bit)
e:\>"e:\PostgreSQL\master\install 1\ins@1\bin\initdb.exe" -D "e:
\PostgreSQL\master\Data 1"
e:\>"e:\PostgreSQL\master\install 1\ins@1\bin\pg_ctl.exe" start -D "e:
\PostgreSQL\master\Data 1"Above commands work fine.
Hmm, I'm also testing on 64-bit Windows 7, and it failed for me. Note
that I already committed the narrow fix for initdb - which I also
backpatched - to master, so to reproduce you'll need to revert that
locally (commit 503de546).
I fixed the issues with malloc that Tom pointed out and committed the
wrapper functions to git master. But please let me know if there's still
a problem with it.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, May 5, 2014 at 6:38 PM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:
On 05/01/2014 07:55 AM, Amit Kapila wrote:
4. Similar to Andrew, I also could not reproduce this problem on my
Windows system (Windows 7 64 bit)
e:\>"e:\PostgreSQL\master\install 1\ins@1\bin\initdb.exe" -D "e:
\PostgreSQL\master\Data 1"
e:\>"e:\PostgreSQL\master\install 1\ins@1\bin\pg_ctl.exe" start -D "e:
\PostgreSQL\master\Data 1"Above commands work fine.
Hmm, I'm also testing on 64-bit Windows 7, and it failed for me. Note that I
already committed the narrow fix for initdb - which I also backpatched - to
master, so to reproduce you'll need to revert that locally (commit
503de546).
After reverting the specified commit, I could reproduce the issue and
verified HEAD(it is fixed).
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers