v2-0006-Refactor-split-verify_file_checksum-function.patch

application/octet-stream

Filename: v2-0006-Refactor-split-verify_file_checksum-function.patch
Type: application/octet-stream
Part: 2
Message: Re: pg_verifybackup: TAR format backup verification

Patch

Format: format-patch
Series: patch v2-0006
Subject: Refactor: split verify_file_checksum() function.
File+
src/bin/pg_verifybackup/pg_verifybackup.c 69 33
src/bin/pg_verifybackup/pg_verifybackup.h 4 0
From ce7198153612faf2afc3b1b45d64f331bbd8bae4 Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Mon, 17 Jun 2024 14:22:40 +0530
Subject: [PATCH v2 06/10] Refactor: split verify_file_checksum() function.

Move the core functionality of verify_file_checksum to a new function
to enable incremental checksum computation.
---
 src/bin/pg_verifybackup/pg_verifybackup.c | 102 +++++++++++++++-------
 src/bin/pg_verifybackup/pg_verifybackup.h |   4 +
 2 files changed, 73 insertions(+), 33 deletions(-)

diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c
index d17b565a604..7b845bece71 100644
--- a/src/bin/pg_verifybackup/pg_verifybackup.c
+++ b/src/bin/pg_verifybackup/pg_verifybackup.c
@@ -773,6 +773,72 @@ verify_backup_checksums(verifier_context *context)
 	progress_report(true);
 }
 
+/*
+ * It computes the checksum incrementally for the received bytes, requiring the
+ * caller to pass a properly initialized checksum_ctx parameter. Once the
+ * complete file content is received, which is tracked using the computed_len
+ * parameter, it verifies against the manifest data. If any error occurs, it
+ * returns false; otherwise, it returns true to indicate either the complete
+ * file content is yet to be received or checksum verification is completed
+ * successfully.
+ */
+bool
+verify_content_checksum(verifier_context *context,
+						pg_checksum_context *checksum_ctx,
+						manifest_file *m, uint8 *buffer,
+						int buffer_len, size_t *computed_len)
+{
+	char	   *relpath = m->pathname;
+	uint8		checksumbuf[PG_CHECKSUM_MAX_LENGTH];
+	int			checksumlen;
+
+	if (pg_checksum_update(checksum_ctx, buffer, buffer_len) < 0)
+	{
+		report_backup_error(context, "could not update checksum of file \"%s\"",
+							relpath);
+		return false;
+	}
+
+	/* Update the total count of computed checksum bytes. */
+	*computed_len += buffer_len;
+
+	/* Report progress */
+	done_size += buffer_len;
+	progress_report(false);
+
+	/* Yet to receive the full content of the file. */
+	if (*computed_len < m->size)
+		return true;
+
+	/* Get the final checksum. */
+	checksumlen = pg_checksum_final(checksum_ctx, checksumbuf);
+	if (checksumlen < 0)
+	{
+		report_backup_error(context,
+							"could not finalize checksum of file \"%s\"",
+							relpath);
+		return false;
+	}
+
+	/* And check it against the manifest. */
+	if (checksumlen != m->checksum_length)
+	{
+		report_backup_error(context,
+							"file \"%s\" has checksum of length %d, but expected %d",
+							relpath, m->checksum_length, checksumlen);
+		return false;
+	}
+	else if (memcmp(checksumbuf, m->checksum_payload, checksumlen) != 0)
+	{
+		report_backup_error(context,
+							"checksum mismatch for file \"%s\"",
+							relpath);
+		return false;
+	}
+
+	return true;
+}
+
 /*
  * Verify the checksum of a single file.
  */
@@ -785,8 +851,6 @@ verify_file_checksum(verifier_context *context, manifest_file *m,
 	int			fd;
 	int			rc;
 	size_t		bytes_read = 0;
-	uint8		checksumbuf[PG_CHECKSUM_MAX_LENGTH];
-	int			checksumlen;
 
 	/* Open the target file. */
 	if ((fd = open(fullpath, O_RDONLY | PG_BINARY, 0)) < 0)
@@ -808,19 +872,14 @@ verify_file_checksum(verifier_context *context, manifest_file *m,
 	/* Read the file chunk by chunk, updating the checksum as we go. */
 	while ((rc = read(fd, buffer, READ_CHUNK_SIZE)) > 0)
 	{
-		bytes_read += rc;
-		if (pg_checksum_update(&checksum_ctx, buffer, rc) < 0)
+		if (!verify_content_checksum(context, &checksum_ctx, m, buffer, rc,
+									 &bytes_read))
 		{
-			report_backup_error(context, "could not update checksum of file \"%s\"",
-								relpath);
 			close(fd);
 			return;
 		}
-
-		/* Report progress */
-		done_size += rc;
-		progress_report(false);
 	}
+
 	if (rc < 0)
 		report_backup_error(context, "could not read file \"%s\": %m",
 							relpath);
@@ -845,32 +904,9 @@ verify_file_checksum(verifier_context *context, manifest_file *m,
 	 * filesystem misbehavior.
 	 */
 	if (bytes_read != m->size)
-	{
 		report_backup_error(context,
 							"file \"%s\" should contain %zu bytes, but read %zu bytes",
 							relpath, m->size, bytes_read);
-		return;
-	}
-
-	/* Get the final checksum. */
-	checksumlen = pg_checksum_final(&checksum_ctx, checksumbuf);
-	if (checksumlen < 0)
-	{
-		report_backup_error(context,
-							"could not finalize checksum of file \"%s\"",
-							relpath);
-		return;
-	}
-
-	/* And check it against the manifest. */
-	if (checksumlen != m->checksum_length)
-		report_backup_error(context,
-							"file \"%s\" has checksum of length %d, but expected %d",
-							relpath, m->checksum_length, checksumlen);
-	else if (memcmp(checksumbuf, m->checksum_payload, checksumlen) != 0)
-		report_backup_error(context,
-							"checksum mismatch for file \"%s\"",
-							relpath);
 }
 
 /*
diff --git a/src/bin/pg_verifybackup/pg_verifybackup.h b/src/bin/pg_verifybackup/pg_verifybackup.h
index c11ff33a100..50a285752aa 100644
--- a/src/bin/pg_verifybackup/pg_verifybackup.h
+++ b/src/bin/pg_verifybackup/pg_verifybackup.h
@@ -101,6 +101,10 @@ typedef struct verifier_context
 
 extern manifest_file *verify_manifest_entry(verifier_context *context,
 											char *relpath, size_t filesize);
+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 report_backup_error(verifier_context *context,
 								const char *pg_restrict fmt,...)
-- 
2.18.0