Add and use symbolic constants for tar header offsets and file types.
authorRobert Haas <rhaas@postgresql.org>
Tue, 1 Aug 2023 17:50:42 +0000 (13:50 -0400)
committerRobert Haas <rhaas@postgresql.org>
Tue, 1 Aug 2023 17:50:42 +0000 (13:50 -0400)
Because symbolic constants in a header file are better than magic
constants embedded in the code.

Patch by me, reviewed by Tom Lane, Dagfinn Ilmari MannsÃ¥ker, and
Tristan Partin.

Discussion: http://postgr.es/m/CA+TgmoZNbLwhmCrNtkJAvi8FLkwFdMeVU3myV2HQQpA5bvbRZg@mail.gmail.com

src/bin/pg_basebackup/bbstreamer_tar.c
src/bin/pg_basebackup/walmethods.c
src/bin/pg_dump/pg_backup_tar.c
src/include/pgtar.h
src/port/tar.c

index 03d7fd3375a85b80305ee001e6a2b19d30a1ac93..d7b438d0f5920e6701cd06de0c2d3134297ba8f8 100644 (file)
@@ -286,22 +286,20 @@ bbstreamer_tar_header(bbstreamer_tar_parser *mystreamer)
 
    /*
     * Parse key fields out of the header.
-    *
-    * FIXME: It's terrible that we use hard-coded values here instead of some
-    * more principled approach. It's been like this for a long time, but we
-    * ought to do better.
     */
-   strlcpy(member->pathname, &buffer[0], MAXPGPATH);
+   strlcpy(member->pathname, &buffer[TAR_OFFSET_NAME], MAXPGPATH);
    if (member->pathname[0] == '\0')
        pg_fatal("tar member has empty name");
-   member->size = read_tar_number(&buffer[124], 12);
-   member->mode = read_tar_number(&buffer[100], 8);
-   member->uid = read_tar_number(&buffer[108], 8);
-   member->gid = read_tar_number(&buffer[116], 8);
-   member->is_directory = (buffer[156] == '5');
-   member->is_link = (buffer[156] == '2');
+   member->size = read_tar_number(&buffer[TAR_OFFSET_SIZE], 12);
+   member->mode = read_tar_number(&buffer[TAR_OFFSET_MODE], 8);
+   member->uid = read_tar_number(&buffer[TAR_OFFSET_UID], 8);
+   member->gid = read_tar_number(&buffer[TAR_OFFSET_GID], 8);
+   member->is_directory =
+       (buffer[TAR_OFFSET_TYPEFLAG] == TAR_FILETYPE_DIRECTORY);
+   member->is_link =
+       (buffer[TAR_OFFSET_TYPEFLAG] == TAR_FILETYPE_SYMLINK);
    if (member->is_link)
-       strlcpy(member->linktarget, &buffer[157], 100);
+       strlcpy(member->linktarget, &buffer[TAR_OFFSET_LINKNAME], 100);
 
    /* Compute number of padding bytes. */
    mystreamer->pad_bytes_expected = tarPaddingBytesRequired(member->size);
index 376ddf72b7104640cede8dd049cbb00ad5e0af28..d780c4055cd99d64d62b7ec3f7e786bc8e8f7534 100644 (file)
@@ -1131,7 +1131,7 @@ tar_close(Walfile *f, WalCloseMethod method)
     * possibly also renaming the file. We overwrite the entire current header
     * when done, including the checksum.
     */
-   print_tar_number(&(tf->header[124]), 12, filesize);
+   print_tar_number(&(tf->header[TAR_OFFSET_SIZE]), 12, filesize);
 
    if (method == CLOSE_NORMAL)
 
@@ -1139,9 +1139,10 @@ tar_close(Walfile *f, WalCloseMethod method)
         * We overwrite it with what it was before if we have no tempname,
         * since we're going to write the buffer anyway.
         */
-       strlcpy(&(tf->header[0]), tf->base.pathname, 100);
+       strlcpy(&(tf->header[TAR_OFFSET_NAME]), tf->base.pathname, 100);
 
