From 691292b23ad0faf9a1be3405554716f583ab7918 Mon Sep 17 00:00:00 2001 From: Shane Whitmire Date: Sun, 18 Feb 2024 21:44:31 -0600 Subject: [PATCH] Fix assert not firing when only x index is out of bounds Original protection to UB here was to simply check if the produced index is out of bounds, but that isn't entirely correct imo. What is correct is to fire if either the x or the y go out of bounds. This shouldn't cause the UB problem to reappear, and be a little more "correct" from the user's perspective. I believe that the most correct way would be to use a boolean return in the setPixel(), and a std::optional and return None on a failed fetch for the getPixel() function. I don't know if ya'll are willing to do this, but this seems sufficient too. Atleast there is a clear place where the programs fails to work. --- src/SFML/Graphics/Image.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/SFML/Graphics/Image.cpp b/src/SFML/Graphics/Image.cpp index dba24b6ff..ded88ea1a 100644 --- a/src/SFML/Graphics/Image.cpp +++ b/src/SFML/Graphics/Image.cpp @@ -490,8 +490,10 @@ void Image::createMaskFromColor(const Color& color, std::uint8_t alpha) //////////////////////////////////////////////////////////// void Image::setPixel(const Vector2u& coords, const Color& color) { - const auto index = (coords.x + coords.y * m_size.x) * 4; - assert(index < m_pixels.size() && "Image::setPixel() cannot access out of bounds pixel"); + assert(coords.x < m_size.x && "Image::setPixel() x coordinate is out of bounds"); + assert(coords.y < m_size.y && "Image::setPixel() y coordinate is out of bounds"); + + const auto index = (coords.x + coords.y * m_size.x) * 4; std::uint8_t* pixel = &m_pixels[index]; *pixel++ = color.r; *pixel++ = color.g; @@ -503,8 +505,10 @@ void Image::setPixel(const Vector2u& coords, const Color& color) //////////////////////////////////////////////////////////// Color Image::getPixel(const Vector2u& coords) const { - const auto index = (coords.x + coords.y * m_size.x) * 4; - assert(index < m_pixels.size() && "Image::getPixel() cannot access out of bounds pixel"); + assert(coords.x < m_size.x && "Image::getPixel() x coordinate is out of bounds"); + assert(coords.y < m_size.y && "Image::getPixel() y coordinate is out of bounds"); + + const auto index = (coords.x + coords.y * m_size.x) * 4; const std::uint8_t* pixel = &m_pixels[index]; return {pixel[0], pixel[1], pixel[2], pixel[3]}; }