Repository: SerenityOS/serenity
File: Kernel/FileSystem/FATFS/Inode.cpp (C++)

Code before:
    u32 first_block_index = offset / fs().m_device_block_size;
    u32 last_block_index = (offset + size - 1) / fs().m_device_block_size;

    size_t offset_into_first_block = offset - first_block_index * fs().m_device_block_size;

    size_t nwritten = 0;
    size_t remaining_count = size;
    for (u32 block_index = first_block_index; block_index <= last_block_index; ++block_index) {
        size_t offset_into_block = block_index == first_block_index ? offset_into_first_block : 0;

        size_t to_write = min(fs().m_device_block_size - offset_into_block, remaining_count);
        dbgln_if(FAT_DEBUG, "FATInode[{}]::write_bytes_locked(): Writing {} byte(s) to block {} at offset {}", identifier(), to_write, block_list[block_index], offset_into_block);

        TRY(fs().write_block(block_list[block_index], buffer.offset(nwritten), to_write, offset_into_block));

        nwritten += to_write;
        remaining_count -= to_write;
    }

    return size;
}

ErrorOr<void> FATInode::fill_in_creation_time(FATEntry& entry, UnixDateTime const& timestamp)
{
    entry.creation_date = TRY(to_packed_dos_date(timestamp));
    entry.creation_time = TRY(to_packed_dos_time(timestamp));

    // NOTE: The "creation_time_seconds" field not only compensates for the fact that seconds are counted in intervals of two,
    // it also adds in rudimentary support for millisecond precision, though this is limited to only two digits.
    auto day_start_precision = days_since_epoch(entry.creation_date.year + AK::first_dos_year, entry.creation_date.month, entry.creation_date.day) * 86'400;
    auto day_precision = entry.creation_time.hour * 60 * 60 + entry.creation_time.minute * 60 + entry.creation_time.second * 2;
    entry.creation_time_seconds = (timestamp.truncated_milliseconds_since_epoch() - day_start_precision * 1000 - day_precision * 1000) / 10;
    VERIFY(entry.creation_time_seconds < 200);

    return {};
}

ErrorOr<NonnullRefPtr<Inode>> FATInode::create_child(StringView name, mode_t mode, dev_t, UserID, GroupID)
{
    MutexLocker locker(m_inode_lock);
    VERIFY(has_flag(m_entry.attributes, FATAttributes::Directory));

    dbgln_if(FAT_DEBUG, "FATInode[{}]::create_child(): creating inode \"{}\"", identifier(), name);

    if (!Kernel::is_directory(mode) && !Kernel::is_regular_file(mode))
        return ENOTSUP;

    FATEntry entry {};

    bool valid_sfn = SFNUtils::is_valid_sfn(name);


Review comment (bug):
Shouldn't we do this in an all or-or-nothing way?
Having a partially initialized struct feels wrong.

I would use 2 temporary variables for the date and time.

Code after:
    u32 first_block_index = offset / fs().m_device_block_size;
    u32 last_block_index = (offset + size - 1) / fs().m_device_block_size;

    size_t offset_into_first_block = offset - first_block_index * fs().m_device_block_size;

    size_t nwritten = 0;
    size_t remaining_count = size;
    for (u32 block_index = first_block_index; block_index <= last_block_index; ++block_index) {
        size_t offset_into_block = block_index == first_block_index ? offset_into_first_block : 0;

        size_t to_write = min(fs().m_device_block_size - offset_into_block, remaining_count);
        dbgln_if(FAT_DEBUG, "FATInode[{}]::write_bytes_locked(): Writing {} byte(s) to block {} at offset {}", identifier(), to_write, block_list[block_index], offset_into_block);

        TRY(fs().write_block(block_list[block_index], buffer.offset(nwritten), to_write, offset_into_block));

        nwritten += to_write;
        remaining_count -= to_write;
    }

    return size;
}

ErrorOr<void> FATInode::fill_in_creation_time(FATEntry& entry, UnixDateTime const& timestamp)
{
    auto packed_date = TRY(to_packed_dos_date(timestamp));
    auto packed_time = TRY(to_packed_dos_time(timestamp));

    entry.creation_date = move(packed_date);
    entry.creation_time = move(packed_time);

    // NOTE: The "creation_time_seconds" field not only compensates for the fact that seconds are counted in intervals of two,
    // it also adds in rudimentary support for millisecond precision, though this is limited to only two digits.
    auto day_start_precision = days_since_epoch(entry.creation_date.year + AK::first_dos_year, entry.creation_date.month, entry.creation_date.day) * 86'400;
    auto day_precision = entry.creation_time.hour * 60 * 60 + entry.creation_time.minute * 60 + entry.creation_time.second * 2;
    entry.creation_time_seconds = (timestamp.truncated_milliseconds_since_epoch() - day_start_precision * 1000 - day_precision * 1000) / 10;
    VERIFY(entry.creation_time_seconds < 200);

    return {};
}

ErrorOr<NonnullRefPtr<Inode>> FATInode::create_child(StringView name, mode_t mode, dev_t, UserID, GroupID)
{
    MutexLocker locker(m_inode_lock);
    VERIFY(has_flag(m_entry.attributes, FATAttributes::Directory));

    dbgln_if(FAT_DEBUG, "FATInode[{}]::create_child(): creating inode \"{}\"", identifier(), name);

    if (!Kernel::is_directory(mode) && !Kernel::is_regular_file(mode))
        return ENOTSUP;

    FATEntry entry {};