improve segment code robustness

* Avoid uint16 underflow in width() and height(): stop > start is possible, and means "inactive segment".

* use size_t for ledsrgbSize, _dataLen and _usedSegmentData -  uint16_t could overflow in some situations.

* try to catch attempts to allocate zero bytes (inactive segment => size 0)
This commit is contained in:
Frank
2023-06-21 01:43:26 +02:00
parent 708cd8e73d
commit 310daa61a8
4 changed files with 40 additions and 29 deletions

View File

@@ -31,6 +31,7 @@
#include "const.h"
bool canUseSerial(void); // WLEDMM implemented in wled_serial.cpp
void strip_wait_until_idle(String whoCalledMe); // WLEDMM implemented in FX_fcn.cpp
#define FASTLED_INTERNAL //remove annoying pragma messages
@@ -398,7 +399,7 @@ typedef struct Segment {
uint16_t aux1; // custom var
byte* data; // effect data pointer
CRGB* ledsrgb; // local leds[] array (may be a pointer to global) //WLEDMM rename to ledsrgb to search on them (temp?)
uint16_t ledsrgbSize; //WLEDMM
size_t ledsrgbSize; //WLEDMM
static CRGB *_globalLeds; // global leds[] array
static uint16_t maxWidth, maxHeight; // these define matrix width & height (max. segment dimensions)
void *jMap = nullptr; //WLEDMM jMap
@@ -414,8 +415,8 @@ typedef struct Segment {
uint8_t _reserved : 4;
};
};
uint16_t _dataLen;
static uint16_t _usedSegmentData;
size_t _dataLen; // WLEDMM uint16_t is too small
static size_t _usedSegmentData; // WLEDMM uint16_t is too small
// transition data, valid only if transitional==true, holds values during transition
struct Transition {
@@ -502,11 +503,13 @@ typedef struct Segment {
~Segment() {
//#ifdef WLED_DEBUG
Serial.print(F("Destroying segment:"));
if (name) Serial.printf(" %s (%p)", name, name);
if (data) Serial.printf(" %d (%p)", (int)_dataLen, data);
if (ledsrgb) Serial.printf(" [%u]", length()*sizeof(CRGB));
Serial.println();
if(canUseSerial()) {
Serial.print(F("Destroying segment:"));
if (name) Serial.printf(" name=%s (%p)", name, name);
if (data) Serial.printf(" dataLen=%d (%p)", (int)_dataLen, data);
if (ledsrgb) Serial.printf(" [%sledsrgb %u bytes]", Segment::_globalLeds ? "global ":"",length()*sizeof(CRGB));
Serial.println();
}
//#endif
// WLEDMM only delete segments when they are not in use
@@ -514,7 +517,7 @@ typedef struct Segment {
strip_wait_until_idle("~Segment()");
#endif
if (!Segment::_globalLeds && ledsrgb) free(ledsrgb);
if (!Segment::_globalLeds && (ledsrgb != nullptr)) {free(ledsrgb); ledsrgb = nullptr;}
if (name) delete[] name;
if (_t) delete _t;
deallocateData();
@@ -534,13 +537,13 @@ typedef struct Segment {
inline bool hasRGB(void) const { return _isRGB; }
inline bool hasWhite(void) const { return _hasW; }
inline bool isCCT(void) const { return _isCCT; }
inline uint16_t width(void) const { return stop - start; } // segment width in physical pixels (length if 1D)
inline uint16_t height(void) const { return max(1, stopY - startY); } // segment height (if 2D) in physical pixels // WLEDMM make sure its always > 0
inline uint16_t width(void) const { return (stop > start) ? (stop - start) : 0; } // segment width in physical pixels (length if 1D)
inline uint16_t height(void) const { return (stopY > startY) ? (stopY - startY) : 0; } // segment height (if 2D) in physical pixels // WLEDMM make sure its always > 0
inline uint16_t length(void) const { return width() * height(); } // segment length (count) in physical pixels
inline uint16_t groupLength(void) const { return grouping + spacing; }
inline uint8_t getLightCapabilities(void) const { return _capabilities; }
static uint16_t getUsedSegmentData(void) { return _usedSegmentData; }
static size_t getUsedSegmentData(void) { return _usedSegmentData; } // WLEDMM size_t
static void addUsedSegmentData(int len) { _usedSegmentData += len; }
void allocLeds(); //WLEDMM
@@ -556,7 +559,7 @@ typedef struct Segment {
void refreshLightCapabilities(void);
// runtime data functions
inline uint16_t dataSize(void) const { return _dataLen; }
inline size_t dataSize(void) const { return _dataLen; }
bool allocateData(size_t len);
void deallocateData(void);
void resetIfRequired(void);

View File

@@ -67,16 +67,16 @@ void WS2812FX::setUpMatrix() {
//WLEDMM recreate customMappingTable if more space needed
if (Segment::maxWidth * Segment::maxHeight > customMappingTableSize) {
uint32_t size = MAX(ledmapMaxSize, Segment::maxWidth * Segment::maxHeight);//TroyHack
size_t size = max(ledmapMaxSize, size_t(Segment::maxWidth * Segment::maxHeight));//TroyHack
USER_PRINTF("setupmatrix customMappingTable alloc %d from %d\n", size, customMappingTableSize);
//if (customMappingTable != nullptr) delete[] customMappingTable;
//customMappingTable = new uint16_t[size];
// don't use new / delete
if (customMappingTable != nullptr) { // resize
if ((size > 0) && (customMappingTable != nullptr)) { // resize
customMappingTable = (uint16_t*) reallocf(customMappingTable, sizeof(uint16_t) * size); // reallocf will free memory if it cannot resize
}
if (customMappingTable == nullptr) { // second try
if ((size > 0) && (customMappingTable == nullptr)) { // second try
DEBUG_PRINTLN("setUpMatrix: trying to get fresh memory block.");
customMappingTable = (uint16_t*) calloc(size, sizeof(uint16_t));
if (customMappingTable == nullptr) DEBUG_PRINTLN("setUpMatrix: alloc failed");
@@ -144,7 +144,7 @@ void WS2812FX::setUpMatrix() {
}
// delete gap array as we no longer need it
if (gapTable) delete[] gapTable;
if (gapTable) {delete[] gapTable; gapTable=nullptr;} // softhack prevent dangling pointer
#ifdef WLED_DEBUG_MAPS
DEBUG_PRINTF("Matrix ledmap: \n");

View File

@@ -87,7 +87,7 @@ void strip_wait_until_idle(String whoCalledMe) {
///////////////////////////////////////////////////////////////////////////////
// Segment class implementation
///////////////////////////////////////////////////////////////////////////////
uint16_t Segment::_usedSegmentData = 0U; // amount of RAM all segments use for their data[]
size_t Segment::_usedSegmentData = 0U; // amount of RAM all segments use for their data[]
CRGB *Segment::_globalLeds = nullptr;
uint16_t Segment::maxWidth = DEFAULT_LED_COUNT;
uint16_t Segment::maxHeight = 1;
@@ -110,14 +110,22 @@ Segment::Segment(const Segment &orig) {
//WLEDMM: recreate ledsrgb if more space needed (will not free ledsrgb!)
void Segment::allocLeds() {
uint16_t size = sizeof(CRGB)*MAX(length(), ledmapMaxSize); //TroyHack
if (!ledsrgb || size > ledsrgbSize) {
USER_PRINTF("allocLeds %d from %d\n", size, ledsrgb?ledsrgbSize:0);
size_t size = sizeof(CRGB)*max((size_t) length(), ledmapMaxSize); //TroyHack
if ((size < sizeof(CRGB)) || (size > 164000)) { //softhack too small (<3) or too large (>160Kb)
USER_PRINTF("allocLeds warning: size == %u !!\n", size);
USER_FLUSH();
if (ledsrgb && (ledsrgbSize == 0)) {
USER_PRINTLN("allocLeds warning: ledsrgbSize == 0 but ledsrgb!=NULL");
free(ledsrgb); ledsrgb=nullptr;
} // softhack007 clean up buffer
}
if ((size > 0) && (!ledsrgb || size > ledsrgbSize)) { //softhack dont allocate zero bytes
USER_PRINTF("allocLeds %u from %u\n", size, ledsrgb?ledsrgbSize:0);
ledsrgb = (CRGB*)malloc(size);
ledsrgbSize = ledsrgb?size:0;
}
else {
USER_PRINTF("reuse Leds %d from %d\n", size, ledsrgb?ledsrgbSize:0);
USER_PRINTF("reuse Leds %u from %u\n", size, ledsrgb?ledsrgbSize:0);
}
}
@@ -143,7 +151,7 @@ Segment& Segment::operator= (const Segment &orig) {
// WLEDMM reuse leds instead of removing themn
// if (ledsrgb && !Segment::_globalLeds) free(ledsrgb); //WLEDMM: nullify below!
CRGB* oldLeds = ledsrgb;
uint16_t oldLedsSize = ledsrgbSize;
size_t oldLedsSize = ledsrgbSize;
deallocateData();
// copy source
memcpy((void*)this, (void*)&orig, sizeof(Segment));
@@ -2249,16 +2257,16 @@ bool WS2812FX::deserializeMap(uint8_t n) {
//WLEDMM recreate customMappingTable if more space needed
if (Segment::maxWidth * Segment::maxHeight > customMappingTableSize) {
uint32_t size = MAX(ledmapMaxSize, Segment::maxWidth * Segment::maxHeight);//TroyHack
USER_PRINTF("deserializemap customMappingTable alloc %d from %d\n", size, customMappingTableSize);
size_t size = max(ledmapMaxSize, size_t(Segment::maxWidth * Segment::maxHeight));//TroyHack
USER_PRINTF("deserializemap customMappingTable alloc %u from %u\n", size, customMappingTableSize);
//if (customMappingTable != nullptr) delete[] customMappingTable;
//customMappingTable = new uint16_t[size];
// don't use new / delete
if (customMappingTable != nullptr) {
if ((size > 0) && (customMappingTable != nullptr)) {
customMappingTable = (uint16_t*) reallocf(customMappingTable, sizeof(uint16_t) * size); // reallocf will free memory if it cannot resize
}
if (customMappingTable == nullptr) { // second try
if ((size > 0) && (customMappingTable == nullptr)) { // second try
DEBUG_PRINTLN("deserializeMap: trying to get fresh memory block.");
customMappingTable = (uint16_t*) calloc(size, sizeof(uint16_t));
if (customMappingTable == nullptr) DEBUG_PRINTLN("deserializeMap: alloc failed!");

View File

@@ -8,7 +8,7 @@
*/
// version code in format yymmddb (b = daily build)
#define VERSION 2306160
#define VERSION 2306210
//uncomment this if you have a "my_config.h" file you'd like to use
//#define WLED_USE_MY_CONFIG
@@ -703,7 +703,7 @@ WLED_GLOBAL volatile bool suspendStripService _INIT(false); // WLEDMM temporaril
#ifndef ESP8266
WLED_GLOBAL char *ledmapNames[WLED_MAX_LEDMAPS-1] _INIT_N(({nullptr}));
#endif
WLED_GLOBAL uint16_t ledmapMaxSize _INIT(0); //WLEDMM TroyHack
WLED_GLOBAL size_t ledmapMaxSize _INIT(0); //WLEDMM TroyHack
#if WLED_MAX_LEDMAPS>16
WLED_GLOBAL uint32_t ledMaps _INIT(0); // bitfield representation of available ledmaps
#else