From 332fce28ed2cd2c611220c8e5e3a87ab2c5292a6 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 4 Feb 2023 11:34:17 -0800 Subject: [PATCH] fixes (#639) improper copy amounts not accounting for size of the copy (non byte copy) Some documenting comments --- src/NeoPixelBrightnessBus.h | 9 +++++ src/internal/NeoColorFeatures.h | 61 ++++++++++++++++++--------------- 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/NeoPixelBrightnessBus.h b/src/NeoPixelBrightnessBus.h index 8dba1b1..b3a9bb2 100644 --- a/src/NeoPixelBrightnessBus.h +++ b/src/NeoPixelBrightnessBus.h @@ -35,6 +35,10 @@ private: void ScaleColor(uint16_t scale, typename T_COLOR_FEATURE::ColorObject* color) { + // This is the similiar as calling Dim on the color object + // there is an assumption that all color elements are byte aligned + // so if any future color object gets introduced that is not it will + // cause a problem uint8_t* ptr = (uint8_t*)color; uint8_t* ptrEnd = ptr + sizeof(typename T_COLOR_FEATURE::ColorObject); @@ -47,12 +51,17 @@ private: void ConvertColor(typename T_COLOR_FEATURE::ColorObject* color) { + // This is the same as calling Dim on the color object uint16_t scale = _brightness + 1; ScaleColor(scale, color); } void RecoverColor(typename T_COLOR_FEATURE::ColorObject* color) const { + // this is the same as calling Brighton on the color object + // there is an assumption that all color elements are byte aligned + // so if any future color object gets introduced that is not it will + // cause a problem uint8_t* ptr = (uint8_t*)color; uint8_t* ptrEnd = ptr + sizeof(typename T_COLOR_FEATURE::ColorObject); uint16_t scale = _brightness + 1; diff --git a/src/internal/NeoColorFeatures.h b/src/internal/NeoColorFeatures.h index 5c01170..afe1538 100644 --- a/src/internal/NeoColorFeatures.h +++ b/src/internal/NeoColorFeatures.h @@ -42,7 +42,8 @@ public: static void replicatePixel(uint8_t* pPixelDest, const uint8_t* pPixelSrc, uint16_t count) { - uint8_t* pEnd = pPixelDest + (count * PixelSize); + const uint8_t* pEnd = pPixelDest + (count * PixelSize); + while (pPixelDest < pEnd) { for (uint8_t iElement = 0; iElement < PixelSize; iElement++) @@ -54,7 +55,8 @@ public: static void movePixelsInc(uint8_t* pPixelDest, const uint8_t* pPixelSrc, uint16_t count) { - uint8_t* pEnd = pPixelDest + (count * PixelSize); + const uint8_t* pEnd = pPixelDest + (count * PixelSize); + while (pPixelDest < pEnd) { *pPixelDest++ = *pPixelSrc++; @@ -63,8 +65,9 @@ public: static void movePixelsInc_P(uint8_t* pPixelDest, PGM_VOID_P pPixelSrc, uint16_t count) { - uint8_t* pEnd = pPixelDest + (count * PixelSize); + const uint8_t* pEnd = pPixelDest + (count * PixelSize); const uint8_t* pSrc = reinterpret_cast(pPixelSrc); + while (pPixelDest < pEnd) { *pPixelDest++ = pgm_read_byte(pSrc++); @@ -75,6 +78,7 @@ public: { uint8_t* pDestBack = pPixelDest + (count * PixelSize); const uint8_t* pSrcBack = pPixelSrc + (count * PixelSize); + while (pDestBack > pPixelDest) { *--pDestBack = *--pSrcBack; @@ -102,8 +106,8 @@ public: { uint32_t* pDest = reinterpret_cast(pPixelDest); const uint32_t* pSrc = reinterpret_cast(pPixelSrc); + const uint32_t* pEnd = pDest + count; // * PixelSize / sizeof(*pDest); - uint32_t* pEnd = pDest + count; while (pDest < pEnd) { *pDest++ = *pSrc; @@ -114,7 +118,8 @@ public: { uint32_t* pDest = reinterpret_cast(pPixelDest); const uint32_t* pSrc = reinterpret_cast(pPixelSrc); - uint32_t* pEnd = pDest + count; + const uint32_t* pEnd = pDest + count; // * PixelSize / sizeof(*pDest); + while (pDest < pEnd) { *pDest++ = *pSrc++; @@ -125,7 +130,8 @@ public: { uint32_t* pDest = reinterpret_cast(pPixelDest); const uint32_t* pSrc = reinterpret_cast(pPixelSrc); - uint32_t* pEnd = pDest + count; + const uint32_t* pEnd = pDest + count; // * PixelSize / sizeof(*pDest); + while (pDest < pEnd) { *pDest++ = pgm_read_dword(pSrc++); @@ -136,8 +142,9 @@ public: { uint32_t* pDest = reinterpret_cast(pPixelDest); const uint32_t* pSrc = reinterpret_cast(pPixelSrc); - uint32_t* pDestBack = pDest + count; - const uint32_t* pSrcBack = pSrc + count; + uint32_t* pDestBack = pDest + count; // * PixelSize / sizeof(*pDest); + const uint32_t* pSrcBack = pSrc + count; // * PixelSize / sizeof(*pSrc); + while (pDestBack > pDest) { *--pDestBack = *--pSrcBack; @@ -165,8 +172,8 @@ public: { uint16_t* pDest = reinterpret_cast(pPixelDest); const uint16_t* pSrc = reinterpret_cast(pPixelSrc); + const uint16_t* pEnd = pDest + (count * PixelSize / sizeof(*pDest)); - uint16_t* pEnd = pDest + (count * PixelSize / 2); while (pDest < pEnd) { *pDest++ = *pSrc; @@ -177,7 +184,8 @@ public: { uint16_t* pDest = reinterpret_cast(pPixelDest); const uint16_t* pSrc = reinterpret_cast(pPixelSrc); - uint16_t* pEnd = pDest + (count * PixelSize / 2); + const uint16_t* pEnd = pDest + (count * PixelSize / sizeof(*pDest)); + while (pDest < pEnd) { *pDest++ = *pSrc++; @@ -188,7 +196,8 @@ public: { uint16_t* pDest = reinterpret_cast(pPixelDest); const uint16_t* pSrc = reinterpret_cast(pPixelSrc); - uint16_t* pEnd = pDest + (count * PixelSize / 2); + const uint16_t* pEnd = pDest + (count * PixelSize / sizeof(*pDest)); + while (pDest < pEnd) { *pDest++ = pgm_read_word(pSrc++); @@ -199,8 +208,9 @@ public: { uint16_t* pDest = reinterpret_cast(pPixelDest); const uint16_t* pSrc = reinterpret_cast(pPixelSrc); - uint16_t* pDestBack = pDest + (count * PixelSize / 2); - const uint16_t* pSrcBack = pSrc + (count * PixelSize / 2); + uint16_t* pDestBack = pDest + (count * PixelSize / sizeof(*pDest)); + const uint16_t* pSrcBack = pSrc + (count * PixelSize / sizeof(*pSrc)); + while (pDestBack > pDest) { *--pDestBack = *--pSrcBack; @@ -228,8 +238,8 @@ public: { uint32_t* pDest = reinterpret_cast(pPixelDest); const uint32_t* pSrc = reinterpret_cast(pPixelSrc); + const uint32_t* pEnd = pDest + (count * PixelSize / sizeof(*pDest)); - uint32_t* pEnd = pDest + (count * PixelSize); while (pDest < pEnd) { *pDest++ = *pSrc; @@ -240,7 +250,8 @@ public: { uint32_t* pDest = reinterpret_cast(pPixelDest); const uint32_t* pSrc = reinterpret_cast(pPixelSrc); - uint32_t* pEnd = pDest + (count * PixelSize); + const uint32_t* pEnd = pDest + (count * PixelSize / sizeof(*pDest)); + while (pDest < pEnd) { *pDest++ = *pSrc++; @@ -251,7 +262,8 @@ public: { uint32_t* pDest = reinterpret_cast(pPixelDest); const uint32_t* pSrc = reinterpret_cast(pPixelSrc); - uint32_t* pEnd = pDest + (count * PixelSize); + const uint32_t* pEnd = pDest + (count * PixelSize / sizeof(*pDest)); + while (pDest < pEnd) { *pDest++ = pgm_read_dword(pSrc++); @@ -262,8 +274,9 @@ public: { uint32_t* pDest = reinterpret_cast(pPixelDest); const uint32_t* pSrc = reinterpret_cast(pPixelSrc); - uint32_t* pDestBack = pDest + (count * PixelSize); - const uint32_t* pSrcBack = pSrc + (count * PixelSize); + uint32_t* pDestBack = pDest + (count * PixelSize / sizeof(*pDest)); + const uint32_t* pSrcBack = pSrc + (count * PixelSize / sizeof(*pSrc)); + while (pDestBack > pDest) { *--pDestBack = *--pSrcBack; @@ -392,7 +405,6 @@ public: return color; } - }; class NeoGrbwFeature : public Neo4ByteElementsNoSettings @@ -434,7 +446,6 @@ public: return color; } - }; class NeoRgbwFeature : public Neo4ByteElementsNoSettings @@ -475,7 +486,6 @@ public: return color; } - }; class NeoRgbFeature : public Neo3ByteElementsNoSettings @@ -512,8 +522,7 @@ public: color.B = pgm_read_byte(p); return color; - } - + } }; class NeoBrgFeature : public Neo3ByteElementsNoSettings @@ -550,8 +559,7 @@ public: color.G = pgm_read_byte(p); return color; - } - + } }; class NeoRbgFeature : public Neo3ByteElementsNoSettings @@ -590,7 +598,6 @@ public: return color; } - }; class NeoBgrFeature : public Neo3ByteElementsNoSettings @@ -629,7 +636,6 @@ public: return color; } - }; class NeoRgbw64Feature : public Neo8ByteElementsNoSettings @@ -713,7 +719,6 @@ typedef NeoRgb48Feature NeoRgbUcs8903Feature; typedef NeoRgbw64Feature NeoRgbwUcs8904Feature; - class NeoGrb48Feature : public Neo6ByteElementsNoSettings { public: