pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.

Started by Heikki Linnakangasabout 12 years ago7 messagescomitters
Jump to latest
#1Heikki Linnakangas
heikki.linnakangas@enterprisedb.com

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(-)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#2Andres Freund
andres@anarazel.de
In reply to: Heikki Linnakangas (#1)
Re: pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#2)
Re: pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#4Bruce Momjian
bruce@momjian.us
In reply to: Heikki Linnakangas (#1)
Re: pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.

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. +

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#5Michael Paquier
michael@paquier.xyz
In reply to: Bruce Momjian (#4)
Re: pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#6Bruce Momjian
bruce@momjian.us
In reply to: Michael Paquier (#5)
Re: pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.

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. +

Attachments:

quote.difftext/x-diff; charset=us-asciiDownload+15-0
#7Michael Paquier
michael@paquier.xyz
In reply to: Bruce Momjian (#6)
Re: pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers