ronantakizawa/codereview-bench huggingface dataset:
Repository: SerenityOS/serenity
File: Userland/Libraries/LibC/stdlib.cpp (C++)

Code before:
}

void setprogname(char const* progname)
{
    for (int i = strlen(progname) - 1; i >= 0; i--) {
        if (progname[i] == '/') {
            __progname = progname + i + 1;
            return;
        }
    }

    __progname = progname;
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/strtod.html
double strtod(char const* str, char** endptr)
{
    return c_str_to_floating_point<double>(str, endptr);
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/strtold.html
long double strtold(char const* str, char** endptr)
{
    // FIXME: eisel-lemire needs a deep overhaul to work with fp80 and fp128.
    // For now, we just cast to long double and accept the loss of precision.
    return (long double)c_str_to_floating_point<double>(str, endptr);
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/strtof.html
float strtof(char const* str, char** endptr)
{
    return c_str_to_floating_point<float>(str, endptr);
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/atof.html
double atof(char const* str)
{
    return strtod(str, nullptr);
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/atoi.html
int atoi(char const* str)
{
    long value = strtol(str, nullptr, 10);
    if (value > INT_MAX) {
        return INT_MAX;
    }
    return value;
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/atol.html

Review comment (suggestion):
Oh, and I missed this. Prefer `static_cast<long double>` to C-style casts.

Code after:
}

void setprogname(char const* progname)
{
    for (int i = strlen(progname) - 1; i >= 0; i--) {
        if (progname[i] == '/') {
            __progname = progname + i + 1;
            return;
        }
    }

    __progname = progname;
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/strtod.html
double strtod(char const* str, char** endptr)
{
    return c_str_to_floating_point<double>(str, endptr);
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/strtold.html
long double strtold(char const* str, char** endptr)
{
    // FIXME: eisel-lemire needs a deep overhaul to work with fp80 and fp128.
    // For now, we just implicitly cast to long double and accept the loss of precision.
    return strtod(str, endptr);
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/strtof.html
float strtof(char const* str, char** endptr)
{
    return c_str_to_floating_point<float>(str, endptr);
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/atof.html
double atof(char const* str)
{
    return strtod(str, nullptr);
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/atoi.html
int atoi(char const* str)
{
    long value = strtol(str, nullptr, 10);
    if (value > INT_MAX) {
        return INT_MAX;
    }
    return value;
}

// https://pubs.opengroup.org/onlinepubs/9699919799/functions/atol.html
Repository: SerenityOS/serenity
File: AK/DOSPackedTime.h (C/C++)

Code before:

union DOSPackedTime {
    u16 value;
    struct {
        u16 second : 5;
        u16 minute : 6;
        u16 hour : 5;
    };
};
static_assert(sizeof(DOSPackedTime) == 2);

union DOSPackedDate {
    u16 value;
    struct {
        u16 day : 5;
        u16 month : 4;
        u16 year : 7;
    };
};
static_assert(sizeof(DOSPackedDate) == 2);

inline constexpr u16 first_dos_year = 1980;
// 1980/01/01
inline constexpr u32 first_dos_representable_unix_timestamp = 86'400UL * 365 * 10 + 86'400 * 2;
// 2107/12/31 23:59:59
inline constexpr u64 last_dos_representable_unix_timestamp = 4354819199ULL;

UnixDateTime time_from_packed_dos(DOSPackedDate, DOSPackedTime);
DOSPackedDate to_packed_dos_date(unsigned year, unsigned month, unsigned day);
DOSPackedTime to_packed_dos_time(unsigned hour, unsigned minute, unsigned second);
DOSPackedDate to_packed_dos_date(UnixDateTime const&);
DOSPackedTime to_packed_dos_time(UnixDateTime const& unix_date_time);

}

#if USING_AK_GLOBALLY
using AK::DOSPackedDate;
using AK::DOSPackedTime;
using AK::first_dos_representable_unix_timestamp;
using AK::first_dos_year;
using AK::last_dos_representable_unix_timestamp;
using AK::time_from_packed_dos;
using AK::to_packed_dos_date;
using AK::to_packed_dos_time;
#endif


Review comment (suggestion):
nit: `UnixDateTime::from_unix_time_parts` is `constexpr` so you could use that here.

Code after:

union DOSPackedTime {
    u16 value;
    struct {
        u16 second : 5;
        u16 minute : 6;
        u16 hour : 5;
    };
};
static_assert(sizeof(DOSPackedTime) == 2);

union DOSPackedDate {
    u16 value;
    struct {
        u16 day : 5;
        u16 month : 4;
        u16 year : 7;
    };
};
static_assert(sizeof(DOSPackedDate) == 2);

inline constexpr u16 first_dos_year = 1980;
inline constexpr u32 first_dos_representable_unix_timestamp = UnixDateTime::from_unix_time_parts(1980, 1, 1, 0, 0, 0, 0).seconds_since_epoch();
inline constexpr u64 last_dos_representable_unix_timestamp = UnixDateTime::from_unix_time_parts(2107, 12, 31, 23, 59, 59, 0).seconds_since_epoch();

UnixDateTime time_from_packed_dos(DOSPackedDate, DOSPackedTime);
DOSPackedDate to_packed_dos_date(unsigned year, unsigned month, unsigned day);
DOSPackedTime to_packed_dos_time(unsigned hour, unsigned minute, unsigned second);
ErrorOr<DOSPackedDate> to_packed_dos_date(UnixDateTime const&);
ErrorOr<DOSPackedTime> to_packed_dos_time(UnixDateTime const&);

}

#if USING_AK_GLOBALLY
using AK::DOSPackedDate;
using AK::DOSPackedTime;
using AK::time_from_packed_dos;
using AK::to_packed_dos_date;
using AK::to_packed_dos_time;
#endif
Repository: SerenityOS/serenity
File: AK/DOSPackedTime.h (C/C++)

Code before:

namespace AK {

union DOSPackedTime {
    u16 value;
    struct {
        u16 second : 5;
        u16 minute : 6;
        u16 hour : 5;
    };
};
static_assert(sizeof(DOSPackedTime) == 2);

union DOSPackedDate {
    u16 value;
    struct {
        u16 day : 5;
        u16 month : 4;
        u16 year : 7;
    };
};
static_assert(sizeof(DOSPackedDate) == 2);

inline constinit u16 first_dos_year = 1980;
inline constinit u32 first_dos_representable_unix_timestamp = UnixDateTime::from_unix_time_parts(1980, 1, 1, 0, 0, 0, 0).seconds_since_epoch();
inline constinit u64 last_dos_representable_unix_timestamp = UnixDateTime::from_unix_time_parts(2107, 12, 31, 23, 59, 59, 0).seconds_since_epoch();

UnixDateTime time_from_packed_dos(DOSPackedDate, DOSPackedTime);
DOSPackedDate to_packed_dos_date(unsigned year, unsigned month, unsigned day);
DOSPackedTime to_packed_dos_time(unsigned hour, unsigned minute, unsigned second);
ErrorOr<DOSPackedDate> to_packed_dos_date(UnixDateTime const&);
ErrorOr<DOSPackedTime> to_packed_dos_time(UnixDateTime const&);

}

#if USING_AK_GLOBALLY
using AK::DOSPackedDate;
using AK::DOSPackedTime;
using AK::time_from_packed_dos;
using AK::to_packed_dos_date;
using AK::to_packed_dos_time;
#endif


Review comment (suggestion):
`constexpr` was correct here. `constinit` is weaker than `constexpr` and just means it needs a constant initializer. `constinit` doesn't define a compile-time constant.

(these `const*` keywords are a bit confusing :^))

Code after:

namespace AK {

union DOSPackedTime {
    u16 value;
    struct {
        u16 second : 5;
        u16 minute : 6;
        u16 hour : 5;
    };
};
static_assert(sizeof(DOSPackedTime) == 2);

union DOSPackedDate {
    u16 value;
    struct {
        u16 day : 5;
        u16 month : 4;
        u16 year : 7;
    };
};
static_assert(sizeof(DOSPackedDate) == 2);

inline constexpr u16 first_dos_year = 1980;
inline constexpr u32 first_dos_representable_unix_timestamp = UnixDateTime::from_unix_time_parts(1980, 1, 1, 0, 0, 0, 0).seconds_since_epoch();
inline constexpr u64 last_dos_representable_unix_timestamp = UnixDateTime::from_unix_time_parts(2107, 12, 31, 23, 59, 59, 0).seconds_since_epoch();

UnixDateTime time_from_packed_dos(DOSPackedDate, DOSPackedTime);
DOSPackedDate to_packed_dos_date(unsigned year, unsigned month, unsigned day);
DOSPackedTime to_packed_dos_time(unsigned hour, unsigned minute, unsigned second);
ErrorOr<DOSPackedDate> to_packed_dos_date(UnixDateTime const&);
ErrorOr<DOSPackedTime> to_packed_dos_time(UnixDateTime const&);

}

#if USING_AK_GLOBALLY
using AK::DOSPackedDate;
using AK::DOSPackedTime;
using AK::time_from_packed_dos;
using AK::to_packed_dos_date;
using AK::to_packed_dos_time;
#endif
Repository: SerenityOS/serenity
File: Userland/Applications/DisplaySettings/BackgroundSettingsWidget.cpp (C++)

Code before:

    auto palette_desktop_color = palette().desktop_background();
    auto background_color = ws_config->read_entry("Background", "Color", "");

    if (!background_color.is_empty()) {
        auto opt_color = Color::from_string(background_color);
        if (opt_color.has_value())
            palette_desktop_color = opt_color.value();
    }

    m_color_input->set_color(palette_desktop_color, GUI::AllowCallback::No);
    m_monitor_widget->set_background_color(palette_desktop_color);
    m_background_settings_changed = false;

    return {};
}

void BackgroundSettingsWidget::apply_settings()
{
    // We need to provide an empty path (not OptionalNone) to set_wallpaper to save a solid color wallpaper.
    auto wallpaper_path = m_monitor_widget->wallpaper().value_or(""sv);
    if (!GUI::Desktop::the().set_wallpaper(m_monitor_widget->wallpaper_bitmap(), wallpaper_path)) {
        if (!wallpaper_path.is_empty())
            GUI::MessageBox::show_error(window(), MUST(String::formatted("Unable to load file {} as wallpaper", wallpaper_path)));
        else
            GUI::MessageBox::show_error(window(), "Unable set wallpaper"sv);
    }

    GUI::Desktop::the().set_background_color(m_color_input->text());
    GUI::Desktop::the().set_wallpaper_mode(m_monitor_widget->wallpaper_mode());
}

}


Review comment (suggestion):
The error message appears to have a typo; consider changing it to "Unable to set wallpaper" for better clarity.
```suggestion
            GUI::MessageBox::show_error(window(), "Unable to set wallpaper"sv);
```

Code after:

    auto palette_desktop_color = palette().desktop_background();
    auto background_color = ws_config->read_entry("Background", "Color", "");

    if (!background_color.is_empty()) {
        auto opt_color = Color::from_string(background_color);
        if (opt_color.has_value())
            palette_desktop_color = opt_color.value();
    }

    m_color_input->set_color(palette_desktop_color, GUI::AllowCallback::No);
    m_monitor_widget->set_background_color(palette_desktop_color);
    m_background_settings_changed = false;

    return {};
}

void BackgroundSettingsWidget::apply_settings()
{
    // We need to provide an empty path (not OptionalNone) to set_wallpaper to save a solid color wallpaper.
    auto wallpaper_path = m_monitor_widget->wallpaper().value_or(""sv);
    if (!GUI::Desktop::the().set_wallpaper(m_monitor_widget->wallpaper_bitmap(), wallpaper_path)) {
        if (!wallpaper_path.is_empty())
            GUI::MessageBox::show_error(window(), MUST(String::formatted("Unable to load file {} as wallpaper", wallpaper_path)));
        else
            GUI::MessageBox::show_error(window(), "Unable to set wallpaper"sv);
    }

    GUI::Desktop::the().set_background_color(m_color_input->text());
    GUI::Desktop::the().set_wallpaper_mode(m_monitor_widget->wallpaper_mode());
}

}
Repository: SerenityOS/serenity
File: Ports/gmp/package.sh (Shell)

Code before:
#!/usr/bin/env -S bash ../.port_include.sh
port='gmp'
version='6.3.0'
useconfigure='true'
configopts=(
   # We do not support 32-bit ABIs on 64-bit systems,
   # and only support 64-bit systems
   "ABI=64"
   # C20 does not allow incomplete prototypes, which gmp implicitly uses
   "CFLAGS=-O2 -pedantic -std=c11"
)
files=(
    "https://ftpmirror.gnu.org/gnu/gmp/gmp-${version}.tar.xz#a3c2b80201b89e68616f4ad30bc66aee4927c3ce50e33929ca819d5c43538898"
)


Review comment (suggestion):
This breaks on riscv64:
`configure: error: ABI=64 is not among the following valid choices: standard`

Code after:
#!/usr/bin/env -S bash ../.port_include.sh
port='gmp'
version='6.3.0'
useconfigure='true'
configopts=(
   # C20+ does not allow incomplete prototypes, which gmp implicitly uses
   "CFLAGS=-O2 -pedantic -std=c11"
)
files=(
    "https://ftpmirror.gnu.org/gnu/gmp/gmp-${version}.tar.xz#a3c2b80201b89e68616f4ad30bc66aee4927c3ce50e33929ca819d5c43538898"
)
Repository: SerenityOS/serenity
File: Userland/Applications/PixelPaint/ImageEditor.cpp (C++)

Code before:
        return;

    if (event.key() == Key_Escape && !m_image->selection().is_empty()) {
        m_image->selection().clear();
        did_complete_action("Clear Selection"sv);
        return;
    }

    event.ignore();
}

void ImageEditor::keyup_event(GUI::KeyEvent& event)
{
    if (!m_active_tool)
        return;

    if (!m_active_tool->is_overriding_alt() && event.key() == Key_LeftAlt)
        update_tool_cursor();

    if (event.key() == Key_Space) {
        stop_panning();

        // The set_override_cursor is called in stop_panning, but for some reason it doesn't really reset the cursor
        // But if it's being called here, it does reset the cursor. Weird...
        set_override_cursor(m_active_cursor);
    }

    m_active_tool->on_keyup(event);
}

void ImageEditor::enter_event(Core::Event&)
{
    set_override_cursor(m_active_cursor);
}

void ImageEditor::leave_event(Core::Event&)
{
    set_override_cursor(Gfx::StandardCursor::None);

    if (on_leave)
        on_leave();
}

void ImageEditor::set_active_layer(Layer* layer)
{
    if (m_active_layer == layer)
        return;
    m_active_layer = layer;

    if (m_active_layer) {
        VERIFY(&m_active_layer->image() == m_image.ptr());

Review comment (refactor):
As discussed on Discord, it would be great if for this revision of the PR (not necessarily the same commit) we could at least move the bandaid fix into `AbstractZoomPanWidget` (in the form of `if (!m_is_panning)`) and with a comment noting that we are doing this because keydown events get duplicated on key hold.

Code after:
        did_complete_action("Clear Selection"sv);
        return;
    }

    if (event.key() == Key_Space) {
        start_panning(m_mouse_position);
        return;
    }

    event.ignore();
}

void ImageEditor::keyup_event(GUI::KeyEvent& event)
{
    if (!m_active_tool)
        return;

    if (!m_active_tool->is_overriding_alt() && event.key() == Key_LeftAlt)
        update_tool_cursor();

    if (event.key() == Key_Space)
        stop_panning();

    m_active_tool->on_keyup(event);
}

void ImageEditor::enter_event(Core::Event&)
{
    set_override_cursor(m_active_cursor);
}

void ImageEditor::leave_event(Core::Event&)
{
    set_override_cursor(Gfx::StandardCursor::None);

    if (on_leave)
        on_leave();
}

void ImageEditor::set_active_layer(Layer* layer)
{
    if (m_active_layer == layer)
        return;
    m_active_layer = layer;

    if (m_active_layer) {
        VERIFY(&m_active_layer->image() == m_image.ptr());
        size_t index = 0;
        for (; index < m_image->layer_count(); ++index) {
            if (&m_image->layer(index) == layer)
                break;
Repository: SerenityOS/serenity
File: Kernel/Arch/aarch64/RegisterState.h (C/C++)

Code before:
        arg3 = x[3];
        arg4 = x[4];
    }
};

#define REGISTER_STATE_SIZE (36 * 8)
static_assert(AssertSize<RegisterState, REGISTER_STATE_SIZE>());

inline void copy_kernel_registers_into_ptrace_registers(PtraceRegisters& ptrace_regs, RegisterState const& kernel_regs)
{
    for (auto i = 0; i < 31; i++)
        ptrace_regs.x[i] = kernel_regs.x[i];

    ptrace_regs.sp = kernel_regs.userspace_sp();
    ptrace_regs.pc = kernel_regs.ip();
    ptrace_regs.spsr_el1 = kernel_regs.pstate();
}

inline void copy_ptrace_registers_into_kernel_registers(RegisterState& kernel_regs, PtraceRegisters const& ptrace_regs)
{
    for (auto i = 0; i < 31; i++)
        kernel_regs.x[i] = ptrace_regs.x[i];

    kernel_regs.set_userspace_sp(ptrace_regs.sp);
    kernel_regs.set_ip(ptrace_regs.pc);
    kernel_regs.set_pstate(ptrace_regs.spsr_el1);
}

struct DebugRegisterState {
};

}


Review comment (security):
You need to sanitize the userspace value here, see the x86 version of `copy_ptrace_registers_into_kernel_registers`.
We definitely don't want to allow userspace to disable interrupts. There may be other fields we have to mask here as well. We also shouldn't allow userspace to set reserved fields.

Code after:
        arg2 = x[2];
        arg3 = x[3];
        arg4 = x[4];
    }
};

#define REGISTER_STATE_SIZE (36 * 8)
static_assert(AssertSize<RegisterState, REGISTER_STATE_SIZE>());

inline void copy_kernel_registers_into_ptrace_registers(PtraceRegisters& ptrace_regs, RegisterState const& kernel_regs)
{
    for (auto i = 0; i < 31; i++)
        ptrace_regs.x[i] = kernel_regs.x[i];

    ptrace_regs.sp = kernel_regs.userspace_sp();
    ptrace_regs.pc = kernel_regs.ip();
    ptrace_regs.spsr_el1 = kernel_regs.pstate();
}

inline void copy_ptrace_registers_into_kernel_registers(RegisterState& kernel_regs, PtraceRegisters const& ptrace_regs)
{
    for (auto i = 0; i < 31; i++)
        kernel_regs.x[i] = ptrace_regs.x[i];

    kernel_regs.set_userspace_sp(ptrace_regs.sp);
    kernel_regs.set_ip(ptrace_regs.pc);
    kernel_regs.spsr_el1 = (kernel_regs.spsr_el1 & ~safe_pstate_mask) | (ptrace_regs.spsr_el1 & safe_pstate_mask);
}

struct DebugRegisterState {
    u64 mdscr_el1;
};

inline void read_debug_registers_into(DebugRegisterState& state)
{
    state.mdscr_el1 = Aarch64::Asm::get_mdscr_el1();
}

inline void write_debug_registers_from(DebugRegisterState const& state)
{
    Aarch64::Asm::set_mdscr_el1(state.mdscr_el1);
}

inline void clear_debug_registers()
{
    Aarch64::Asm::set_mdscr_el1(0);
}

}
Repository: SerenityOS/serenity
File: Tests/Kernel/TestPosixSpawn.cpp (C++)

Code before:
/*
 * Copyright (c) 2025, Tomás Simões <tomasprsimoes@tecnico.ulisboa.pt>
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

#include <AK/StringView.h>
#include <LibCore/System.h>
#include <LibTest/TestCase.h>

TEST_CASE(test_posix_spawn_bin_true_success)
{
    // Arguments for the spawned process. argv[0] is the program name.
    char* const argv[] = { const_cast<char*>("/bin/true"), nullptr };
    int status;

    // Attempt to spawn /bin/true with no fileactions or spawnattr
    constexpr StringView path_argument = "/bin/true"sv;

    auto pid = Core::System::posix_spawn(path_argument, nullptr, nullptr, argv, environ);
    if (pid.is_error()) {
        FAIL(ByteString::formatted("posix_spawn for '{}' failed: {}", path_argument, pid.error()));
    }

    // Wait for the child process to terminate.
    pid_t waited_pid = waitpid(pid.value(), &status, 0);
    EXPECT_EQ(waited_pid, pid.value());

    EXPECT_EQ(WEXITSTATUS(status), 0);
}


Review comment (suggestion):
is the const-ptr really necessary,
imo it makes it a bit more confusing with the `const_cast`

Code after:
/*
 * Copyright (c) 2025, Tomás Simões <tomasprsimoes@tecnico.ulisboa.pt>
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

#include <AK/StringView.h>
#include <LibCore/System.h>
#include <LibTest/TestCase.h>

TEST_CASE(test_posix_spawn_bin_true_success)
{
    // Arguments for the spawned process. argv[0] is the program name.
    char* argv[] = { const_cast<char*>("/bin/true"), nullptr };
    int status;

    // Attempt to spawn /bin/true with no fileactions or spawnattr
    constexpr StringView path_argument = "/bin/true"sv;

    auto pid = TRY_OR_FAIL(Core::System::posix_spawn(path_argument, nullptr, nullptr, argv, environ));

    // Wait for the child process to terminate.
    pid_t waited_pid = waitpid(pid, &status, 0);
    EXPECT_EQ(waited_pid, pid);

    EXPECT_EQ(WEXITSTATUS(status), 0);
}
Repository: SerenityOS/serenity
File: Userland/Libraries/LibCrypto/Checksum/IPv4Header.cpp (C++)

Code before:
/*
 * Copyright (c) 2020-2022, the SerenityOS developers.
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

#include <AK/Array.h>
#include <AK/NumericLimits.h>
#include <AK/Span.h>
#include <AK/Types.h>
#include <LibCrypto/Checksum/IPv4Header.h>
#include <arpa/inet.h>

namespace Crypto::Checksum {

void IPv4Header::update(ReadonlyBytes data)
{
    for (size_t i = 0; i < data.size() / 2; ++i)
        // Dealing with the byte order isn't technically a part of the checksumming
        // process, because it's expected that you'd already have the packet decoded,
        // but since we're dealing with raw data, the responsibility of decoding (also)
        // falls on us.
        m_state += ntohs(*reinterpret_cast<u16 const*>(data.data() + (i * 2)));
}

u16 IPv4Header::digest()
{
    u32 output = m_state;
    // While there are any bits above the bottom 16...
    while (output >> 16)
        // Drop the top bits, and add the carries to the sum.
        output = (output & 0xFFFF) + (output >> 16);

    // Return the one's complement.
    return htons(~static_cast<u16>(output));
}

}


Review comment (suggestion):
2.5 possibly better options:
A: Force Aligned data to be passed in
B: Use a FixedMemoryStream and read BigEndian<u16>
B.5; Reinterpret cast to use BigEndian<u16>[] as that is packed/byte-alligned 

Code after:
/*
 * Copyright (c) 2020-2022, the SerenityOS developers.
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

#include <AK/Array.h>
#include <AK/ByteReader.h>
#include <AK/Endian.h>
#include <AK/NumericLimits.h>
#include <AK/Span.h>
#include <AK/Types.h>
#include <LibCrypto/Checksum/IPv4Header.h>
#include <arpa/inet.h>

namespace Crypto::Checksum {

void IPv4Header::update(ReadonlyBytes data)
{
    for (size_t i = 0; i < data.size() / sizeof(u16); ++i)
        // Dealing with the byte order isn't technically a part of the checksumming
        // process, because it's expected that you'd already have the packet decoded,
        // but since we're dealing with raw data, the responsibility of decoding (also)
        // falls on us.
        m_state += AK::convert_between_host_and_network_endian(ByteReader::load16(data.offset_pointer(i * sizeof(u16))));
}

u16 IPv4Header::digest()
{
    u32 output = m_state;
    // While there are any bits above the bottom 16...
    while (output >> 16)
        // Drop the top bits, and add the carries to the sum.
        output = (output & 0xFFFF) + (output >> 16);

    // Return the one's complement.
    return htons(~static_cast<u16>(output));
}

}
Repository: SerenityOS/serenity
File: AK/SIMDFormat.h (C/C++)

Code before:
/*
 * Copyright (c) 2025, Lucas Chollet <lucas.chollet@serenityos.org>
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

#pragma once

#include <AK/Format.h>

namespace AK {

template<SIMD::SIMDVector V>
struct Formatter<V> : Formatter<StringView> {
    ErrorOr<void> format(FormatBuilder& builder, V const& vector)
    {
        TRY(builder.put_literal("{ "sv));
        for (u32 i = 0; i < SIMD::vector_length<V>; ++i) {
            if constexpr (IsFloatingPoint<SIMD::ElementOf<V>>)
                TRY(builder.put_f32_or_f64(vector[i]));
            else
                TRY(builder.put_u64(vector[i]));
            if (i != SIMD::vector_length<V> - 1)
                TRY(builder.put_literal(", "sv));
        }
        TRY(builder.put_literal("}"sv));

        return {};
    }
};

}


Review comment (suggestion):
Sorry I missed this, but why is there a space after the `{`?
The test case doesn't expect a space there but still succeeds somehow.

Code after:
/*
 * Copyright (c) 2025, Lucas Chollet <lucas.chollet@serenityos.org>
 *
 * SPDX-License-Identifier: BSD-2-Clause
 */

#pragma once

#include <AK/Format.h>

namespace AK {

template<SIMD::SIMDVector V>
struct Formatter<V> : Formatter<StringView> {
    ErrorOr<void> format(FormatBuilder& builder, V const& vector)
    {
        TRY(builder.put_literal("{"sv));
        for (u32 i = 0; i < SIMD::vector_length<V>; ++i) {
            if constexpr (IsFloatingPoint<SIMD::ElementOf<V>>)
                TRY(builder.put_f32_or_f64(vector[i]));
            else
                TRY(builder.put_u64(vector[i]));
            if (i != SIMD::vector_length<V> - 1)
                TRY(builder.put_literal(", "sv));
        }
        TRY(builder.put_literal("}"sv));

        return {};
    }
};

}