Re: ALTER SYSTEM SET command to change postgresql.conf parameters

On Sun, Dec 22, 2013 at 3:01 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> I found the bug of ALTER SYSTEM SET patch. The procedure to reproduce
> it is here.
>
> $ psql
> =# ALTER SYSTEM SET shared_buffers = '512MB';
> ALTER SYSTEM
> =# \q
> $ pg_ctl -D data reload
> server signaled
> 2013-12-22 18:24:13 JST LOG: received SIGHUP, reloading configuration files
> 2013-12-22 18:24:13 JST LOG: parameter "shared_buffers" cannot be
> changed without restarting the server
> 2013-12-22 18:24:13 JST LOG: configuration file "X??" contains
> errors; unaffected changes were applied
>
> The configuration file name in the last log message is broken. This problem was
> introduced by the ALTER SYSTEM SET patch.
>
>> FreeConfigVariables(head);
>> <snip>
>> else if (apply)
>> ereport(elevel,
>> (errcode(ERRCODE_CONFIG_FILE_ERROR),
>> errmsg("configuration file \"%s\" contains errors; unaffected changes were applied",
>> ErrorConfFile)));
>
> The cause of the problem is that, in ProcessConfigFile(), the log
> message including
> the 'ErrorConfFile' is emitted after 'head' is free'd even though
> 'ErrorConfFile' points
> to one of entry of the 'head'.

Your analysis is absolutely right.
The reason for this issue is that we want to display filename to which
erroneous parameter
belongs and in this case we are freeing the parameter info before actual error.
To fix, we need to save the filename value before freeing parameter list.

Please find the attached patch to fix this issue.

Note - In my m/c, I could not reproduce the issue. I think this is not
surprising because we
are not setting freed memory to NULL, so it can display anything
(sometimes right value as well)

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com