-   print_tar_number(&(tf->header[148]), 8, tarChecksum(((TarMethodFile *) f)->header));
+   print_tar_number(&(tf->header[TAR_OFFSET_CHECKSUM]), 8,
+                    tarChecksum(((TarMethodFile *) f)->header));
    if (lseek(tar_data->fd, tf->ofs_start, SEEK_SET) != ((TarMethodFile *) f)->ofs_start)
    {
        f->wwmethod->lasterrno = errno;
index db5fb43bae8f783b7db29f8c76f2d041ff5d919b..aad88ad559fa8eada78f489860a04bbc70e44c95 100644 (file)
@@ -975,20 +975,20 @@ isValidTarHeader(char *header)
    int         sum;
    int         chk = tarChecksum(header);
 
-   sum = read_tar_number(&header[148], 8);
+   sum = read_tar_number(&header[TAR_OFFSET_CHECKSUM], 8);
 
    if (sum != chk)
        return false;
 
    /* POSIX tar format */
-   if (memcmp(&header[257], "ustar\0", 6) == 0 &&
-       memcmp(&header[263], "00", 2) == 0)
+   if (memcmp(&header[TAR_OFFSET_MAGIC], "ustar\0", 6) == 0 &&
+       memcmp(&header[TAR_OFFSET_VERSION], "00", 2) == 0)
        return true;
    /* GNU tar format */
-   if (memcmp(&header[257], "ustar  \0", 8) == 0)
+   if (memcmp(&header[TAR_OFFSET_MAGIC], "ustar  \0", 8) == 0)
        return true;
    /* not-quite-POSIX format written by pre-9.3 pg_dump */
-   if (memcmp(&header[257], "ustar00\0", 8) == 0)
+   if (memcmp(&header[TAR_OFFSET_MAGIC], "ustar00\0", 8) == 0)
        return true;
 
    return false;
@@ -1151,7 +1151,7 @@ _tarGetHeader(ArchiveHandle *AH, TAR_MEMBER *th)
 
        /* Calc checksum */
        chk = tarChecksum(h);
-       sum = read_tar_number(&h[148], 8);
+       sum = read_tar_number(&h[TAR_OFFSET_CHECKSUM], 8);
 
        /*
         * If the checksum failed, see if it is a null block. If so, silently
@@ -1175,9 +1175,9 @@ _tarGetHeader(ArchiveHandle *AH, TAR_MEMBER *th)
    }
 
    /* Name field is 100 bytes, might not be null-terminated */
-   strlcpy(tag, &h[0], 100 + 1);
+   strlcpy(tag, &h[TAR_OFFSET_NAME], 100 + 1);
 
-   len = read_tar_number(&h[124], 12);
+   len = read_tar_number(&h[TAR_OFFSET_SIZE], 12);
 
    pg_log_debug("TOC Entry %s at %llu (length %llu, checksum %d)",
                 tag, (unsigned long long) hPos, (unsigned long long) len, sum);
index 661f9d7c59f104f16aab03cfe5a648f0e22bd134..8abfb9c19c305c5bf86b0429081585ab858fcd11 100644 (file)
@@ -23,6 +23,45 @@ enum tarError
    TAR_SYMLINK_TOO_LONG
 };
 
