pg_upgrade exit_nicely()
While reading around in pg_upgrade code I came across the slightly
bizarre function
void exit_nicely(bool need_cleanup)
The parameter doesn't actually determine whether any cleanup is done.
The "cleanup" is done anyway and the parameter only controls the exit
code in a backwards way.
Also most of the cleanup appears to be useless, because you don't need
to close files or free memory before the program exits.
I figured this could be written more cleanly with an exit hook, so here
is a patch. I don't care much whether this patch is for now or later,
just wanted to throw it out there.
Attachments:
pgupgrade-exit.difftext/x-patch; charset=UTF-8; name=pgupgrade-exit.diffDownload
diff --git i/contrib/pg_upgrade/check.c w/contrib/pg_upgrade/check.c
index 1c4847a..05aac8f 100644
--- i/contrib/pg_upgrade/check.c
+++ w/contrib/pg_upgrade/check.c
@@ -131,7 +131,7 @@ report_clusters_compatible(void)
pg_log(PG_REPORT, "\n*Clusters are compatible*\n");
/* stops new cluster */
stop_postmaster(false, false);
- exit_nicely(false);
+ exit(0);
}
pg_log(PG_REPORT, "\n"
diff --git i/contrib/pg_upgrade/option.c w/contrib/pg_upgrade/option.c
index c984535..857d652 100644
--- i/contrib/pg_upgrade/option.c
+++ w/contrib/pg_upgrade/option.c
@@ -77,12 +77,12 @@ parseCommandLine(int argc, char *argv[])
strcmp(argv[1], "-?") == 0)
{
usage();
- exit_nicely(false);
+ exit(0);
}
if (strcmp(argv[1], "--version") == 0 || strcmp(argv[1], "-V") == 0)
{
pg_log(PG_REPORT, "pg_upgrade " PG_VERSION "\n");
- exit_nicely(false);
+ exit(0);
}
}
@@ -125,7 +125,7 @@ parseCommandLine(int argc, char *argv[])
if ((log_opts.debug_fd = fopen(optarg, "w")) == NULL)
{
pg_log(PG_FATAL, "cannot open debug file\n");
- exit_nicely(false);
+ exit(1);
}
break;
@@ -141,7 +141,7 @@ parseCommandLine(int argc, char *argv[])
if ((old_cluster.port = atoi(optarg)) <= 0)
{
pg_log(PG_FATAL, "invalid old port number\n");
- exit_nicely(false);
+ exit(1);
}
break;
@@ -149,7 +149,7 @@ parseCommandLine(int argc, char *argv[])
if ((new_cluster.port = atoi(optarg)) <= 0)
{
pg_log(PG_FATAL, "invalid new port number\n");
- exit_nicely(false);
+ exit(1);
}
break;
diff --git i/contrib/pg_upgrade/pg_upgrade.h w/contrib/pg_upgrade/pg_upgrade.h
index 4461952..8f72ea8 100644
--- i/contrib/pg_upgrade/pg_upgrade.h
+++ w/contrib/pg_upgrade/pg_upgrade.h
@@ -363,7 +363,6 @@ void check_for_libpq_envvars(void);
/* util.c */
-void exit_nicely(bool need_cleanup);
char *quote_identifier(const char *s);
int get_user_info(char **user_name);
void check_ok(void);
diff --git i/contrib/pg_upgrade/server.c w/contrib/pg_upgrade/server.c
index 4d5f373..a155f90 100644
--- i/contrib/pg_upgrade/server.c
+++ w/contrib/pg_upgrade/server.c
@@ -23,7 +23,7 @@ static bool test_server_conn(ClusterInfo *cluster, int timeout);
*
* Connects to the desired database on the designated server.
* If the connection attempt fails, this function logs an error
- * message and calls exit_nicely() to kill the program.
+ * message and calls exit() to kill the program.
*/
PGconn *
connectToServer(ClusterInfo *cluster, const char *db_name)
@@ -45,7 +45,8 @@ connectToServer(ClusterInfo *cluster, const char *db_name)
if (conn)
PQfinish(conn);
- exit_nicely(true);
+ printf("Failure, exiting\n");
+ exit(1);
}
return conn;
@@ -57,7 +58,7 @@ connectToServer(ClusterInfo *cluster, const char *db_name)
*
* Formats a query string from the given arguments and executes the
* resulting query. If the query fails, this function logs an error
- * message and calls exit_nicely() to kill the program.
+ * message and calls exit() to kill the program.
*/
PGresult *
executeQueryOrDie(PGconn *conn, const char *fmt,...)
@@ -81,8 +82,8 @@ executeQueryOrDie(PGconn *conn, const char *fmt,...)
PQerrorMessage(conn));
PQclear(result);
PQfinish(conn);
- exit_nicely(true);
- return NULL; /* Never get here, but keeps compiler happy */
+ printf("Failure, exiting\n");
+ exit(1);
}
else
return result;
@@ -152,6 +153,18 @@ get_major_server_version(ClusterInfo *cluster)
}
+static void
+#ifdef HAVE_ATEXIT
+stop_postmaster_exit_hook(void)
+#else
+stop_postmaster_exit_hook(int exitstatus, void *arg)
+#endif
+{
+ stop_postmaster(true, true);
+
+}
+
+
void
start_postmaster(ClusterInfo *cluster, bool quiet)
{
@@ -159,11 +172,22 @@ start_postmaster(ClusterInfo *cluster, bool quiet)
const char *bindir;
const char *datadir;
unsigned short port;
+ bool exit_hook_registered = false;
bindir = cluster->bindir;
datadir = cluster->pgdata;
port = cluster->port;
+ if (!exit_hook_registered)
+ {
+#ifdef HAVE_ATEXIT
+ atexit(stop_postmaster_exit_hook);
+#else
+ on_exit(stop_postmaster_exit_hook);
+#endif
+ exit_hook_registered = true;
+ }
+
/*
* On Win32, we can't send both pg_upgrade output and pg_ctl output to the
* same file because we get the error: "The process cannot access the file
diff --git i/contrib/pg_upgrade/util.c w/contrib/pg_upgrade/util.c
index f957508..804aa0d 100644
--- i/contrib/pg_upgrade/util.c
+++ w/contrib/pg_upgrade/util.c
@@ -99,7 +99,8 @@ pg_log(eLogType type, char *fmt,...)
case PG_FATAL:
printf("%s", "\n");
printf("%s", _(message));
- exit_nicely(true);
+ printf("Failure, exiting\n");
+ exit(1);
break;
case PG_DEBUG:
@@ -184,36 +185,6 @@ get_user_info(char **user_name)
}
-void
-exit_nicely(bool need_cleanup)
-{
- stop_postmaster(true, true);
-
- pg_free(log_opts.filename);
-
- if (log_opts.fd)
- fclose(log_opts.fd);
-
- if (log_opts.debug_fd)
- fclose(log_opts.debug_fd);
-
- /* terminate any running instance of postmaster */
- if (os_info.postmasterPID != 0)
- kill(os_info.postmasterPID, SIGTERM);
-
- if (need_cleanup)
- {
- printf("Failure, exiting\n");
- /*
- * FIXME must delete intermediate files
- */
- exit(1);
- }
- else
- exit(0);
-}
-
-
void *
pg_malloc(int n)
{
Feel free to apply this to HEAD.
---------------------------------------------------------------------------
Peter Eisentraut wrote:
While reading around in pg_upgrade code I came across the slightly
bizarre functionvoid exit_nicely(bool need_cleanup)
The parameter doesn't actually determine whether any cleanup is done.
The "cleanup" is done anyway and the parameter only controls the exit
code in a backwards way.Also most of the cleanup appears to be useless, because you don't need
to close files or free memory before the program exits.I figured this could be written more cleanly with an exit hook, so here
is a patch. I don't care much whether this patch is for now or later,
just wanted to throw it out there.
[ Attachment, skipping... ]
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +