Assorted cleanup of tar-related code.
authorRobert Haas <rhaas@postgresql.org>
Fri, 24 Apr 2020 14:38:10 +0000 (10:38 -0400)
committerRobert Haas <rhaas@postgresql.org>
Mon, 15 Jun 2020 19:28:49 +0000 (15:28 -0400)
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.

Discussion: http://postgr.es/m/CA+TgmobWbfReO9-XFk8urR1K4wTNwqoHx_v56t7=T8KaiEoKNw@mail.gmail.com

src/backend/replication/basebackup.c
src/bin/pg_basebackup/pg_basebackup.c
src/bin/pg_basebackup/walmethods.c
src/bin/pg_dump/pg_backup_tar.c
src/include/pgtar.h

index 3b46bfe9ab03c2b2c374ec1fdc29c69d719d4140..100828206550c65a002c81fdec56a5cd38dc3ad6 100644 (file)
@@ -665,7 +665,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);
 
@@ -1118,11 +1122,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);
@@ -1491,9 +1495,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
@@ -1784,11 +1793,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);
@@ -1822,7 +1831,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)
index b81b5679ec31dc076306bfcbe8fec8f4bb225661..256c0c074c7522a51885157f3b5afcee578dc8df 100644 (file)
@@ -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));
@@ -1169,7 +1169,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,
@@ -1188,7 +1188,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,
@@ -1224,7 +1224,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);
@@ -1242,7 +1242,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
@@ -1303,9 +1303,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;
@@ -1318,17 +1318,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
-                                        * in whole by the last PQgetCopyData call.
+                                        * 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,
@@ -1361,14 +1361,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,
@@ -1388,7 +1388,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);
                                                }
                                        }
 
@@ -1425,15 +1425,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);
@@ -1551,12 +1551,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);
 
@@ -1566,10 +1566,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
index ecff08740ce3c38edee9a9f77511515ea356f63c..bd1947d623feff8982f789d78376f7feeba7e14e 100644 (file)
@@ -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 */
index d5bfa5564615207c544b8e2a0675c166ccb63277..b4f59429592701080e746e440035c5e90b582f78 100644 (file)
@@ -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;
 }
index 0a875903a71207878b79be056e96bdd98be331d1..0f08dc0c2ca88ddf860de1bf3ba43cfd7bc4996c 100644 (file)
  *
  *-------------------------------------------------------------------------
  */
+#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