LibArchive: Refactor Tar to make DRY (Don't Repeat Yourself)

Problem:
- The getters and setters duplicate code for conversions.
- Getters are returning `const StringView` rather than non-`const`.

Solution:
- Factor out common code to helper functions.
- Return `StringView` as non-`const`.

2 files changed, 72 insertions, 50 deletions

diff --git a/Userland/Libraries/LibArchive/Tar.h b/Userland/Libraries/LibArchive/Tar.h
index 95fb48acfa..5ed6822b91 100644
--- a/Userland/Libraries/LibArchive/Tar.h
+++ b/Userland/Libraries/LibArchive/Tar.h
@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2020, Peter Elliott <pelliott@serenityos.org>
  * Copyright (c) 2021, Idan Horowitz <idan.horowitz@serenityos.org>
+ * Copyright (c) 2022, the SerenityOS developers.
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -36,42 +37,81 @@ constexpr const char* ustar_version = "00";    // ustar format version
 constexpr const char* posix1_tar_magic = "";   // POSIX.1-1988 format magic
 constexpr const char* posix1_tar_version = ""; // POSIX.1-1988 format version
 
+template<size_t N>
+static size_t get_field_as_integral(const char (&field)[N])
+{
+    size_t value = 0;
+    for (size_t i = 0; i < N; ++i) {
+        if (field[i] == 0)
+            break;
+
+        VERIFY(field[i] >= '0' && field[i] <= '7');
+        value *= 8;
+        value += field[i] - '0';
+    }
+    return value;
+}
+
+template<size_t N>
+static StringView get_field_as_string_view(const char (&field)[N])
+{
+    return { field, min(__builtin_strlen(field), N) };
+}
+
+template<size_t N, class TSource>
+static void set_field(char (&field)[N], TSource&& source)
+{
+    if constexpr (requires { source.copy_characters_to_buffer(field, N); }) {
+        VERIFY(source.copy_characters_to_buffer(field, N));
+    } else {
+        memcpy(field, source.characters_without_null_termination(), min(N, source.length()));
+    }
+}
+
+template<class TSource, size_t N>
+static void set_octal_field(char (&field)[N], TSource&& source)
+{
+    set_field(field, String::formatted("{:o}", forward<TSource>(source)));
+}
+
 class [[gnu::packed]] TarFileHeader {
 public:
-    const StringView filename() const { return StringView(m_filename, min(__builtin_strlen(m_filename), sizeof(m_filename))); }
-    mode_t mode() const { return get_tar_field(m_mode); }
-    uid_t uid() const { return get_tar_field(m_uid); }
-    gid_t gid() const { return get_tar_field(m_gid); }
+    StringView filename() const { return get_field_as_string_view(m_filename); }
+    mode_t mode() const { return get_field_as_integral(m_mode); }
+    uid_t uid() const { return get_field_as_integral(m_uid); }
+    gid_t gid() const { return get_field_as_integral(m_gid); }
     // FIXME: support 2001-star size encoding
-    size_t size() const { return get_tar_field(m_size); }
-    time_t timestamp() const { return get_tar_field(m_timestamp); }
-    unsigned checksum() const { return get_tar_field(m_checksum); }
+    size_t size() const { return get_field_as_integral(m_size); }
+    time_t timestamp() const { return get_field_as_integral(m_timestamp); }
+    unsigned checksum() const { return get_field_as_integral(m_checksum); }
     TarFileType type_flag() const { return TarFileType(m_type_flag); }
-    const StringView link_name() const { return m_link_name; }
-    const StringView magic() const { return StringView(m_magic, min(__builtin_strlen(m_magic), sizeof(m_magic))); }
-    const StringView version() const { return StringView(m_version, min(__builtin_strlen(m_version), sizeof(m_version))); }
-    const StringView owner_name() const { return StringView(m_owner_name, min(__builtin_strlen(m_owner_name), sizeof(m_owner_name))); }
-    const StringView group_name() const { return StringView(m_group_name, min(__builtin_strlen(m_group_name), sizeof(m_group_name))); }
-    int major() const { return get_tar_field(m_major); }
-    int minor() const { return get_tar_field(m_minor); }
+    StringView link_name() const { return m_link_name; }
+    StringView magic() const { return get_field_as_string_view(m_magic); }
+    StringView version() const { return get_field_as_string_view(m_version); }
+    StringView owner_name() const { return get_field_as_string_view(m_owner_name); }
+    StringView group_name() const { return get_field_as_string_view(m_group_name); }
+    int major() const { return get_field_as_integral(m_major); }
+    int minor() const { return get_field_as_integral(m_minor); }
     // FIXME: support ustar filename prefix
-    const StringView prefix() const { return StringView(m_prefix, min(__builtin_strlen(m_prefix), sizeof(m_prefix))); }
+    StringView prefix() const { return get_field_as_string_view(m_prefix); }
 
-    void set_filename(const String& filename) { VERIFY(filename.copy_characters_to_buffer(m_filename, sizeof(m_filename))); }
-    void set_mode(mode_t mode) { VERIFY(String::formatted("{:o}", mode).copy_characters_to_buffer(m_mode, sizeof(m_mode))); }
-    void set_uid(uid_t uid) { VERIFY(String::formatted("{:o}", uid).copy_characters_to_buffer(m_uid, sizeof(m_uid))); }
-    void set_gid(gid_t gid) { VERIFY(String::formatted("{:o}", gid).copy_characters_to_buffer(m_gid, sizeof(m_gid))); }
-    void set_size(size_t size) { VERIFY(String::formatted("{:o}", size).copy_characters_to_buffer(m_size, sizeof(m_size))); }
-    void set_timestamp(time_t timestamp) { VERIFY(String::formatted("{:o}", timestamp).copy_characters_to_buffer(m_timestamp, sizeof(m_timestamp))); }
-    void set_type_flag(TarFileType type) { m_type_flag = static_cast<char>(type); }
-    void set_link_name(const String& link_name) { VERIFY(link_name.copy_characters_to_buffer(m_link_name, sizeof(m_link_name))); }
-    void set_magic(const char* magic) { memcpy(m_magic, magic, sizeof(m_magic)); }           // magic doesn't necessarily include a null byte
-    void set_version(const char* version) { memcpy(m_version, version, sizeof(m_version)); } // version doesn't necessarily include a null byte
-    void set_owner_name(const String& owner_name) { VERIFY(owner_name.copy_characters_to_buffer(m_owner_name, sizeof(m_owner_name))); }
-    void set_group_name(const String& group_name) { VERIFY(group_name.copy_characters_to_buffer(m_group_name, sizeof(m_group_name))); }
-    void set_major(int major) { VERIFY(String::formatted("{:o}", major).copy_characters_to_buffer(m_major, sizeof(m_major))); }
-    void set_minor(int minor) { VERIFY(String::formatted("{:o}", minor).copy_characters_to_buffer(m_minor, sizeof(m_minor))); }
-    void set_prefix(const String& prefix) { VERIFY(prefix.copy_characters_to_buffer(m_prefix, sizeof(m_prefix))); }
+    void set_filename(const String& filename) { set_field(m_filename, filename); }
+    void set_mode(mode_t mode) { set_octal_field(m_mode, mode); }
+    void set_uid(uid_t uid) { set_octal_field(m_uid, uid); }
+    void set_gid(gid_t gid) { set_octal_field(m_gid, gid); }
+    void set_size(size_t size) { set_octal_field(m_size, size); }
+    void set_timestamp(time_t timestamp) { set_octal_field(m_timestamp, timestamp); }
+    void set_type_flag(TarFileType type) { m_type_flag = to_underlying(type); }
+    void set_link_name(const String& link_name) { set_field(m_link_name, link_name); }
+    // magic doesn't necessarily include a null byte
+    void set_magic(StringView magic) { set_field(m_magic, magic); }
+    // version doesn't necessarily include a null byte
+    void set_version(StringView version) { set_field(m_version, version); }
+    void set_owner_name(const String& owner_name) { set_field(m_owner_name, owner_name); }
+    void set_group_name(const String& group_name) { set_field(m_group_name, group_name); }
+    void set_major(int major) { set_octal_field(m_major, major); }
+    void set_minor(int minor) { set_octal_field(m_minor, minor); }
+    void set_prefix(const String& prefix) { set_field(m_prefix, prefix); }
 
     unsigned expected_checksum() const;
     void calculate_checksum();
@@ -93,24 +133,6 @@ private:
     char m_major[8];
     char m_minor[8];
     char m_prefix[155]; // zero out the prefix for archiving
-
-    template<size_t N>
-    static size_t get_tar_field(const char (&field)[N]);
 };
 
-template<size_t N>
-size_t TarFileHeader::get_tar_field(const char (&field)[N])
-{
-    size_t value = 0;
-    for (size_t i = 0; i < N; ++i) {
-        if (field[i] == 0)
-            break;
-
-        VERIFY(field[i] >= '0' && field[i] <= '7');
-        value *= 8;
-        value += field[i] - '0';
-    }
-    return value;
-}
-
 }
diff --git a/Userland/Libraries/LibArchive/TarStream.cpp b/Userland/Libraries/LibArchive/TarStream.cpp
index b23a9d1a23..a98a5ccf10 100644
--- a/Userland/Libraries/LibArchive/TarStream.cpp
+++ b/Userland/Libraries/LibArchive/TarStream.cpp
@@ -103,8 +103,8 @@ void TarInputStream::advance()
 
 bool TarInputStream::valid() const
 {
-    auto& header_magic = header().magic();
-    auto& header_version = header().version();
+    auto const header_magic = header().magic();
+    auto const header_version = header().version();
 
     if (!((header_magic == gnu_magic && header_version == gnu_version)
             || (header_magic == ustar_magic && header_version == ustar_version)