pg_upgrade - a function parameter shadows global 'new_cluster'

Started by Peter Smithover 2 years ago3 messages
#1Peter Smith
smithpb2250@gmail.com
1 attachment(s)

Hi hackers.

During a recent review of nearby code I noticed that there was a shadowing
of the 'new_cluster' global variable by a function parameter:

Here:
static void check_for_new_tablespace_dir(ClusterInfo *new_cluster);

~~~

It looks like it has been like this for a couple of years. I guess this
might have been found/fixed earlier had the code been compiled differently:

check.c: In function ‘check_for_new_tablespace_dir’:
check.c:381:43: warning: declaration of ‘new_cluster’ shadows a global
declaration [-Wshadow]
check_for_new_tablespace_dir(ClusterInfo *new_cluster)
^
In file included from check.c:16:0:
pg_upgrade.h:337:4: warning: shadowed declaration is here [-Wshadow]
new_cluster;
^

~~~

PSA a small patch to remove the unnecessary parameter, and so eliminate
this shadowing.

Thoughts?

------
Kind Regards,
Peter Smith.
Fujitsu Australia.

Attachments:

v1-0001-Remove-the-shadowing-of-new_cluster-global.patchapplication/octet-stream; name=v1-0001-Remove-the-shadowing-of-new_cluster-global.patchDownload
From c73d4ad6499d12969bdd3ad437587bf326c02e39 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Wed, 23 Aug 2023 11:16:58 +1000
Subject: [PATCH v1] Remove the shadowing of new_cluster global

---
 src/bin/pg_upgrade/check.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 64024e3..65ced12 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -28,7 +28,7 @@ static void check_for_reg_data_type_usage(ClusterInfo *cluster);
 static void check_for_aclitem_data_type_usage(ClusterInfo *cluster);
 static void check_for_jsonb_9_4_usage(ClusterInfo *cluster);
 static void check_for_pg_role_prefix(ClusterInfo *cluster);
-static void check_for_new_tablespace_dir(ClusterInfo *new_cluster);
+static void check_for_new_tablespace_dir(void);
 static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster);
 
 
@@ -209,7 +209,7 @@ check_new_cluster(void)
 
 	check_for_prepared_transactions(&new_cluster);
 
-	check_for_new_tablespace_dir(&new_cluster);
+	check_for_new_tablespace_dir();
 }
 
 
@@ -377,7 +377,7 @@ check_new_cluster_is_empty(void)
  * during schema restore.
  */
 static void
-check_for_new_tablespace_dir(ClusterInfo *new_cluster)
+check_for_new_tablespace_dir(void)
 {
 	int			tblnum;
 	char		new_tablespace_dir[MAXPGPATH];
@@ -390,7 +390,7 @@ check_for_new_tablespace_dir(ClusterInfo *new_cluster)
 
 		snprintf(new_tablespace_dir, MAXPGPATH, "%s%s",
 				 os_info.old_tablespaces[tblnum],
-				 new_cluster->tablespace_suffix);
+				 new_cluster.tablespace_suffix);
 
 		if (stat(new_tablespace_dir, &statbuf) == 0 || errno != ENOENT)
 			pg_fatal("new cluster tablespace directory already exists: \"%s\"",
-- 
1.8.3.1

#2Daniel Gustafsson
daniel@yesql.se
In reply to: Peter Smith (#1)
Re: pg_upgrade - a function parameter shadows global 'new_cluster'

On 23 Aug 2023, at 03:28, Peter Smith <smithpb2250@gmail.com> wrote:

PSA a small patch to remove the unnecessary parameter, and so eliminate this shadowing.

Agreed, applied. Thanks!

--
Daniel Gustafsson

#3Peter Smith
smithpb2250@gmail.com
In reply to: Daniel Gustafsson (#2)
Re: pg_upgrade - a function parameter shadows global 'new_cluster'

On Wed, Aug 23, 2023 at 6:00 PM Daniel Gustafsson <daniel@yesql.se> wrote:

On 23 Aug 2023, at 03:28, Peter Smith <smithpb2250@gmail.com> wrote:

PSA a small patch to remove the unnecessary parameter, and so eliminate

this shadowing.

Agreed, applied. Thanks!

Thanks for pushing!

------
Kind Regards,
Peter Smith.
Fujitsu Australia