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
Attachments:
pg-upgrade-atexit.patchtext/x-patch; name=pg-upgrade-atexit.patchDownload
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c
index 969e5d6..a13800f 100644
--- a/src/bin/pg_upgrade/server.c
+++ b/src/bin/pg_upgrade/server.c
@@ -14,6 +14,8 @@
static PGconn *get_db_conn(ClusterInfo *cluster, const char *db_name);
+static bool exit_hook_registered = false;
+
/*
* connectToServer()
@@ -174,7 +176,6 @@ start_postmaster(ClusterInfo *cluster, bool throw_error)
{
char cmd[MAXPGPATH * 4 + 1000];
PGconn *conn;
- bool exit_hook_registered = false;
bool pg_ctl_return = false;
char socket_string[MAXPGPATH + 200];
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
Attachments:
0001-start_postmaster-of-pg_upgarde-registers-exit-functi.patchtext/x-patch; charset=us-asciiDownload
From 21695d427b146a0a44297558c747bb75e48c62dd Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 28 Jul 2016 19:13:46 +0900
Subject: [PATCH] start_postmaster of pg_upgarde registers exit function more
than once.
start_postmaster of pg_upgrade is intending to register an exit
function but it fails to prevent duplicate regstration by defining
control variable as function-local. It should be static.
---
src/bin/pg_upgrade/server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c
index 969e5d6..3fe169d 100644
--- a/src/bin/pg_upgrade/server.c
+++ b/src/bin/pg_upgrade/server.c
@@ -174,7 +174,7 @@ start_postmaster(ClusterInfo *cluster, bool throw_error)
{
char cmd[MAXPGPATH * 4 + 1000];
PGconn *conn;
- bool exit_hook_registered = false;
+ static bool exit_hook_registered = false;
bool pg_ctl_return = false;
char socket_string[MAXPGPATH + 200];
--
1.8.3.1
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers