From c0735b047e09aea0e6890a30aa6169f2eaea5a04 Mon Sep 17 00:00:00 2001 From: Lenny Maiorani Date: Wed, 16 Feb 2022 16:06:28 -0700 Subject: [PATCH] LibArchive: Use designated initializers Benefits: - Braced-initialization prevents unknown narrowing conversions. - Using designated initializers will result in a compiler error when a member is skipped or forgotten. --- Userland/Libraries/LibArchive/Zip.cpp | 115 ++++++++++++++------------ Userland/Libraries/LibArchive/Zip.h | 10 ++- 2 files changed, 70 insertions(+), 55 deletions(-) diff --git a/Userland/Libraries/LibArchive/Zip.cpp b/Userland/Libraries/LibArchive/Zip.cpp index 8347e8c97d2..01097365cfa 100644 --- a/Userland/Libraries/LibArchive/Zip.cpp +++ b/Userland/Libraries/LibArchive/Zip.cpp @@ -68,17 +68,17 @@ Optional Zip::try_create(ReadonlyBytes buffer) member_offset += central_directory_record.size(); } - Zip zip; - zip.m_input_data = buffer; - zip.member_count = end_of_central_directory.total_records_count; - zip.members_start_offset = end_of_central_directory.central_directory_offset; - return zip; + return Zip { + end_of_central_directory.total_records_count, + end_of_central_directory.central_directory_offset, + buffer, + }; } bool Zip::for_each_member(Function callback) { - size_t member_offset = members_start_offset; - for (size_t i = 0; i < member_count; i++) { + size_t member_offset = m_members_start_offset; + for (size_t i = 0; i < m_member_count; i++) { CentralDirectoryRecord central_directory_record {}; VERIFY(central_directory_record.read(m_input_data.slice(member_offset))); LocalFileHeader local_file_header {}; @@ -108,6 +108,12 @@ ZipOutputStream::ZipOutputStream(OutputStream& stream) { } +static u16 minimum_version_needed(ZipCompressionMethod method) +{ + // Deflate was added in PKZip 2.0 + return method == ZipCompressionMethod::Deflate ? 20 : 10; +} + void ZipOutputStream::add_member(const ZipMember& member) { VERIFY(!m_finished); @@ -115,20 +121,21 @@ void ZipOutputStream::add_member(const ZipMember& member) VERIFY(member.compressed_data.size() <= UINT32_MAX); m_members.append(member); - LocalFileHeader local_file_header {}; - local_file_header.minimum_version = member.compression_method == ZipCompressionMethod::Deflate ? 20 : 10; // Deflate was added in PKZip 2.0 - local_file_header.general_purpose_flags = 0; - local_file_header.compression_method = static_cast(member.compression_method); - local_file_header.modification_time = 0; // TODO: support modification time - local_file_header.modification_date = 0; - local_file_header.crc32 = member.crc32; - local_file_header.compressed_size = member.compressed_data.size(); - local_file_header.uncompressed_size = member.uncompressed_size; - local_file_header.name_length = member.name.length(); - local_file_header.extra_data_length = 0; - local_file_header.name = (const u8*)(member.name.characters()); - local_file_header.extra_data = nullptr; - local_file_header.compressed_data = member.compressed_data.data(); + LocalFileHeader local_file_header { + .minimum_version = minimum_version_needed(member.compression_method), + .general_purpose_flags = 0, + .compression_method = static_cast(member.compression_method), + .modification_time = 0, // TODO: support modification time + .modification_date = 0, + .crc32 = member.crc32, + .compressed_size = static_cast(member.compressed_data.size()), + .uncompressed_size = member.uncompressed_size, + .name_length = static_cast(member.name.length()), + .extra_data_length = 0, + .name = reinterpret_cast(member.name.characters()), + .extra_data = nullptr, + .compressed_data = member.compressed_data.data(), + }; local_file_header.write(m_stream); } @@ -137,44 +144,46 @@ void ZipOutputStream::finish() VERIFY(!m_finished); m_finished = true; - auto file_header_offset = 0; - auto central_directory_size = 0; + auto file_header_offset = 0u; + auto central_directory_size = 0u; for (const ZipMember& member : m_members) { - CentralDirectoryRecord central_directory_record {}; - auto zip_version = member.compression_method == ZipCompressionMethod::Deflate ? 20 : 10; // Deflate was added in PKZip 2.0 - central_directory_record.made_by_version = zip_version; - central_directory_record.minimum_version = zip_version; - central_directory_record.general_purpose_flags = 0; - central_directory_record.compression_method = static_cast(member.compression_method); - central_directory_record.modification_time = 0; // TODO: support modification time - central_directory_record.modification_date = 0; - central_directory_record.crc32 = member.crc32; - central_directory_record.compressed_size = member.compressed_data.size(); - central_directory_record.uncompressed_size = member.uncompressed_size; - central_directory_record.name_length = member.name.length(); - central_directory_record.extra_data_length = 0; - central_directory_record.comment_length = 0; - central_directory_record.start_disk = 0; - central_directory_record.internal_attributes = 0; - central_directory_record.external_attributes = member.is_directory ? zip_directory_external_attribute : 0; - central_directory_record.local_file_header_offset = file_header_offset; // FIXME: we assume the wrapped output stream was never written to before us + auto zip_version = minimum_version_needed(member.compression_method); + CentralDirectoryRecord central_directory_record { + .made_by_version = zip_version, + .minimum_version = zip_version, + .general_purpose_flags = 0, + .compression_method = member.compression_method, + .modification_time = 0, // TODO: support modification time + .modification_date = 0, + .crc32 = member.crc32, + .compressed_size = static_cast(member.compressed_data.size()), + .uncompressed_size = member.uncompressed_size, + .name_length = static_cast(member.name.length()), + .extra_data_length = 0, + .comment_length = 0, + .start_disk = 0, + .internal_attributes = 0, + .external_attributes = member.is_directory ? zip_directory_external_attribute : 0, + .local_file_header_offset = file_header_offset, // FIXME: we assume the wrapped output stream was never written to before us + .name = reinterpret_cast(member.name.characters()), + .extra_data = nullptr, + .comment = nullptr, + }; file_header_offset += sizeof(LocalFileHeader::signature) + (sizeof(LocalFileHeader) - (sizeof(u8*) * 3)) + member.name.length() + member.compressed_data.size(); - central_directory_record.name = (const u8*)(member.name.characters()); - central_directory_record.extra_data = nullptr; - central_directory_record.comment = nullptr; central_directory_record.write(m_stream); central_directory_size += central_directory_record.size(); } - EndOfCentralDirectory end_of_central_directory {}; - end_of_central_directory.disk_number = 0; - end_of_central_directory.central_directory_start_disk = 0; - end_of_central_directory.disk_records_count = m_members.size(); - end_of_central_directory.total_records_count = m_members.size(); - end_of_central_directory.central_directory_size = central_directory_size; - end_of_central_directory.central_directory_offset = file_header_offset; - end_of_central_directory.comment_length = 0; - end_of_central_directory.comment = nullptr; + EndOfCentralDirectory end_of_central_directory { + .disk_number = 0, + .central_directory_start_disk = 0, + .disk_records_count = static_cast(m_members.size()), + .total_records_count = static_cast(m_members.size()), + .central_directory_size = central_directory_size, + .central_directory_offset = file_header_offset, + .comment_length = 0, + .comment = nullptr, + }; end_of_central_directory.write(m_stream); } diff --git a/Userland/Libraries/LibArchive/Zip.h b/Userland/Libraries/LibArchive/Zip.h index bca2b4fd750..d7a62296ea3 100644 --- a/Userland/Libraries/LibArchive/Zip.h +++ b/Userland/Libraries/LibArchive/Zip.h @@ -221,8 +221,14 @@ public: private: static bool find_end_of_central_directory_offset(ReadonlyBytes, size_t& offset); - u16 member_count { 0 }; - size_t members_start_offset { 0 }; + Zip(u16 member_count, size_t members_start_offset, ReadonlyBytes input_data) + : m_member_count { member_count } + , m_members_start_offset { members_start_offset } + , m_input_data { input_data } + { + } + u16 m_member_count { 0 }; + size_t m_members_start_offset { 0 }; ReadonlyBytes m_input_data; };