Postgres Pro
Downloads
Home   >   mailing lists

Re: Requesting external_pid_file with postgres -C when not initialized lead to coredump - Mailing list pgsql-hackers

From alain radix
Subject Re: Requesting external_pid_file with postgres -C when not initialized lead to coredump
Date
Msg-id CA+YdpwyMhULqKJi=RfqiThRnubNUVcLFdvMwtQhWxvuvdDpS-A@mail.gmail.com
Whole thread Raw
In response to Re: Requesting external_pid_file with postgres -C when not initialized lead to coredump  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Requesting external_pid_file with postgres -C when not initialized lead to coredump  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Tree view
Hi Tom,

The actual fix do the job for preventing coredump.
Effectively, reporting "" instead of (null) would be more consistent with the values found in pg_settings.

Another effect of the patch is that it become possible to start a cluster with external_pid_file='' in postgresql.conf
It would have that same behavior as many other parameters.

I started a 9.6 beta with the following lines in postgresql.conf and the only one that reported a problem is external_pid_file

external_pid_file = ''                  # write an extra PID file
unix_socket_group = ''                  # (change requires restart)
bonjour_name = ''                       # defaults to the computer name
ssl_ca_file = ''                        # (change requires restart)
ssl_crl_file = ''                       # (change requires restart)
krb_server_keyfile = ''
shared_preload_libraries = ''           # (change requires restart)
synchronous_standby_names = ''  # standby servers that provide sync rep
cluster_name = ''                       # added to process titles if nonempty
default_tablespace = ''         # a tablespace name, '' uses the default
temp_tablespaces = ''                   # a list of tablespace names, '' uses
local_preload_libraries = ''
session_preload_libraries = ''

It seems to me that the general behavior it to consider an empty string as an unset parameter in postgresql.conf, why should external_pid_file be an exception ?

I would prefer postgresql -C to report an empty string to be consistent with pg_settings in a backport of the fix.

I also would prefer external_pid_file to behave as other parameters in the next versions.

Regards.

Alain


On 22 June 2016 at 17:24, Tom Lane <tgl@sss.pgh.pa.us> wrote:
alain radix <alain.radix@gmail.com> writes:
> Here is a new patch with postmaster.c modification so that it check about a
> empty string instead of a null pointer.

Why should we adopt this, when there is already a better (more complete)
fix in git?

I'm not sold on the wisdom of modifying the semantics of
external_pid_file, which is what this patch effectively does.  I don't
know if anything outside our core code looks at that variable, but if
anything does, this would create issues for it.

I'm even less sold on the wisdom of changing every other GUC that
has a null bootstrap default, which is what we would also have to do
in order to avoid the identical misbehavior for other -C cases.

> The meaning is no more to avoid a core dump as you've done a change for
> that but to have the same result with postgres -C as with a request to
> pg_settings, an empty string when the parameter is not set.

Well, maybe we should make -C print an empty string not "(nil)" for
such cases.  You're right that we don't make a distinction between
NULL and "" in other code paths that display a string GUC, so doing
so here is a bit inconsistent.

                        regards, tom lane



--
Alain Radix

pgsql-hackers by date:

Previous
From: Hafez Kamal
Date:
Subject: [HITB-Announce] HITB2016AMS Videos & GSEC Singapore Voting
Next
From: Bruce Momjian
Date:
Subject: Re: Question and suggestion about application binary compatibility policy