Use XLOG_CONTROL_FILE macro everywhere?

Started by Anton A. Melnikovover 1 year ago26 messages

a.melnikov@postgrespro.ru

over 1 year ago

1 attachment(s)

Hello!

There is a macro XLOG_CONTROL_FILE for control file name
defined in access/xlog_internal.h
And there are some places in code where this macro is used
like here
https://github.com/postgres/postgres/blob/84db9a0eb10dd1dbee6db509c0e427fa237177dc/src/bin/pg_resetwal/pg_resetwal.c#L588
or here
https://github.com/postgres/postgres/blob/84db9a0eb10dd1dbee6db509c0e427fa237177dc/src/common/controldata_utils.c#L214

But there are some other places where the control file
name is used as text string directly.

May be better use this macro everywhere in C code?
The patch attached tries to do this.

Would be glad if take a look on it.

With the best regards,

--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachments:

0001-Use-XLOG_CONTROL_FILE-macro-everywhere-in-C-code.patchtext/x-patch; charset=UTF-8; name=0001-Use-XLOG_CONTROL_FILE-macro-everywhere-in-C-code.patchDownload

From 2692683142c98572114c32f696b55c6a642dd3e9 Mon Sep 17 00:00:00 2001
From: "Anton A. Melnikov" <a.melnikov@postgrespro.ru>
Date: Fri, 19 Apr 2024 06:12:44 +0300
Subject: [PATCH] Use XLOG_CONTROL_FILE macro  everywhere in C code

---
 src/backend/backup/basebackup.c             |  2 +-
 src/backend/postmaster/postmaster.c         |  3 ++-
 src/bin/pg_combinebackup/pg_combinebackup.c |  5 +++--
 src/bin/pg_controldata/pg_controldata.c     |  2 +-
 src/bin/pg_rewind/filemap.c                 |  3 ++-
 src/bin/pg_rewind/pg_rewind.c               |  8 ++++----
 src/bin/pg_upgrade/controldata.c            | 11 ++++++-----
 src/bin/pg_verifybackup/pg_verifybackup.c   |  3 ++-
 src/common/controldata_utils.c              |  2 +-
 9 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index 9a2bf59e84..c5a1e18104 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -1347,7 +1347,7 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly,
 		snprintf(pathbuf, sizeof(pathbuf), "%s/%s", path, de->d_name);
 
 		/* Skip pg_control here to back up it last */
-		if (strcmp(pathbuf, "./global/pg_control") == 0)
+		if (strcmp(pathbuf, "./" XLOG_CONTROL_FILE) == 0)
 			continue;
 
 		if (lstat(pathbuf, &statbuf) != 0)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 7f3170a8f0..02b36caea7 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -90,6 +90,7 @@
 #endif
 
 #include "access/xlog.h"
+#include "access/xlog_internal.h"
 #include "access/xlogrecovery.h"
 #include "common/file_perm.h"
 #include "common/file_utils.h"
@@ -1489,7 +1490,7 @@ checkControlFile(void)
 	char		path[MAXPGPATH];
 	FILE	   *fp;
 
-	snprintf(path, sizeof(path), "%s/global/pg_control", DataDir);
+	snprintf(path, sizeof(path), "%s/%s", DataDir, XLOG_CONTROL_FILE);
 
 	fp = AllocateFile(path, PG_BINARY_R);
 	if (fp == NULL)
diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c
index b26c532445..16448e78b8 100644
--- a/src/bin/pg_combinebackup/pg_combinebackup.c
+++ b/src/bin/pg_combinebackup/pg_combinebackup.c
@@ -16,6 +16,7 @@
 #include <fcntl.h>
 #include <limits.h>
 
+#include "access/xlog_internal.h"
 #include "backup_label.h"
 #include "common/blkreftable.h"
 #include "common/checksum_helper.h"
