From f9b84f54654400c0d40d2692ed45ee123d3818f6 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Fri, 24 Apr 2020 10:38:10 -0400 Subject: [PATCH 2/2] 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 fbdc28ec39..8b332d110d 100644 --- a/src/backend/replication/basebackup.c +++ b/src/backend/replication/basebackup.c @@ -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) diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index c1d3d8624b..0b7660d15b 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(©buf[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 775118f297..d5ed7cce68 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)