Thread: pg_upgrade: exit_hook_registered variable

Hello hackers,

I noticed that exit_hook_registered variable in start_postmaster() is
local variable. Shouldn't it be a static variable?

I attached a patch. Thank you!

--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

Hello,

> I noticed that exit_hook_registered variable in start_postmaster() is
> local variable. Shouldn't it be a static variable?
> 
> I attached a patch. Thank you!

Good catch! It is totally useless unless being static.

But I think it is not necessary to go outside
start_postmaster. Just being static staying in start_postmaster
is enough.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
>> I noticed that exit_hook_registered variable in start_postmaster() is
>> local variable. Shouldn't it be a static variable?

> Good catch! It is totally useless unless being static.

Indeed.

> But I think it is not necessary to go outside
> start_postmaster. Just being static staying in start_postmaster
> is enough.

I agree, since there's no need for any other function to touch it.
But personally I'd want to visually separate the static variable
from the non-static ones.  So maybe like this:

{char        cmd[MAXPGPATH * 4 + 1000];PGconn       *conn;
-    bool        exit_hook_registered = false;bool        pg_ctl_return = false;char        socket_string[MAXPGPATH +
200];
+
+    static bool    exit_hook_registered = false;
if (!exit_hook_registered){


Will push it in a bit.
        regards, tom lane

At Thu, 28 Jul 2016 10:58:24 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in <4782.1469717904@sss.pgh.pa.us>
> Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
> >> I noticed that exit_hook_registered variable in start_postmaster() is
> >> local variable. Shouldn't it be a static variable?
> 
> > Good catch! It is totally useless unless being static.
> 
> Indeed.
> 
> > But I think it is not necessary to go outside
> > start_postmaster. Just being static staying in start_postmaster
> > is enough.
> 
> I agree, since there's no need for any other function to touch it.
> But personally I'd want to visually separate the static variable
> from the non-static ones.  So maybe like this:

It looks better indeed.

> {
>     char        cmd[MAXPGPATH * 4 + 1000];
>     PGconn       *conn;
> -    bool        exit_hook_registered = false;
>     bool        pg_ctl_return = false;
>     char        socket_string[MAXPGPATH + 200];
> +
> +    static bool    exit_hook_registered = false;
> 
>     if (!exit_hook_registered)
>     {
> 
> 
> Will push it in a bit.

Thanks.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center