Обсуждение: pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.
Replace SYSTEMQUOTEs with Windows-specific wrapper functions. It's easy to forget using SYSTEMQUOTEs when constructing command strings for system() or popen(). Even if we fix all the places missing it now, it is bound to be forgotten again in the future. Introduce wrapper functions that do the the extra quoting for you, and get rid of SYSTEMQUOTEs in all the callers. We previosly used SYSTEMQUOTEs in all the hard-coded command strings, and this doesn't change the behavior of those. But user-supplied commands, like archive_command, restore_command, COPY TO/FROM PROGRAM calls, as well as pgbench's \shell, will now gain an extra pair of quotes. That is desirable, but if you have existing scripts or config files that include an extra pair of quotes, those might need to be adjusted. Reviewed by Amit Kapila and Tom Lane Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/a692ee5870f0f442565b4c4bff367094599e9bdf Modified Files -------------- configure.in | 1 + contrib/pg_upgrade/check.c | 2 +- contrib/pg_upgrade/controldata.c | 2 +- contrib/pg_upgrade/exec.c | 4 +- src/bin/initdb/initdb.c | 40 +++++----- src/bin/pg_ctl/pg_ctl.c | 14 ++-- src/bin/pg_dump/pg_dumpall.c | 4 +- src/bin/psql/command.c | 6 +- src/include/port.h | 45 +++-------- src/interfaces/ecpg/test/pg_regress_ecpg.c | 2 +- src/interfaces/libpq/Makefile | 6 +- src/interfaces/libpq/bcc32.mak | 7 ++ src/interfaces/libpq/win32.mak | 7 ++ src/port/system.c | 117 ++++++++++++++++++++++++++++ src/test/isolation/isolation_main.c | 2 +- src/test/regress/pg_regress.c | 26 +++---- src/test/regress/pg_regress_main.c | 2 +- src/tools/msvc/Mkvcbuild.pm | 2 +- 18 files changed, 196 insertions(+), 93 deletions(-)
Hi Heikki, On 2014-05-05 13:08:32 +0000, Heikki Linnakangas wrote: > Replace SYSTEMQUOTEs with Windows-specific wrapper functions. You noticed that the non MSVC windows animals don't like this patch? http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2014-05-05%2015%3A01%3A24 http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=frogmouth&dt=2014-05-05%2018%3A30%3A01 Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > On 2014-05-05 13:08:32 +0000, Heikki Linnakangas wrote: >> Replace SYSTEMQUOTEs with Windows-specific wrapper functions. > You noticed that the non MSVC windows animals don't like this patch? I think it might be just this: http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=9252b8eec27bbefbeae9d60d8cd4f6b8be80b861 regards, tom lane
On Mon, May 5, 2014 at 01:08:32PM +0000, Heikki Linnakangas wrote: > Replace SYSTEMQUOTEs with Windows-specific wrapper functions. > > It's easy to forget using SYSTEMQUOTEs when constructing command strings > for system() or popen(). Even if we fix all the places missing it now, it is > bound to be forgotten again in the future. Introduce wrapper functions that > do the the extra quoting for you, and get rid of SYSTEMQUOTEs in all the > callers. > > We previosly used SYSTEMQUOTEs in all the hard-coded command strings, and > this doesn't change the behavior of those. But user-supplied commands, like > archive_command, restore_command, COPY TO/FROM PROGRAM calls, as well as > pgbench's \shell, will now gain an extra pair of quotes. That is desirable, > but if you have existing scripts or config files that include an extra > pair of quotes, those might need to be adjusted. Should this be mentioned as an incompatibility in the 9.4 release notes? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
On Tue, May 13, 2014 at 8:18 AM, Bruce Momjian <bruce@momjian.us> wrote: > On Mon, May 5, 2014 at 01:08:32PM +0000, Heikki Linnakangas wrote: >> Replace SYSTEMQUOTEs with Windows-specific wrapper functions. >> >> It's easy to forget using SYSTEMQUOTEs when constructing command strings >> for system() or popen(). Even if we fix all the places missing it now, it is >> bound to be forgotten again in the future. Introduce wrapper functions that >> do the the extra quoting for you, and get rid of SYSTEMQUOTEs in all the >> callers. >> >> We previosly used SYSTEMQUOTEs in all the hard-coded command strings, and >> this doesn't change the behavior of those. But user-supplied commands, like >> archive_command, restore_command, COPY TO/FROM PROGRAM calls, as well as >> pgbench's \shell, will now gain an extra pair of quotes. That is desirable, >> but if you have existing scripts or config files that include an extra >> pair of quotes, those might need to be adjusted. > > Should this be mentioned as an incompatibility in the 9.4 release notes? Definitely worth mentioning. -- Michael
On Tue, May 13, 2014 at 08:19:22AM +0900, Michael Paquier wrote: > On Tue, May 13, 2014 at 8:18 AM, Bruce Momjian <bruce@momjian.us> wrote: > > On Mon, May 5, 2014 at 01:08:32PM +0000, Heikki Linnakangas wrote: > >> Replace SYSTEMQUOTEs with Windows-specific wrapper functions. > >> > >> It's easy to forget using SYSTEMQUOTEs when constructing command strings > >> for system() or popen(). Even if we fix all the places missing it now, it is > >> bound to be forgotten again in the future. Introduce wrapper functions that > >> do the the extra quoting for you, and get rid of SYSTEMQUOTEs in all the > >> callers. > >> > >> We previosly used SYSTEMQUOTEs in all the hard-coded command strings, and > >> this doesn't change the behavior of those. But user-supplied commands, like > >> archive_command, restore_command, COPY TO/FROM PROGRAM calls, as well as > >> pgbench's \shell, will now gain an extra pair of quotes. That is desirable, > >> but if you have existing scripts or config files that include an extra > >> pair of quotes, those might need to be adjusted. > > > > Should this be mentioned as an incompatibility in the 9.4 release notes? > Definitely worth mentioning. Done with the attached applied patch. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Вложения
On Thu, May 15, 2014 at 12:02 AM, Bruce Momjian <bruce@momjian.us> wrote: > On Tue, May 13, 2014 at 08:19:22AM +0900, Michael Paquier wrote: >> On Tue, May 13, 2014 at 8:18 AM, Bruce Momjian <bruce@momjian.us> wrote: >> > On Mon, May 5, 2014 at 01:08:32PM +0000, Heikki Linnakangas wrote: >> >> Replace SYSTEMQUOTEs with Windows-specific wrapper functions. >> >> >> >> It's easy to forget using SYSTEMQUOTEs when constructing command strings >> >> for system() or popen(). Even if we fix all the places missing it now, it is >> >> bound to be forgotten again in the future. Introduce wrapper functions that >> >> do the the extra quoting for you, and get rid of SYSTEMQUOTEs in all the >> >> callers. >> >> >> >> We previosly used SYSTEMQUOTEs in all the hard-coded command strings, and >> >> this doesn't change the behavior of those. But user-supplied commands, like >> >> archive_command, restore_command, COPY TO/FROM PROGRAM calls, as well as >> >> pgbench's \shell, will now gain an extra pair of quotes. That is desirable, >> >> but if you have existing scripts or config files that include an extra >> >> pair of quotes, those might need to be adjusted. >> > >> > Should this be mentioned as an incompatibility in the 9.4 release notes? >> Definitely worth mentioning. > > Done with the attached applied patch. Thanks! -- Michael