@@ -284,7 +285,7 @@ main(int argc, char *argv[])
 		{
 			char	   *controlpath;
 
-			controlpath = psprintf("%s/%s", prior_backup_dirs[i], "global/pg_control");
+			controlpath = psprintf("%s/%s", prior_backup_dirs[i], XLOG_CONTROL_FILE);
 
 			pg_fatal("%s: manifest system identifier is %llu, but control file has %llu",
 					 controlpath,
@@ -591,7 +592,7 @@ check_control_files(int n_backups, char **backup_dirs)
 		bool		crc_ok;
 		char	   *controlpath;
 
-		controlpath = psprintf("%s/%s", backup_dirs[i], "global/pg_control");
+		controlpath = psprintf("%s/%s", backup_dirs[i], XLOG_CONTROL_FILE);
 		pg_log_debug("reading \"%s\"", controlpath);
 		control_file = get_controlfile_by_exact_path(controlpath, &crc_ok);
 
diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c
index 93e0837947..1fe6618e7f 100644
--- a/src/bin/pg_controldata/pg_controldata.c
+++ b/src/bin/pg_controldata/pg_controldata.c
@@ -1,7 +1,7 @@
 /*
  * pg_controldata
  *
- * reads the data from $PGDATA/global/pg_control
+ * reads the data from $PGDATA/<control file>
  *
  * copyright (c) Oliver Elphick <olly@lfix.co.uk>, 2001;
  * license: BSD
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index 4458324c9d..6224e94d09 100644
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -26,6 +26,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#include "access/xlog_internal.h"
 #include "catalog/pg_tablespace_d.h"
 #include "common/file_utils.h"
 #include "common/hashfn_unstable.h"
@@ -643,7 +644,7 @@ decide_file_action(file_entry_t *entry)
 	 * Don't touch the control file. It is handled specially, after copying
 	 * all the other files.
 	 */
-	if (strcmp(path, "global/pg_control") == 0)
+	if (strcmp(path, XLOG_CONTROL_FILE) == 0)
 		return FILE_ACTION_NONE;
 
 	/* Skip macOS system files */
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 8449ae78ef..213a8b22cd 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -328,7 +328,7 @@ main(int argc, char **argv)
 	 * need to make sure by themselves that the target cluster is in a clean
 	 * state.
 	 */
-	buffer = slurpFile(datadir_target, "global/pg_control", &size);
+	buffer = slurpFile(datadir_target, XLOG_CONTROL_FILE, &size);
 	digestControlFile(&ControlFile_target, buffer, size);
 	pg_free(buffer);
 
@@ -338,12 +338,12 @@ main(int argc, char **argv)
 	{
 		ensureCleanShutdown(argv[0]);
 
-		buffer = slurpFile(datadir_target, "global/pg_control", &size);
+		buffer = slurpFile(datadir_target, XLOG_CONTROL_FILE, &size);
 		digestControlFile(&ControlFile_target, buffer, size);
 		pg_free(buffer);
 	}
 
-	buffer = source->fetch_file(source, "global/pg_control", &size);
+	buffer = source->fetch_file(source, XLOG_CONTROL_FILE, &size);
 	digestControlFile(&ControlFile_source, buffer, size);
 	pg_free(buffer);
 
@@ -631,7 +631,7 @@ perform_rewind(filemap_t *filemap, rewind_source *source,
 	 * Fetch the control file from the source last. This ensures that the
 	 * minRecoveryPoint is up-to-date.
 	 */
-	buffer = source->fetch_file(source, "global/pg_control", &size);
+	buffer = source->fetch_file(source, XLOG_CONTROL_FILE, &size);
 	digestControlFile(&ControlFile_source_after, buffer, size);
 	pg_free(buffer);
 
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 1f0ccea3ed..ae2c5d580a 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -11,6 +11,7 @@
 
 #include <ctype.h>
 
+#include "access/xlog_internal.h"
 #include "common/string.h"
 #include "pg_upgrade.h"
 
@@ -714,10 +715,10 @@ disable_old_cluster(void)
 				new_path[MAXPGPATH];
 
 	/* rename pg_control so old server cannot be accidentally started */
-	prep_status("Adding \".old\" suffix to old global/pg_control");
+	prep_status("Adding \".old\" suffix to old " XLOG_CONTROL_FILE);
 
-	snprintf(old_path, sizeof(old_path), "%s/global/pg_control", old_cluster.pgdata);
-	snprintf(new_path, sizeof(new_path), "%s/global/pg_control.old", old_cluster.pgdata);
+	snprintf(old_path, sizeof(old_path), "%s/%s", old_cluster.pgdata, XLOG_CONTROL_FILE);
+	snprintf(new_path, sizeof(new_path), "%s/%s.old", old_cluster.pgdata, XLOG_CONTROL_FILE);
 	if (pg_mv_file(old_path, new_path) != 0)
 		pg_fatal("could not rename file \"%s\" to \"%s\": %m",
 				 old_path, new_path);
@@ -725,8 +726,8 @@ disable_old_cluster(void)
 
 	pg_log(PG_REPORT, "\n"
 		   "If you want to start the old cluster, you will need to remove\n"
-		   "the \".old\" suffix from %s/global/pg_control.old.\n"
+		   "the \".old\" suffix from %s/%s.old.\n"
 		   "Because \"link\" mode was used, the old cluster cannot be safely\n"
 		   "started once the new cluster has been started.",
-		   old_cluster.pgdata);
+		   old_cluster.pgdata, XLOG_CONTROL_FILE);
 }
diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c
index fd610c20a6..53338df397 100644
--- a/src/bin/pg_verifybackup/pg_verifybackup.c
+++ b/src/bin/pg_verifybackup/pg_verifybackup.c
@@ -18,6 +18,7 @@
 #include <sys/stat.h>
 #include <time.h>
 
+#include "access/xlog_internal.h"
 #include "common/controldata_utils.h"
 #include "common/hashfn_unstable.h"
 #include "common/logging.h"
@@ -735,7 +736,7 @@ verify_backup_file(verifier_context *context, char *relpath, char *fullpath)
 	 * version 1.
 	 */
 	if (context->manifest->version != 1 &&
-		strcmp(relpath, "global/pg_control") == 0)
+		strcmp(relpath, XLOG_CONTROL_FILE) == 0)
 		verify_control_file(fullpath, context->manifest->system_identifier);
 
 	/* Update statistics for progress report, if necessary */
diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c
index 82309b2510..411139eeb0 100644
--- a/src/common/controldata_utils.c
+++ b/src/common/controldata_utils.c
@@ -53,7 +53,7 @@ get_controlfile(const char *DataDir, bool *crc_ok_p)
 {
 	char		ControlFilePath[MAXPGPATH];
 
-	snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir);
+	snprintf(ControlFilePath, MAXPGPATH, "%s/%s", DataDir, XLOG_CONTROL_FILE);
 
 	return get_controlfile_by_exact_path(ControlFilePath, crc_ok_p);
 }
-- 
2.43.2