+/*
+ * Offsets of fields within a 512-byte tar header.
+ *
+ * "tar number" values should be generated using print_tar_number() and can be
+ * read using read_tar_number(). Fields that contain strings are generally
+ * both filled and read using strlcpy().
+ *
+ * The value for the checksum field can be computed using tarChecksum().
+ *
+ * Some fields are not used by PostgreSQL; see tarCreateHeader().
+ */
+enum tarHeaderOffset
+{
+   TAR_OFFSET_NAME = 0,        /* 100 byte string */
+   TAR_OFFSET_MODE = 100,      /* 8 byte tar number, excludes S_IFMT */
+   TAR_OFFSET_UID = 108,       /* 8 byte tar number */
+   TAR_OFFSET_GID = 116,       /* 8 byte tar number */
+   TAR_OFFSET_SIZE = 124,      /* 8 byte tar number */
+   TAR_OFFSET_MTIME = 136,     /* 12 byte tar number */
+   TAR_OFFSET_CHECKSUM = 148,  /* 8 byte tar number */
+   TAR_OFFSET_TYPEFLAG = 156,  /* 1 byte file type, see TAR_FILETYPE_* */
+   TAR_OFFSET_LINKNAME = 157,  /* 100 byte string */
+   TAR_OFFSET_MAGIC = 257,     /* "ustar" with terminating zero byte */
+   TAR_OFFSET_VERSION = 263,   /* "00" */
+   TAR_OFFSET_UNAME = 265,     /* 32 byte string */
+   TAR_OFFSET_GNAME = 297,     /* 32 byte string */
+   TAR_OFFSET_DEVMAJOR = 329,  /* 8 byte tar number */
+   TAR_OFFSET_DEVMINOR = 337,  /* 8 byte tar number */
+   TAR_OFFSET_PREFIX = 345     /* 155 byte string */
+   /* last 12 bytes of the 512-byte block are unassigned */
+};
+
+enum tarFileType
+{
+   TAR_FILETYPE_PLAIN = '0',
+   TAR_FILETYPE_SYMLINK = '2',
+   TAR_FILETYPE_DIRECTORY = '5'
+};
+
 extern enum tarError tarCreateHeader(char *h, const char *filename,
                                     const char *linktarget, pgoff_t size,
                                     mode_t mode, uid_t uid, gid_t gid,
index 4afe9f2533482b72b413ee81464fb4a7e577bb99..592b4fb7b0f4e08fb9c9efdee6439e54661aa600 100644 (file)
@@ -120,10 +120,10 @@ tarCreateHeader(char *h, const char *filename, const char *linktarget,
    if (linktarget && strlen(linktarget) > 99)
        return TAR_SYMLINK_TOO_LONG;
 
-   memset(h, 0, 512);          /* assume tar header size */
+   memset(h, 0, TAR_BLOCK_SIZE);
 
    /* Name 100 */
-   strlcpy(&h[0], filename, 100);
+   strlcpy(&h[TAR_OFFSET_NAME], filename, 100);
    if (linktarget != NULL || S_ISDIR(mode))
    {
        /*
@@ -139,68 +139,68 @@ tarCreateHeader(char *h, const char *filename, const char *linktarget,
    }
 
    /* Mode 8 - this doesn't include the file type bits (S_IFMT)  */
-   print_tar_number(&h[100], 8, (mode & 07777));
+   print_tar_number(&h[TAR_OFFSET_MODE], 8, (mode & 07777));
 
    /* User ID 8 */
-   print_tar_number(&h[108], 8, uid);
+   print_tar_number(&h[TAR_OFFSET_UID], 8, uid);
 
    /* Group 8 */
-   print_tar_number(&h[116], 8, gid);
+   print_tar_number(&h[TAR_OFFSET_GID], 8, gid);
 
    /* File size 12 */
    if (linktarget != NULL || S_ISDIR(mode))
        /* Symbolic link or directory has size zero */
-       print_tar_number(&h[124], 12, 0);
+       print_tar_number(&h[TAR_OFFSET_SIZE], 12, 0);
    else
-       print_tar_number(&h[124], 12, size);
+       print_tar_number(&h[TAR_OFFSET_SIZE], 12, size);
 
    /* Mod Time 12 */
-   print_tar_number(&h[136], 12, mtime);
+   print_tar_number(&h[TAR_OFFSET_MTIME], 12, mtime);
 
    /* Checksum 8 cannot be calculated until we've filled all other fields */
 
    if (linktarget != NULL)
    {
        /* Type - Symbolic link */
-       h[156] = '2';
+       h[TAR_OFFSET_TYPEFLAG] = TAR_FILETYPE_SYMLINK;
        /* Link Name 100 */
-       strlcpy(&h[157], linktarget, 100);
+       strlcpy(&h[TAR_OFFSET_LINKNAME], linktarget, 100);
    }
    else if (S_ISDIR(mode))
    {
        /* Type - directory */
-       h[156] = '5';
+       h[TAR_OFFSET_TYPEFLAG] = TAR_FILETYPE_DIRECTORY;
    }
    else
    {
        /* Type - regular file */
-       h[156] = '0';
+       h[TAR_OFFSET_TYPEFLAG] = TAR_FILETYPE_PLAIN;
    }
 
    /* Magic 6 */
-   strcpy(&h[257], "ustar");
+   strcpy(&h[TAR_OFFSET_MAGIC], "ustar");
 
    /* Version 2 */
-   memcpy(&h[263], "00", 2);
+   memcpy(&h[TAR_OFFSET_VERSION], "00", 2);
 
    /* User 32 */
    /* XXX: Do we need to care about setting correct username? */
-   strlcpy(&h[265], "postgres", 32);
+   strlcpy(&h[TAR_OFFSET_UNAME], "postgres", 32);
 
    /* Group 32 */
    /* XXX: Do we need to care about setting correct group name? */
-   strlcpy(&h[297], "postgres", 32);
+   strlcpy(&h[TAR_OFFSET_GNAME], "postgres", 32);
 
    /* Major Dev 8 */
-   print_tar_number(&h[329], 8, 0);
+   print_tar_number(&h[TAR_OFFSET_DEVMAJOR], 8, 0);
 
    /* Minor Dev 8 */
-   print_tar_number(&h[337], 8, 0);
+   print_tar_number(&h[TAR_OFFSET_DEVMINOR], 8, 0);
 
    /* Prefix 155 - not used, leave as nulls */
 
    /* Finally, compute and insert the checksum */
-   print_tar_number(&h[148], 8, tarChecksum(h));
+   print_tar_number(&h[TAR_OFFSET_CHECKSUM], 8, tarChecksum(h));
 
    return TAR_OK;
 }