From 3d7c41530d5c3c81162becf95c12b2babdbec113 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Fri, 24 Apr 2020 10:38:10 -0400
Subject: [PATCH v1 03/11] Assorted cleanup of tar-related code.

Introduce TAR_BLOCK_SIZE and replace many instances of 512 with
the new constant. Introduce function tarPaddingBytesRequired
and use it to replace numerous repetitions of (x + 511) & ~511.

Add preprocessor guards against multiple inclusion to pgtar.h.

Reformat the prototype for tarCreateHeader so it doesn't extend
beyond 80 characters.
---
 src/backend/replication/basebackup.c  | 29 ++++++++++++------
 src/bin/pg_basebackup/pg_basebackup.c | 44 +++++++++++++--------------
 src/bin/pg_basebackup/walmethods.c    | 25 +++++++++------
 src/bin/pg_dump/pg_backup_tar.c       | 30 +++++++++---------
 src/include/pgtar.h                   | 23 ++++++++++++--
 5 files changed, 93 insertions(+), 58 deletions(-)

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 084db4b2e5..54a746abde 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -670,7 +670,11 @@ perform_base_backup(basebackup_options *opt)
 						 errmsg("unexpected WAL file size \"%s\"", walFileName)));
 			}
 
-			/* wal_segment_size is a multiple of 512, so no need for padding */
+			/*
+			 * wal_segment_size is a multiple of TAR_BLOCK_SIZE, so no need
+			 * for padding.
+			 */
+			Assert(wal_segment_size % TAR_BLOCK_SIZE == 0);
 
 			FreeFile(fp);
 
