Re: Segmentation fault with postgres -C external_pid_file

On Thu, Jun 16, 2016 at 3:40 AM, Jehan-Guillaume de Rorthais
<jgdr@dalibo.com> wrote:
> I just found that the following command always produce a segmentation fault
> when external_pid_file is kept commented:
>
>   $ postgres -C external_pid_file
>   Segmentation fault (core dumped)

Confirmed. puts() would print (null) on OSX even if that's not
mentioned in the standard, and Linux would just segfault on it.
config_file, hba_file or ident_file don't suffer that because they are
set at startup using PGDATA.

> Here is the full backtrace from the core file using 9.5.3:
> This has been tested only with 9.5 and 9.4.
>
> The following simple patch seems to fix this bug:

Another thing that we could do is as well to set external_pid_file to
PGDATA/postmaster.pid at startup like the other *_file GUCs for
consistency's sake, but I'd just use your patch to not change the
default setting on back-branches. Some applications may depend on the
current behavior regarding its default value, that's hard to know..

By the way, I found as well that session_authorization suffers the
same problem. In the latter case the variable where is allocated the
value is completely dummy as only its assign hook is used but we
should definitely not crash on that at postmaster startup. And the
trick is that we also should have a NULL value in this case because
check_session_authorization relies on that.

By the way, your patch is not correct. By setting up a value "" as
default value postmaster.c complains with a "cannot write external PID
file" error.

So instead of your patch, I think that we'd rather just change
PostmasterMain() so as it outputs something useful to the user in case
of a NULL value, like that:
$ postgres -C external_pid_file
(null)
$ postgres -C session_authorization
(null)
This will prevent any other error of this type in the future for new
parameters, and has no risk of impacting existing applications. See
the simple patch attached.
--
Michael