From cc53ba8d1cc633685bd2238339b8cfe7da03a513 Mon Sep 17 00:00:00 2001 From: Amul Sul Date: Mon, 22 Jan 2024 10:45:19 +0530 Subject: [PATCH v6 1/2] pg_verifybackup: code refactor Rename parser_context to manifest_data and make parse_manifest_file return that instead of out-argument. --- src/bin/pg_verifybackup/pg_verifybackup.c | 75 ++++++++++------------- 1 file changed, 34 insertions(+), 41 deletions(-) diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c index ae8c18f3737..8561678a7d0 100644 --- a/src/bin/pg_verifybackup/pg_verifybackup.c +++ b/src/bin/pg_verifybackup/pg_verifybackup.c @@ -94,31 +94,28 @@ typedef struct manifest_wal_range } manifest_wal_range; /* - * Details we need in callbacks that occur while parsing a backup manifest. + * All the data parsed from a backup_manifest file. */ -typedef struct parser_context +typedef struct manifest_data { - manifest_files_hash *ht; + manifest_files_hash *files; manifest_wal_range *first_wal_range; manifest_wal_range *last_wal_range; -} parser_context; +} manifest_data; /* * All of the context information we need while checking a backup manifest. */ typedef struct verifier_context { - manifest_files_hash *ht; + manifest_data *manifest; char *backup_directory; SimpleStringList ignore_list; bool exit_on_error; bool saw_any_error; } verifier_context; -static void parse_manifest_file(char *manifest_path, - manifest_files_hash **ht_p, - manifest_wal_range **first_wal_range_p); - +static manifest_data *parse_manifest_file(char *manifest_path); static void verifybackup_per_file_cb(JsonManifestParseContext *context, char *pathname, size_t size, pg_checksum_type checksum_type, @@ -142,8 +139,7 @@ static void verify_file_checksum(verifier_context *context, manifest_file *m, char *fullpath); static void parse_required_wal(verifier_context *context, char *pg_waldump_path, - char *wal_directory, - manifest_wal_range *first_wal_range); + char *wal_directory); static void report_backup_error(verifier_context *context, const char *pg_restrict fmt,...) @@ -185,7 +181,6 @@ main(int argc, char **argv) int c; verifier_context context; - manifest_wal_range *first_wal_range; char *manifest_path = NULL; bool no_parse_wal = false; bool quiet = false; @@ -338,7 +333,7 @@ main(int argc, char **argv) * the manifest as fatal; there doesn't seem to be much point in trying to * verify the backup directory against a corrupted manifest. */ - parse_manifest_file(manifest_path, &context.ht, &first_wal_range); + context.manifest = parse_manifest_file(manifest_path); /* * Now scan the files in the backup directory. At this stage, we verify @@ -367,8 +362,7 @@ main(int argc, char **argv) * not to do so. */ if (!no_parse_wal) - parse_required_wal(&context, pg_waldump_path, - wal_directory, first_wal_range); + parse_required_wal(&context, pg_waldump_path, wal_directory); /* * If everything looks OK, tell the user this, unless we were asked to @@ -385,9 +379,8 @@ main(int argc, char **argv) * all the files it mentions, and a linked list of all the WAL ranges it * mentions. */ -static void -parse_manifest_file(char *manifest_path, manifest_files_hash **ht_p, - manifest_wal_range **first_wal_range_p) +static manifest_data * +parse_manifest_file(char *manifest_path) { int fd; struct stat statbuf; @@ -396,8 +389,8 @@ parse_manifest_file(char *manifest_path, manifest_files_hash **ht_p, manifest_files_hash *ht; char *buffer; int rc; - parser_context private_context; JsonManifestParseContext context; + manifest_data *result; /* Open the manifest file. */ if ((fd = open(manifest_path, O_RDONLY | PG_BINARY, 0)) < 0) @@ -436,10 +429,9 @@ parse_manifest_file(char *manifest_path, manifest_files_hash **ht_p, close(fd); /* Parse the manifest. */ - private_context.ht = ht; - private_context.first_wal_range = NULL; - private_context.last_wal_range = NULL; - context.private_data = &private_context; + result = pg_malloc0(sizeof(manifest_data)); + result->files = ht; + context.private_data = result; context.per_file_cb = verifybackup_per_file_cb; context.per_wal_range_cb = verifybackup_per_wal_range_cb; context.error_cb = report_manifest_error; @@ -448,9 +440,7 @@ parse_manifest_file(char *manifest_path, manifest_files_hash **ht_p, /* Done with the buffer. */ pfree(buffer); - /* Return the file hash table and WAL range list we constructed. */ - *ht_p = ht; - *first_wal_range_p = private_context.first_wal_range; + return result; } /* @@ -480,8 +470,8 @@ verifybackup_per_file_cb(JsonManifestParseContext *context, pg_checksum_type checksum_type, int checksum_length, uint8 *checksum_payload) { - parser_context *pcxt = context->private_data; - manifest_files_hash *ht = pcxt->ht; + manifest_data *manifest = context->private_data; + manifest_files_hash *ht = manifest->files; manifest_file *m; bool found; @@ -508,7 +498,7 @@ verifybackup_per_wal_range_cb(JsonManifestParseContext *context, TimeLineID tli, XLogRecPtr start_lsn, XLogRecPtr end_lsn) { - parser_context *pcxt = context->private_data; + manifest_data *manifest = context->private_data; manifest_wal_range *range; /* Allocate and initialize a struct describing this WAL range. */ @@ -516,15 +506,15 @@ verifybackup_per_wal_range_cb(JsonManifestParseContext *context, range->tli = tli; range->start_lsn = start_lsn; range->end_lsn = end_lsn; - range->prev = pcxt->last_wal_range; + range->prev = manifest->last_wal_range; range->next = NULL; /* Add it to the end of the list. */ - if (pcxt->first_wal_range == NULL) - pcxt->first_wal_range = range; + if (manifest->first_wal_range == NULL) + manifest->first_wal_range = range; else - pcxt->last_wal_range->next = range; - pcxt->last_wal_range = range; + manifest->last_wal_range->next = range; + manifest->last_wal_range = range; } /* @@ -639,7 +629,7 @@ verify_backup_file(verifier_context *context, char *relpath, char *fullpath) } /* Check whether there's an entry in the manifest hash. */ - m = manifest_files_lookup(context->ht, relpath); + m = manifest_files_lookup(context->manifest->files, relpath); if (m == NULL) { report_backup_error(context, @@ -679,11 +669,12 @@ verify_backup_file(verifier_context *context, char *relpath, char *fullpath) static void report_extra_backup_files(verifier_context *context) { + manifest_data *manifest = context->manifest; manifest_files_iterator it; manifest_file *m; - manifest_files_start_iterate(context->ht, &it); - while ((m = manifest_files_iterate(context->ht, &it)) != NULL) + manifest_files_start_iterate(manifest->files, &it); + while ((m = manifest_files_iterate(manifest->files, &it)) != NULL) if (!m->matched && !should_ignore_relpath(context, m->pathname)) report_backup_error(context, "\"%s\" is present in the manifest but not on disk", @@ -698,13 +689,14 @@ report_extra_backup_files(verifier_context *context) static void verify_backup_checksums(verifier_context *context) { + manifest_data *manifest = context->manifest; manifest_files_iterator it; manifest_file *m; progress_report(false); - manifest_files_start_iterate(context->ht, &it); - while ((m = manifest_files_iterate(context->ht, &it)) != NULL) + manifest_files_start_iterate(manifest->files, &it); + while ((m = manifest_files_iterate(manifest->files, &it)) != NULL) { if (should_verify_checksum(m) && !should_ignore_relpath(context, m->pathname)) @@ -833,9 +825,10 @@ verify_file_checksum(verifier_context *context, manifest_file *m, */ static void parse_required_wal(verifier_context *context, char *pg_waldump_path, - char *wal_directory, manifest_wal_range *first_wal_range) + char *wal_directory) { - manifest_wal_range *this_wal_range = first_wal_range; + manifest_data *manifest = context->manifest; + manifest_wal_range *this_wal_range = manifest->first_wal_range; while (this_wal_range != NULL) { -- 2.18.0