From 395ffe7fef21f18883d97888d1edb01478fe1fb6 Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Mon, 22 Jan 2024 10:45:19 +0530
Subject: [PATCH v5 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 ae8c18f373..8561678a7d 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