@@ -1123,11 +1127,11 @@ sendFileWithContent(const char *filename, const char *content,
 	pq_putmessage('d', content, len);
 	update_basebackup_progress(len);
 
-	/* Pad to 512 byte boundary, per tar format requirements */
-	pad = ((len + 511) & ~511) - len;
+	/* Pad to a multiple of the tar block size. */
+	pad = tarPaddingBytesRequired(len);
 	if (pad > 0)
 	{
-		char		buf[512];
+		char		buf[TAR_BLOCK_SIZE];
 
 		MemSet(buf, 0, pad);
 		pq_putmessage('d', buf, pad);
@@ -1496,9 +1500,14 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
 
 			if (sent || sizeonly)
 			{
-				/* Add size, rounded up to 512byte block */
-				size += ((statbuf.st_size + 511) & ~511);
-				size += 512;	/* Size of the header of the file */
+				/* Add size. */
+				size += statbuf.st_size;
+
+				/* Pad to a multiple of the tar block size. */
+				size += tarPaddingBytesRequired(statbuf.st_size);
+
+				/* Size of the header for the file. */
+				size += TAR_BLOCK_SIZE;
 			}
 		}
 		else
@@ -1789,11 +1798,11 @@ sendFile(const char *readfilename, const char *tarfilename,
 	}
 
 	/*
-	 * Pad to 512 byte boundary, per tar format requirements. (This small
+	 * Pad to a block boundary, per tar format requirements. (This small
 	 * piece of data is probably not worth throttling, and is not checksummed
 	 * because it's not actually part of the file.)
 	 */
-	pad = ((len + 511) & ~511) - len;
+	pad = tarPaddingBytesRequired(len);
 	if (pad > 0)
 	{
 		MemSet(buf, 0, pad);
@@ -1827,7 +1836,7 @@ static int64
 _tarWriteHeader(const char *filename, const char *linktarget,
 				struct stat *statbuf, bool sizeonly)
 {
-	char		h[512];
+	char		h[TAR_BLOCK_SIZE];
 	enum tarError rc;
 
 	if (!sizeonly)
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 2e9035d613..29407d5644 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -62,7 +62,7 @@ typedef struct WriteTarState
 	int			tablespacenum;
 	char		filename[MAXPGPATH];
 	FILE	   *tarfile;
-	char		tarhdr[512];
+	char		tarhdr[TAR_BLOCK_SIZE];
 	bool		basetablespace;
 	bool		in_tarhdr;
 	bool		skip_file;
@@ -1024,7 +1024,7 @@ writeTarData(WriteTarState *state, char *buf, int r)
 static void
 ReceiveTarFile(PGconn *conn, PGresult *res, int rownum)
 {
-	char		zerobuf[1024];
+	char		zerobuf[TAR_BLOCK_SIZE * 2];
 	WriteTarState state;
 
 	memset(&state, 0, sizeof(state));
@@ -1168,7 +1168,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum)
 
 	if (state.basetablespace && writerecoveryconf)
 	{
-		char		header[512];
+		char		header[TAR_BLOCK_SIZE];
 
 		/*
 		 * If postgresql.auto.conf has not been found in the streamed data,
@@ -1187,7 +1187,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum)
 							pg_file_create_mode, 04000, 02000,
 							time(NULL));
 
-			padding = ((recoveryconfcontents->len + 511) & ~511) - recoveryconfcontents->len;
+			padding = tarPaddingBytesRequired(recoveryconfcontents->len);
 
 			writeTarData(&state, header, sizeof(header));
 			writeTarData(&state, recoveryconfcontents->data,
@@ -1223,7 +1223,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum)
 	 */
 	if (strcmp(basedir, "-") == 0 && manifest)
 	{
-		char		header[512];
+		char		header[TAR_BLOCK_SIZE];
 		PQExpBufferData	buf;
 
 		initPQExpBuffer(&buf);
@@ -1241,7 +1241,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum)
 		termPQExpBuffer(&buf);
 	}
 
-	/* 2 * 512 bytes empty data at end of file */
+	/* 2 * TAR_BLOCK_SIZE bytes empty data at end of file */
 	writeTarData(&state, zerobuf, sizeof(zerobuf));
 
 #ifdef HAVE_LIBZ
@@ -1302,9 +1302,9 @@ ReceiveTarCopyChunk(size_t r, char *copybuf, void *callback_data)
 		 *
 		 * To do this, we have to process the individual files inside the TAR
 		 * stream. The stream consists of a header and zero or more chunks,
-		 * all 512 bytes long. The stream from the server is broken up into
-		 * smaller pieces, so we have to track the size of the files to find
-		 * the next header structure.
+		 * each with a length equal to TAR_BLOCK_SIZE. The stream from the
+		 * server is broken up into smaller pieces, so we have to track the
+		 * size of the files to find the next header structure.
 		 */
 		int			rr = r;
 		int			pos = 0;
@@ -1317,17 +1317,17 @@ ReceiveTarCopyChunk(size_t r, char *copybuf, void *callback_data)
 				 * We're currently reading a header structure inside the TAR
 				 * stream, i.e. the file metadata.
 				 */
-				if (state->tarhdrsz < 512)
+				if (state->tarhdrsz < TAR_BLOCK_SIZE)
 				{
 					/*
 					 * Copy the header structure into tarhdr in case the
-					 * header is not aligned to 512 bytes or it's not returned
+					 * header is not aligned properly or it's not returned
 					 * in whole by the last PQgetCopyData call.
 					 */
 					int			hdrleft;
 					int			bytes2copy;
 
-					hdrleft = 512 - state->tarhdrsz;
+					hdrleft = TAR_BLOCK_SIZE - state->tarhdrsz;
 					bytes2copy = (rr > hdrleft ? hdrleft : rr);
 
 					memcpy(&state->tarhdr[state->tarhdrsz], copybuf + pos,
@@ -1360,14 +1360,14 @@ ReceiveTarCopyChunk(size_t r, char *copybuf, void *callback_data)
 
 					state->filesz = read_tar_number(&state->tarhdr[124], 12);
 					state->file_padding_len =
-						((state->filesz + 511) & ~511) - state->filesz;
+						tarPaddingBytesRequired(state->filesz);
 
 					if (state->is_recovery_guc_supported &&
 						state->is_postgresql_auto_conf &&
 						writerecoveryconf)
 					{
 						/* replace tar header */
-						char		header[512];
+						char		header[TAR_BLOCK_SIZE];
 
 						tarCreateHeader(header, "postgresql.auto.conf", NULL,
 										state->filesz + recoveryconfcontents->len,
@@ -1387,7 +1387,7 @@ ReceiveTarCopyChunk(size_t r, char *copybuf, void *callback_data)
 							 * If we're not skipping the file, write the tar
 							 * header unmodified.
 							 */
-							writeTarData(state, state->tarhdr, 512);
+							writeTarData(state, state->tarhdr, TAR_BLOCK_SIZE);
 						}
 					}
 
@@ -1424,15 +1424,15 @@ ReceiveTarCopyChunk(size_t r, char *copybuf, void *callback_data)
 					int			padding;
 					int			tailsize;
 
-					tailsize = (512 - state->file_padding_len) + recoveryconfcontents->len;
-					padding = ((tailsize + 511) & ~511) - tailsize;
+					tailsize = (TAR_BLOCK_SIZE - state->file_padding_len) + recoveryconfcontents->len;
+					padding = tarPaddingBytesRequired(tailsize);
 
 					writeTarData(state, recoveryconfcontents->data,
 								 recoveryconfcontents->len);
 
 					if (padding)
 					{
-						char		zerobuf[512];
+						char		zerobuf[TAR_BLOCK_SIZE];
 
 						MemSet(zerobuf, 0, sizeof(zerobuf));
 						writeTarData(state, zerobuf, padding);
@@ -1550,12 +1550,12 @@ ReceiveTarAndUnpackCopyChunk(size_t r, char *copybuf, void *callback_data)
 		/*
 		 * No current file, so this must be the header for a new file
 		 */
-		if (r != 512)
+		if (r != TAR_BLOCK_SIZE)
 		{
 			pg_log_error("invalid tar block header size: %zu", r);
 			exit(1);
 		}
-		totaldone += 512;
+		totaldone += TAR_BLOCK_SIZE;
 
 		state->current_len_left = read_tar_number(&copybuf[124], 12);
 
@@ -1565,10 +1565,10 @@ ReceiveTarAndUnpackCopyChunk(size_t r, char *copybuf, void *callback_data)
 #endif
 
 		/*
-		 * All files are padded up to 512 bytes
+		 * All files are padded up to a multiple of TAR_BLOCK_SIZE
 		 */
 		state->current_padding =
-			((state->current_len_left + 511) & ~511) - state->current_len_left;
+			tarPaddingBytesRequired(state->current_len_left);
 
 		/*
 		 * First part of header is zero terminated filename
diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
index ecff08740c..bd1947d623 100644
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -386,7 +386,7 @@ typedef struct TarMethodFile
 {
 	off_t		ofs_start;		/* Where does the *header* for this file start */
 	off_t		currpos;
-	char		header[512];
+	char		header[TAR_BLOCK_SIZE];
 	char	   *pathname;
 	size_t		pad_to_size;
 } TarMethodFile;
@@ -625,7 +625,8 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 	if (!tar_data->compression)
 	{
 		errno = 0;
-		if (write(tar_data->fd, tar_data->currentfile->header, 512) != 512)
+		if (write(tar_data->fd, tar_data->currentfile->header,
+				  TAR_BLOCK_SIZE) != TAR_BLOCK_SIZE)
 		{
 			save_errno = errno;
 			pg_free(tar_data->currentfile);
@@ -639,7 +640,8 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 	else
 	{
 		/* Write header through the zlib APIs but with no compression */
-		if (!tar_write_compressed_data(tar_data->currentfile->header, 512, true))
+		if (!tar_write_compressed_data(tar_data->currentfile->header,
+									   TAR_BLOCK_SIZE, true))
 			return NULL;
 
 		/* Re-enable compression for the rest of the file */
@@ -665,7 +667,9 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 			/* Uncompressed, so pad now */
 			tar_write_padding_data(tar_data->currentfile, pad_to_size);
 			/* Seek back to start */
-			if (lseek(tar_data->fd, tar_data->currentfile->ofs_start + 512, SEEK_SET) != tar_data->currentfile->ofs_start + 512)
+			if (lseek(tar_data->fd,
+					  tar_data->currentfile->ofs_start + TAR_BLOCK_SIZE,
+					  SEEK_SET) != tar_data->currentfile->ofs_start + TAR_BLOCK_SIZE)
 				return NULL;
 
 			tar_data->currentfile->currpos = 0;
@@ -778,14 +782,14 @@ tar_close(Walfile f, WalCloseMethod method)
 	}
 
 	/*
-	 * Get the size of the file, and pad the current data up to the nearest
-	 * 512 byte boundary.
+	 * Get the size of the file, and pad out to a multiple of the tar block
+	 * size.
 	 */
 	filesize = tar_get_current_pos(f);
-	padding = ((filesize + 511) & ~511) - filesize;
+	padding = tarPaddingBytesRequired(filesize);
 	if (padding)
 	{
-		char		zerobuf[512];
+		char		zerobuf[TAR_BLOCK_SIZE];
 
 		MemSet(zerobuf, 0, padding);
 		if (tar_write(f, zerobuf, padding) != padding)
@@ -826,7 +830,7 @@ tar_close(Walfile f, WalCloseMethod method)
 	if (!tar_data->compression)
 	{
 		errno = 0;
-		if (write(tar_data->fd, tf->header, 512) != 512)
+		if (write(tar_data->fd, tf->header, TAR_BLOCK_SIZE) != TAR_BLOCK_SIZE)
 		{
 			/* if write didn't set errno, assume problem is no disk space */
 			if (errno == 0)
@@ -845,7 +849,8 @@ tar_close(Walfile f, WalCloseMethod method)
 		}
 
 		/* Overwrite the header, assuming the size will be the same */
-		if (!tar_write_compressed_data(tar_data->currentfile->header, 512, true))
+		if (!tar_write_compressed_data(tar_data->currentfile->header,
+									   TAR_BLOCK_SIZE, true))
 			return -1;
 
 		/* Turn compression back on */
diff --git a/src/bin/pg_dump/pg_backup_tar.c b/src/bin/pg_dump/pg_backup_tar.c
index d5bfa55646..b4f5942959 100644
--- a/src/bin/pg_dump/pg_backup_tar.c
+++ b/src/bin/pg_dump/pg_backup_tar.c
@@ -893,7 +893,7 @@ _CloseArchive(ArchiveHandle *AH)
 		/*
 		 * EOF marker for tar files is two blocks of NULLs.
 		 */
-		for (i = 0; i < 512 * 2; i++)
+		for (i = 0; i < TAR_BLOCK_SIZE * 2; i++)
 		{
 			if (fputc(0, ctx->tarFH) == EOF)
 				WRITE_ERROR_EXIT;
@@ -1113,7 +1113,7 @@ _tarAddFile(ArchiveHandle *AH, TAR_MEMBER *th)
 			  buf1, buf2);
 	}
 
-	pad = ((len + 511) & ~511) - len;
+	pad = tarPaddingBytesRequired(len);
 	for (i = 0; i < pad; i++)
 	{
 		if (fputc('\0', th->tarFH) == EOF)
@@ -1130,7 +1130,7 @@ _tarPositionTo(ArchiveHandle *AH, const char *filename)
 	lclContext *ctx = (lclContext *) AH->formatData;
 	TAR_MEMBER *th = pg_malloc0(sizeof(TAR_MEMBER));
 	char		c;
-	char		header[512];
+	char		header[TAR_BLOCK_SIZE];
 	size_t		i,
 				len,
 				blks;
@@ -1189,17 +1189,19 @@ _tarPositionTo(ArchiveHandle *AH, const char *filename)
 				  th->targetFile, filename);
 
 		/* Header doesn't match, so read to next header */
-		len = ((th->fileLen + 511) & ~511); /* Padded length */
-		blks = len >> 9;		/* # of 512 byte blocks */
+		len = th->fileLen;
+		len += tarPaddingBytesRequired(th->fileLen);
+		blks = len / TAR_BLOCK_SIZE;		/* # of tar blocks */
 
 		for (i = 0; i < blks; i++)
-			_tarReadRaw(AH, &header[0], 512, NULL, ctx->tarFH);
+			_tarReadRaw(AH, &header[0], TAR_BLOCK_SIZE, NULL, ctx->tarFH);
 
 		if (!_tarGetHeader(AH, th))
 			fatal("could not find header for file \"%s\" in tar archive", filename);
 	}
 
-	ctx->tarNextMember = ctx->tarFHpos + ((th->fileLen + 511) & ~511);
+	ctx->tarNextMember = ctx->tarFHpos + th->fileLen
+		+ tarPaddingBytesRequired(th->fileLen);
 	th->pos = 0;
 
 	return th;
@@ -1210,7 +1212,7 @@ static int
 _tarGetHeader(ArchiveHandle *AH, TAR_MEMBER *th)
 {
 	lclContext *ctx = (lclContext *) AH->formatData;
-	char		h[512];
+	char		h[TAR_BLOCK_SIZE];
 	char		tag[100 + 1];
 	int			sum,
 				chk;
@@ -1223,12 +1225,12 @@ _tarGetHeader(ArchiveHandle *AH, TAR_MEMBER *th)
 		/* Save the pos for reporting purposes */
 		hPos = ctx->tarFHpos;
 
-		/* Read a 512 byte block, return EOF, exit if short */
-		len = _tarReadRaw(AH, h, 512, NULL, ctx->tarFH);
+		/* Read the next tar block, return EOF, exit if short */
+		len = _tarReadRaw(AH, h, TAR_BLOCK_SIZE, NULL, ctx->tarFH);
 		if (len == 0)			/* EOF */
 			return 0;
 
-		if (len != 512)
+		if (len != TAR_BLOCK_SIZE)
 			fatal(ngettext("incomplete tar header found (%lu byte)",
 						   "incomplete tar header found (%lu bytes)",
 						   len),
@@ -1248,7 +1250,7 @@ _tarGetHeader(ArchiveHandle *AH, TAR_MEMBER *th)
 		{
 			int			i;
 
-			for (i = 0; i < 512; i++)
+			for (i = 0; i < TAR_BLOCK_SIZE; i++)
 			{
 				if (h[i] != 0)
 				{
@@ -1294,12 +1296,12 @@ _tarGetHeader(ArchiveHandle *AH, TAR_MEMBER *th)
 static void
 _tarWriteHeader(TAR_MEMBER *th)
 {
-	char		h[512];
+	char		h[TAR_BLOCK_SIZE];
 
 	tarCreateHeader(h, th->targetFile, NULL, th->fileLen,
 					0600, 04000, 02000, time(NULL));
 
 	/* Now write the completed header. */
-	if (fwrite(h, 1, 512, th->tarFH) != 512)
+	if (fwrite(h, 1, TAR_BLOCK_SIZE, th->tarFH) != TAR_BLOCK_SIZE)
 		WRITE_ERROR_EXIT;
 }
diff --git a/src/include/pgtar.h b/src/include/pgtar.h
index 0a875903a7..0f08dc0c2c 100644
--- a/src/include/pgtar.h
+++ b/src/include/pgtar.h
@@ -11,6 +11,10 @@
  *
  *-------------------------------------------------------------------------
  */
+#ifndef PG_TAR_H
+#define PG_TAR_H
+
+#define		TAR_BLOCK_SIZE	512
 
 enum tarError
 {
@@ -19,8 +23,23 @@ enum tarError
 	TAR_SYMLINK_TOO_LONG
 };
 
-extern enum tarError tarCreateHeader(char *h, const char *filename, const char *linktarget,
-									 pgoff_t size, mode_t mode, uid_t uid, gid_t gid, time_t mtime);
+extern enum tarError tarCreateHeader(char *h, const char *filename,
+									 const char *linktarget, pgoff_t size,
+									 mode_t mode, uid_t uid, gid_t gid,
+									 time_t mtime);
 extern uint64 read_tar_number(const char *s, int len);
 extern void print_tar_number(char *s, int len, uint64 val);
 extern int	tarChecksum(char *header);
+
+/*
+ * Compute the number of padding bytes required for an entry in a tar
+ * archive. We must pad out to a multiple of TAR_BLOCK_SIZE. Since that's
+ * a power of 2, we can use TYPEALIGN().
+ */
+static inline size_t
+tarPaddingBytesRequired(size_t len)
+{
+	return TYPEALIGN(TAR_BLOCK_SIZE, len) - len;
+}
+
+#endif
-- 
2.24.2 (Apple Git-127)

