X Tutup
Skip to content

Fix assertions against buffer overruns in input_event_codec.cpp#113028

Merged
Repiteo merged 1 commit intogodotengine:masterfrom
madsbangh:fix/asserts-in-input_event_codec.cpp
Nov 25, 2025
Merged

Fix assertions against buffer overruns in input_event_codec.cpp#113028
Repiteo merged 1 commit intogodotengine:masterfrom
madsbangh:fix/asserts-in-input_event_codec.cpp

Conversation

@madsbangh
Copy link
Contributor

I noticed DisplayServerEmbeddedState::serialize (in platform/macos/display_server_embedded.mm) had its assertion at the end fixed by @stuartcarnie from
DEV_ASSERT((data - r_data.ptrw()) >= r_data.size());
to
DEV_ASSERT(r_data.size() >= (data - r_data.ptrw()));
in commit c62ad8d

This makes sense, but I wanted to compare this change to similar asserts elsewhere.
Eventually this lead me to core/input/input_event_codec.cpp, which has a handful of equivalent assertions, also by @stuartcarnie.

It makes sense because we want to assert the buffer we are reading from / writing to is at least as long as the amount we have moved our datapointer while decoding/encoding.

This PR just follows his example and applies that fix to the DEV_ASSERTs in input_event_codec.cpp as well.

@madsbangh madsbangh requested a review from a team as a code owner November 21, 2025 22:17
Copy link
Contributor

@stuartcarnie stuartcarnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@AThousandShips AThousandShips added this to the 4.6 milestone Nov 24, 2025
@Repiteo Repiteo merged commit 807df6e into godotengine:master Nov 25, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 25, 2025

Thanks! Congratulations on your first merged contribution! 🎉

@akien-mga akien-mga changed the title Fix assertions against buffer overruns in input_event_codec.cpp Fix assertions against buffer overruns in input_event_codec.cpp Jan 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

X Tutup