v1-0007-Refactor-split-verify_control_file.patch

From 302cf37bb76947fa8e850d4096a77575faf4dca3 Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Tue, 2 Jul 2024 10:10:08 +0530
Subject: [PATCH v1 07/10] Refactor: split verify_control_file.

Moved verify_control_file doing the control file checks to a separate
function that can be called from other places as well.
---
 src/bin/pg_verifybackup/pg_verifybackup.c | 40 ++++++++++-------------
 src/bin/pg_verifybackup/pg_verifybackup.h | 14 ++++++++
 2 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c
index 7b845bece71..375f196b300 100644
--- a/src/bin/pg_verifybackup/pg_verifybackup.c
+++ b/src/bin/pg_verifybackup/pg_verifybackup.c
@@ -18,7 +18,6 @@
 #include <sys/stat.h>
 #include <time.h>
 
-#include "common/logging.h"
 #include "common/parse_manifest.h"
 #include "fe_utils/simple_list.h"
 #include "getopt_long.h"
@@ -61,8 +60,6 @@ static void verify_backup_directory(verifier_context *context,
 									char *relpath, char *fullpath);
 static void verify_backup_file(verifier_context *context,
 							   char *relpath, char *fullpath);
-static void verify_control_file(const char *controlpath,
-								uint64 manifest_system_identifier);
 static void report_extra_backup_files(verifier_context *context);
 static void verify_backup_checksums(verifier_context *context);
 static void verify_file_checksum(verifier_context *context,
@@ -626,14 +623,20 @@ verify_backup_file(verifier_context *context, char *relpath, char *fullpath)
 	/* Check whether there's an entry in the manifest hash. */
 	m = verify_manifest_entry(context, relpath, sb.st_size);
 
-	/*
-	 * Validate the manifest system identifier, not available in manifest
-	 * version 1.
-	 */
-	if (context->manifest->version != 1 &&
-		strcmp(relpath, "global/pg_control") == 0 &&
-		m->matched && !m->bad)
-		verify_control_file(fullpath, context->manifest->system_identifier);
+	/* Validate the manifest system identifier */
+	if (m != NULL && should_verify_sysid(context->manifest, m))
+	{
+		ControlFileData *control_file;
+		bool		crc_ok;
+
+		pg_log_debug("reading \"%s\"", fullpath);
+		control_file = get_controlfile_by_exact_path(fullpath, &crc_ok);
+
+		verify_control_file_data(control_file, fullpath, crc_ok,
+								 context->manifest->system_identifier);
+		/* Release memory. */
+		pfree(control_file);
+	}
 }
 
 /*
@@ -683,15 +686,11 @@ verify_manifest_entry(verifier_context *context, char *relpath, size_t filesize)
  * Sanity check control file and validate system identifier against manifest
  * system identifier.
  */
-static void
-verify_control_file(const char *controlpath, uint64 manifest_system_identifier)
+void
+verify_control_file_data(ControlFileData *control_file,
+						 const char *controlpath, bool crc_ok,
+						 uint64 manifest_system_identifier)
 {
-	ControlFileData *control_file;
-	bool		crc_ok;
-
-	pg_log_debug("reading \"%s\"", controlpath);
-	control_file = get_controlfile_by_exact_path(controlpath, &crc_ok);
-
 	/* Control file contents not meaningful if CRC is bad. */
 	if (!crc_ok)
 		report_fatal_error("%s: CRC is incorrect", controlpath);
@@ -707,9 +706,6 @@ verify_control_file(const char *controlpath, uint64 manifest_system_identifier)
 						   controlpath,
 						   (unsigned long long) manifest_system_identifier,
 						   (unsigned long long) control_file->system_identifier);
-
-	/* Release memory. */
-	pfree(control_file);
 }
 
 /*
diff --git a/src/bin/pg_verifybackup/pg_verifybackup.h b/src/bin/pg_verifybackup/pg_verifybackup.h
index 50a285752aa..64508578290 100644
--- a/src/bin/pg_verifybackup/pg_verifybackup.h
+++ b/src/bin/pg_verifybackup/pg_verifybackup.h
@@ -16,6 +16,7 @@
 
 #include "common/controldata_utils.h"
 #include "common/hashfn_unstable.h"
+#include "common/logging.h"
 #include "common/parse_manifest.h"
 #include "fe_utils/simple_list.h"
 
@@ -46,6 +47,16 @@ typedef struct manifest_file
 #define should_verify_checksum(m) \
 	(((m)->matched) && !((m)->bad) && (((m)->checksum_type) != CHECKSUM_TYPE_NONE))
 
+/*
+ * Validate the manifest system identifier against the control file; this
+ * feature is not available in manifest version 1.  This validation should be
+ * carried out only if the manifest entry validation is completed without any
+ * errors.
+ */
+#define should_verify_sysid(manifest, m) \
+	(((manifest)->version != 1) && \
+	 ((m)->matched) && !((m)->bad) && (strcmp((m)->pathname, "global/pg_control") == 0))
+
 /*
  * Define a hash table which we can use to store information about the files
  * mentioned in the backup manifest.
@@ -105,6 +116,9 @@ extern bool verify_content_checksum(verifier_context *context,
 									pg_checksum_context *checksum_ctx,
 									manifest_file *m, uint8 *buf,
 									int buf_len, size_t *computed_len);
+extern void verify_control_file_data(ControlFileData *control_file,
+									 const char *controlpath, bool crc_ok,
+									 uint64 manifest_system_identifier);
 
 extern void report_backup_error(verifier_context *context,
 								const char *pg_restrict fmt,...)
-- 
2.18.0