From cad4323cd345ec91fbe8d118436d3d50fa53c300 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Wed, 31 Aug 2022 13:06:50 +0200 Subject: [PATCH] Refactor check_ functions to use filehandle for status When reporting failure in check_ functions there is (typically) a text- file mentioned in the error report which contains further details. Some check_ functions kept a separate flag variable to indicate failure, and some just checked the state of the filehandle as it's guaranteed to be open when the check failed. This refactors the functions to consistently do the same check on error reporting. As the error report contains the filepath, it makes more sense to check the filehandle state and skip the flag variable. Reviewed-by: Nathan Bossart Reviewed-by: Bruce Momjian Discussion: https://postgr.es/m/595759F6-625B-4ED7-8125-91AF00437F83@yesql.se --- src/bin/pg_upgrade/check.c | 30 +++++------------------------- src/bin/pg_upgrade/function.c | 4 +--- src/bin/pg_upgrade/version.c | 7 +------ 3 files changed, 7 insertions(+), 34 deletions(-) diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 0ed0590d82d..f4969bcdad7 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -711,7 +711,6 @@ check_proper_datallowconn(ClusterInfo *cluster) int i_datallowconn; FILE *script = NULL; char output_path[MAXPGPATH]; - bool found = false; prep_status("Checking database connection settings"); @@ -750,7 +749,6 @@ check_proper_datallowconn(ClusterInfo *cluster) */ if (strcmp(datallowconn, "f") == 0) { - found = true; if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) pg_fatal("could not open file \"%s\": %s", output_path, strerror(errno)); @@ -765,10 +763,8 @@ check_proper_datallowconn(ClusterInfo *cluster) PQfinish(conn_template1); if (script) - fclose(script); - - if (found) { + fclose(script); pg_log(PG_REPORT, "fatal"); pg_fatal("All non-template0 databases must allow connections, i.e. their\n" "pg_database.datallowconn must be true. Your installation contains\n" @@ -829,7 +825,6 @@ check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster) { int dbnum; FILE *script = NULL; - bool found = false; char output_path[MAXPGPATH]; prep_status("Checking for contrib/isn with bigint-passing mismatch"); @@ -870,7 +865,6 @@ check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster) i_proname = PQfnumber(res, "proname"); for (rowno = 0; rowno < ntups; rowno++) { - found = true; if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) pg_fatal("could not open file \"%s\": %s", output_path, strerror(errno)); @@ -890,10 +884,8 @@ check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster) } if (script) - fclose(script); - - if (found) { + fclose(script); pg_log(PG_REPORT, "fatal"); pg_fatal("Your installation contains \"contrib/isn\" functions which rely on the\n" "bigint data type. Your old and new clusters pass bigint values\n" @@ -915,7 +907,6 @@ check_for_user_defined_postfix_ops(ClusterInfo *cluster) { int dbnum; FILE *script = NULL; - bool found = false; char output_path[MAXPGPATH]; prep_status("Checking for user-defined postfix operators"); @@ -968,7 +959,6 @@ check_for_user_defined_postfix_ops(ClusterInfo *cluster) i_typname = PQfnumber(res, "typname"); for (rowno = 0; rowno < ntups; rowno++) { - found = true; if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) pg_fatal("could not open file \"%s\": %s", @@ -992,10 +982,8 @@ check_for_user_defined_postfix_ops(ClusterInfo *cluster) } if (script) - fclose(script); - - if (found) { + fclose(script); pg_log(PG_REPORT, "fatal"); pg_fatal("Your installation contains user-defined postfix operators, which are not\n" "supported anymore. Consider dropping the postfix operators and replacing\n" @@ -1145,7 +1133,6 @@ check_for_tables_with_oids(ClusterInfo *cluster) { int dbnum; FILE *script = NULL; - bool found = false; char output_path[MAXPGPATH]; prep_status("Checking for tables WITH OIDS"); @@ -1179,7 +1166,6 @@ check_for_tables_with_oids(ClusterInfo *cluster) i_relname = PQfnumber(res, "relname"); for (rowno = 0; rowno < ntups; rowno++) { - found = true; if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) pg_fatal("could not open file \"%s\": %s", output_path, strerror(errno)); @@ -1199,10 +1185,8 @@ check_for_tables_with_oids(ClusterInfo *cluster) } if (script) - fclose(script); - - if (found) { + fclose(script); pg_log(PG_REPORT, "fatal"); pg_fatal("Your installation contains tables declared WITH OIDS, which is not\n" "supported anymore. Consider removing the oid column using\n" @@ -1401,7 +1385,6 @@ check_for_user_defined_encoding_conversions(ClusterInfo *cluster) { int dbnum; FILE *script = NULL; - bool found = false; char output_path[MAXPGPATH]; prep_status("Checking for user-defined encoding conversions"); @@ -1441,7 +1424,6 @@ check_for_user_defined_encoding_conversions(ClusterInfo *cluster) i_nspname = PQfnumber(res, "nspname"); for (rowno = 0; rowno < ntups; rowno++) { - found = true; if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) pg_fatal("could not open file \"%s\": %s", @@ -1463,10 +1445,8 @@ check_for_user_defined_encoding_conversions(ClusterInfo *cluster) } if (script) - fclose(script); - - if (found) { + fclose(script); pg_log(PG_REPORT, "fatal"); pg_fatal("Your installation contains user-defined encoding conversions.\n" "The conversion function parameters changed in PostgreSQL version 14\n" diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.c index 70b492cc3a8..93d975864ba 100644 --- a/src/bin/pg_upgrade/function.c +++ b/src/bin/pg_upgrade/function.c @@ -123,7 +123,6 @@ check_loadable_libraries(void) int libnum; int was_load_failure = false; FILE *script = NULL; - bool found = false; char output_path[MAXPGPATH]; prep_status("Checking for presence of required libraries"); @@ -158,7 +157,6 @@ check_loadable_libraries(void) if (PQresultStatus(res) != PGRES_COMMAND_OK) { - found = true; was_load_failure = true; if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) @@ -181,7 +179,7 @@ check_loadable_libraries(void) PQfinish(conn); - if (found) + if (script) { fclose(script); pg_log(PG_REPORT, "fatal"); diff --git a/src/bin/pg_upgrade/version.c b/src/bin/pg_upgrade/version.c index d2636ba857f..064d23797c5 100644 --- a/src/bin/pg_upgrade/version.c +++ b/src/bin/pg_upgrade/version.c @@ -391,7 +391,6 @@ report_extension_updates(ClusterInfo *cluster) { int dbnum; FILE *script = NULL; - bool found = false; char *output_path = "update_extensions.sql"; prep_status("Checking for extension updates"); @@ -417,8 +416,6 @@ report_extension_updates(ClusterInfo *cluster) i_name = PQfnumber(res, "name"); for (rowno = 0; rowno < ntups; rowno++) { - found = true; - if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) pg_fatal("could not open file \"%s\": %s", output_path, strerror(errno)); @@ -442,10 +439,8 @@ report_extension_updates(ClusterInfo *cluster) } if (script) - fclose(script); - - if (found) { + fclose(script); report_status(PG_REPORT, "notice"); pg_log(PG_REPORT, "\n" "Your installation contains extensions that should be updated\n"