Skip to content

Commit cc62f2b

Browse files
Atlanclaude
andcommitted
HUB75: free the virtual mapping instead of dropping it
Review finding (CodeRabbit, 15.09.): when the arrangement changes on a re-used display, begin() cleared the pointer to the old VirtualMatrixPanel without freeing it, and the non-S3 cleanup() path deleted the display but never the mapping built on it. Both leaks are real - nothing referenced the object afterwards - though small (the object holds a handful of integers and a pointer). The delete had been disabled over -Wdelete-non-virtual-dtor: VirtualMatrixPanel has a virtual getCoords() and no virtual destructor (upstream and the sh7 fork alike). That warning concerns deleting a derived object through this pointer; here the pointer is the exact type, the class has no destructor and owns no resources, so the delete is well-defined. The warning is silenced only around the two delete statements, with the reason next to them. Built warning-free for adafruit_matrixportal_esp32s3_tinyUF2 (S3 path) and esp32_4MB_V4_S_HUB75 (classic path, which compiles the cleanup() delete). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent b9ca982 commit cc62f2b

1 file changed

Lines changed: 16 additions & 2 deletions

File tree

‎wled00/bus_manager.cpp‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1142,10 +1142,19 @@ BusHub75Matrix::BusHub75Matrix(BusConfig &bc) : Bus(bc.type, bc.start, bc.autoWh
11421142
// is not part of it. Keep the mapping only while it still describes what is configured;
11431143
// otherwise drop it, so the block below builds the right one and switching the
11441144
// arrangement off falls back to the plain chain instead of rendering through the old map.
1145-
// Not deleted on purpose: see the note next to the disabled delete in cleanup().
1145+
// The old mapping is freed here: nothing else references it once the static pointer is
1146+
// cleared. GCC warns because VirtualMatrixPanel has a virtual method but no virtual
1147+
// destructor - that warning is about deleting a *derived* object through this pointer.
1148+
// This is the exact type, the class has no destructor and owns nothing but a few
1149+
// integers and a pointer to the display, so the delete is well-defined.
11461150
if (fourScanPanel
11471151
&& !((activeVRows == vRows) && (activeVCols == vCols) && (activeVChainType == vType))) {
1152+
#pragma GCC diagnostic push
1153+
#pragma GCC diagnostic ignored "-Wdelete-non-virtual-dtor"
1154+
delete fourScanPanel;
1155+
#pragma GCC diagnostic pop
11481156
fourScanPanel = nullptr;
1157+
activeFourScanPanel = nullptr;
11491158
}
11501159

11511160
if (!fourScanPanel && ((vRows > 1) || (vCols > 1))) {
@@ -1328,8 +1337,13 @@ void BusHub75Matrix::cleanup() {
13281337
delay(30); // give some time to finish DMA
13291338
deallocatePins();
13301339
_len = 0;
1331-
//if (fourScanPanel != nullptr) delete fourScanPanel; // warning: deleting object of polymorphic class type 'VirtualMatrixPanel' which has non-virtual destructor might cause undefined behavior
13321340
#if !defined(CONFIG_IDF_TARGET_ESP32S3) // S3: don't delete, as we want to re-use the driver later
1341+
// WLEDMM: the mapping goes with the display it was built on. Same exact-type delete as in
1342+
// begin(), see the note there for why the -Wdelete-non-virtual-dtor warning does not apply.
1343+
#pragma GCC diagnostic push
1344+
#pragma GCC diagnostic ignored "-Wdelete-non-virtual-dtor"
1345+
if (fourScanPanel) delete fourScanPanel;
1346+
#pragma GCC diagnostic pop
13331347
if (display) delete display;
13341348
activeDisplay = nullptr;
13351349
activeFourScanPanel = nullptr;

0 commit comments

Comments
 (0)