From ae9064a6eac9029390ca5c0228fcbcafc5ebc4af Mon Sep 17 00:00:00 2001 From: Amul Sul Date: Tue, 2 Jul 2024 10:10:08 +0530 Subject: [PATCH v2 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 #include -#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