Thread: pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.

pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.

From
Heikki Linnakangas
Date:
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(-)


Re: pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.

From
Andres Freund
Date:
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


Re: pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.

From
Bruce Momjian
Date:
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. +


Re: pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.

From
Michael Paquier
Date:
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


Re: pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.

From
Bruce Momjian
Date:
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. +

Attachment

Re: pgsql: Replace SYSTEMQUOTEs with Windows-specific wrapper functions.

From
Michael Paquier
Date:
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