Re: Requesting external_pid_file with postgres -C when not initialized lead to coredump

Поиск
Список
Период
Сортировка
От alain radix
Тема Re: Requesting external_pid_file with postgres -C when not initialized lead to coredump
Дата
Msg-id CA+Ydpwy_xom0anYXwmuFpgyg7DTpYUGtCHU=26EA_hEN4gONUQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Requesting external_pid_file with postgres -C when not initialized lead to coredump  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
Дерево обсуждения


Testing that external_pid_file was not null didn't seem necessary to me because it's initialized to '' and background workers could only start after the startup piece of code.

If external_pid_file is set to null while the server run, then I prefer to not hide the problem as it would only be reported at the server shutdown

I tested with ALTER SYSTEM SET but couldn't set external_pid_file to null, only to ''
This another good reason to use '' instead of null, because it can be set with ALTER SYSTEM SET even if I see no reason to ;-)

Using strlen(external_pid_file) instead of external_pid_file[0] seems more readable to me.
I agree that it cost a few cpu cycles more.

I'm also unsure that testing if(A && B) would always be compiled to A being tested before B so that it won't always protect from testing external_pid_file[0] with external_pid_file being null.
To ensure that, I would prefer :
if(! external_pid_file){
    if(external_pid_file[0])

But, I see it at useless protective code against an impossible situation.
Maybe I'm wrong, but I don't see how it could happen.

About parameters being null, as they're reported as '' in pg_settings and ALTER SYSTEM SET can only set them to '', I consider this should be avoided.
I used all parameters reported to '' in pg_settings after initdb and found only external_pid_file to crash postgres -C

Alain

PS : Thanks for pgBackRest ;-)


On 22 June 2016 at 18:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:
alain radix <alain.radix@gmail.com> writes:
> 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'd still be inclined to do that with something along the lines of

-       if (external_pid_file)
+       if (external_pid_file && external_pid_file[0])

rather than changing the default per se.

                        regards, tom lane



--
Alain Radix

В списке pgsql-hackers по дате отправления: