From 4530325412ad184cda8610354826131418068518 Mon Sep 17 00:00:00 2001 From: Scott Allen Date: Sat, 18 Jul 2020 18:56:48 -0400 Subject: [PATCH] Fix portability issues in the Sprites classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issues discovered and fixes developed by @Pharap The Sprites classes relied on the behavior of 16 bit integer arithmetic for operations that cause an overflow; specifically (ofs + WIDTH) calculations for an index into the screen buffer. This would cause problems if the code was ported to an environment in which the basic integer type (i.e. "int") is larger than 16 bits. Also, there was code that assumed little endianness for a technique used to retrieve the high byte a 16 bit value. This would cause problems if the code was ported to a big endian architecture. The changes made are strictly for the sake of improved portability and better programming practices. They don’t affect code compiled for the Arduboy in any way. --- src/Sprites.cpp | 24 ++++++++++++++---------- src/SpritesB.cpp | 9 +++++---- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/Sprites.cpp b/src/Sprites.cpp index 8b81ce3..d6860e7 100644 --- a/src/Sprites.cpp +++ b/src/Sprites.cpp @@ -148,10 +148,11 @@ void Sprites::drawBitmap(int16_t x, int16_t y, Arduboy2Base::sBuffer[ofs] = data; } if (yOffset != 0 && sRow < 7) { - data = Arduboy2Base::sBuffer[ofs + WIDTH]; - data &= (*((unsigned char *) (&mask_data) + 1)); - data |= (*((unsigned char *) (&bitmap_data) + 1)); - Arduboy2Base::sBuffer[ofs + WIDTH] = data; + const size_t index = static_cast(ofs + WIDTH); + data = Arduboy2Base::sBuffer[index]; + data &= (uint8_t)(mask_data >> 8); + data |= (uint8_t)(bitmap_data >> 8); + Arduboy2Base::sBuffer[index] = data; } ofs++; bofs++; @@ -170,7 +171,8 @@ void Sprites::drawBitmap(int16_t x, int16_t y, Arduboy2Base::sBuffer[ofs] |= (uint8_t)(bitmap_data); } if (yOffset != 0 && sRow < 7) { - Arduboy2Base::sBuffer[ofs + WIDTH] |= (*((unsigned char *) (&bitmap_data) + 1)); + const size_t index = static_cast(ofs + WIDTH); + Arduboy2Base::sBuffer[index] |= (uint8_t)(bitmap_data >> 8); } ofs++; bofs++; @@ -189,7 +191,8 @@ void Sprites::drawBitmap(int16_t x, int16_t y, Arduboy2Base::sBuffer[ofs] &= ~(uint8_t)(bitmap_data); } if (yOffset != 0 && sRow < 7) { - Arduboy2Base::sBuffer[ofs + WIDTH] &= ~(*((unsigned char *) (&bitmap_data) + 1)); + const size_t index = static_cast(ofs + WIDTH); + Arduboy2Base::sBuffer[index] &= ~(uint8_t)(bitmap_data >> 8); } ofs++; bofs++; @@ -223,10 +226,11 @@ void Sprites::drawBitmap(int16_t x, int16_t y, Arduboy2Base::sBuffer[ofs] = data; } if (yOffset != 0 && sRow < 7) { - data = Arduboy2Base::sBuffer[ofs + WIDTH]; - data &= (*((unsigned char *) (&mask_data) + 1)); - data |= (*((unsigned char *) (&bitmap_data) + 1)); - Arduboy2Base::sBuffer[ofs + WIDTH] = data; + const size_t index = static_cast(ofs + WIDTH); + data = Arduboy2Base::sBuffer[index]; + data &= (uint8_t)(mask_data >> 8); + data |= (uint8_t)(bitmap_data >> 8); + Arduboy2Base::sBuffer[index] = data; } ofs++; mask_ofs++; diff --git a/src/SpritesB.cpp b/src/SpritesB.cpp index 27924ae..5f7c1b9 100644 --- a/src/SpritesB.cpp +++ b/src/SpritesB.cpp @@ -159,10 +159,11 @@ void SpritesB::drawBitmap(int16_t x, int16_t y, Arduboy2Base::sBuffer[ofs] = data; } if (yOffset != 0 && sRow < 7) { - data = Arduboy2Base::sBuffer[ofs + WIDTH]; - data &= (*((unsigned char *) (&mask_data) + 1)); - data |= (*((unsigned char *) (&bitmap_data) + 1)); - Arduboy2Base::sBuffer[ofs + WIDTH] = data; + const size_t index = static_cast(ofs + WIDTH); + data = Arduboy2Base::sBuffer[index]; + data &= (uint8_t)(mask_data >> 8); + data |= (uint8_t)(bitmap_data >> 8); + Arduboy2Base::sBuffer[index] = data; } ofs++; mask_ofs += ofs_step;