Repository: SerenityOS/serenity
File: Userland/Libraries/LibArchive/Zip.h (C/C++)

Code before:
    struct {
        u16 encrypted : 1;
        u16 compression_options : 2;
        u16 data_descriptor : 1;
        u16 enhanced_deflation : 1;
        u16 compressed_patched_data : 1;
        u16 strong_encryption : 1;
        u16 : 4;
        u16 language_encoding : 1;
        u16 : 1;
        u16 masked_data_values : 1;
        u16 : 2;
    };
};
static_assert(sizeof(ZipGeneralPurposeFlags) == sizeof(u16));

enum class ZipMadeBy : u8 {
    MSDOS = 0,
    Unix = 3,
};

struct [[gnu::packed]] CentralDirectoryRecord {
    static constexpr Array<u8, signature_length> signature = { 0x50, 0x4b, 0x01, 0x02 }; // 'PK\x01\x02'

    u8 supported_version; // lower byte of version made by
    ZipMadeBy made_by;    // upper byte of version made by
    u16 minimum_version;
    ZipGeneralPurposeFlags general_purpose_flags;
    ZipCompressionMethod compression_method;
    DOSPackedTime modification_time;
    DOSPackedDate modification_date;
    u32 crc32;
    u32 compressed_size;
    u32 uncompressed_size;
    u16 name_length;
    u16 extra_data_length;
    u16 comment_length;
    u16 start_disk;
    u16 internal_attributes;
    u16 msdos_attributes;    // lower bytes of external file attributes
    u16 external_attributes; // upper bytes of external file attributes
    u32 local_file_header_offset;
    u8 const* name;
    u8 const* extra_data;
    u8 const* comment;

    bool read(ReadonlyBytes buffer)
    {
        constexpr auto fields_size = sizeof(CentralDirectoryRecord) - (sizeof(u8*) * 3);
        if (!read_helper<fields_size>(buffer, this))
            return false;

Review comment (suggestion):
These do not seem like they should be split. If you need to access individual bits of them then it is probably more appropriate to create getter and setter functions or to `union` it with a bitfield.

Code after:
    struct {
        u16 encrypted : 1;
        u16 compression_options : 2;
        u16 data_descriptor : 1;
        u16 enhanced_deflation : 1;
        u16 compressed_patched_data : 1;
        u16 strong_encryption : 1;
        u16 : 4;
        u16 language_encoding : 1;
        u16 : 1;
        u16 masked_data_values : 1;
        u16 : 2;
    };
};
static_assert(sizeof(ZipGeneralPurposeFlags) == sizeof(u16));

enum class ZipMadeBy : u8 {
    MSDOS = 0,
    Unix = 3,
};

struct [[gnu::packed]] CentralDirectoryRecord {
    static constexpr Array<u8, signature_length> signature = { 0x50, 0x4b, 0x01, 0x02 }; // 'PK\x01\x02'

    struct {
        u8 version;
        ZipMadeBy made_by;
    } made_by_version;
    u16 minimum_version;
    ZipGeneralPurposeFlags general_purpose_flags;
    ZipCompressionMethod compression_method;
    DOSPackedTime modification_time;
    DOSPackedDate modification_date;
    u32 crc32;
    u32 compressed_size;
    u32 uncompressed_size;
    u16 name_length;
    u16 extra_data_length;
    u16 comment_length;
    u16 start_disk;
    u16 internal_attributes;
    struct {
        u16 msdos;
        u16 unix;
    } external_attributes;
    u32 local_file_header_offset;
    u8 const* name;
    u8 const* extra_data;
    u8 const* comment;

    bool read(ReadonlyBytes buffer)