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

Code before:
    ZipMadeBy made_by;
};

struct ZipExternalAttributes {
    u16 msdos;
    u16 unix;
};

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

    ZipMadeByVersion 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;
    ZipExternalAttributes external_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;
        if (buffer.size() < size())
            return false;
        name = buffer.data() + signature.size() + fields_size;
        extra_data = name + name_length;
        comment = extra_data + extra_data_length;
        return true;
    }

    ErrorOr<void> write(Stream& stream) const
    {
        auto write_value = [&stream](auto value) {
            return stream.write_until_depleted({ &value, sizeof(value) });
        };

        TRY(stream.write_until_depleted(signature));

Review comment (suggestion):
I was thinking more of the following:

```cpp
[...]
u16 internal_attributes;
union {
    u32 external_attributes;
    struct {
        u16 msdos_attributes;
        u16 unix_attributes;
    };
};
u32 local_file_header_offset;
[...]
```

That way nobody gets burned if they actually expect the external attributes to be what is indicated by the specification that you linked.

Code after:

    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)
    {
        constexpr auto fields_size = sizeof(CentralDirectoryRecord) - (sizeof(u8*) * 3);
        if (!read_helper<fields_size>(buffer, this))
            return false;
        if (buffer.size() < size())
            return false;
        name = buffer.data() + signature.size() + fields_size;
        extra_data = name + name_length;
        comment = extra_data + extra_data_length;
        return true;
    }

    ErrorOr<void> write(Stream& stream) const
    {
        auto write_value = [&stream](auto value) {
            return stream.write_until_depleted({ &value, sizeof(value) });
        };

        TRY(stream.write_until_depleted(signature));
        TRY(write_value(made_by_version));
        TRY(write_value(minimum_version));
        TRY(write_value(general_purpose_flags.flags));
        TRY(write_value(compression_method));