From b5df6cb2b5f4415ec7ffa49b06c853a78b741f68 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 13 Mar 2026 23:45:42 +0100 Subject: [PATCH] fix: jpeg resource cleanup (#1320) ## Summary * **What is the goal of this PR?** Fix leak on decode error path in JPEG converter * **What changes are included?** Unif resource cleanup ## Additional Context --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**PARTIALLY**_ Identification of the issue by AI --- lib/JpegToBmpConverter/JpegToBmpConverter.cpp | 66 +++++++++---------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/lib/JpegToBmpConverter/JpegToBmpConverter.cpp b/lib/JpegToBmpConverter/JpegToBmpConverter.cpp index 4e674fbd..bdc368ab 100644 --- a/lib/JpegToBmpConverter/JpegToBmpConverter.cpp +++ b/lib/JpegToBmpConverter/JpegToBmpConverter.cpp @@ -278,8 +278,37 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm bytesPerRow = (outWidth * 2 + 31) / 32 * 4; } + uint8_t* rowBuffer = nullptr; + uint8_t* mcuRowBuffer = nullptr; + AtkinsonDitherer* atkinsonDitherer = nullptr; + FloydSteinbergDitherer* fsDitherer = nullptr; + Atkinson1BitDitherer* atkinson1BitDitherer = nullptr; + uint32_t* rowAccum = nullptr; // Accumulator for each output X (32-bit for larger sums) + uint32_t* rowCount = nullptr; // Count of source pixels accumulated per output X + + // RAII guard: frees all heap resources on any return path, including early exits. + // Holds references so it always sees the latest pointer values assigned below. + struct Cleanup { + uint8_t*& rowBuffer; + uint8_t*& mcuRowBuffer; + AtkinsonDitherer*& atkinsonDitherer; + FloydSteinbergDitherer*& fsDitherer; + Atkinson1BitDitherer*& atkinson1BitDitherer; + uint32_t*& rowAccum; + uint32_t*& rowCount; + ~Cleanup() { + delete[] rowAccum; + delete[] rowCount; + delete atkinsonDitherer; + delete fsDitherer; + delete atkinson1BitDitherer; + free(mcuRowBuffer); + free(rowBuffer); + } + } cleanup{rowBuffer, mcuRowBuffer, atkinsonDitherer, fsDitherer, atkinson1BitDitherer, rowAccum, rowCount}; + // Allocate row buffer - auto* rowBuffer = static_cast(malloc(bytesPerRow)); + rowBuffer = static_cast(malloc(bytesPerRow)); if (!rowBuffer) { LOG_ERR("JPG", "Failed to allocate row buffer"); return false; @@ -293,23 +322,17 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm // Validate MCU row buffer size before allocation if (mcuRowPixels > MAX_MCU_ROW_BYTES) { LOG_DBG("JPG", "MCU row buffer too large (%d bytes), max: %d", mcuRowPixels, MAX_MCU_ROW_BYTES); - free(rowBuffer); return false; } - auto* mcuRowBuffer = static_cast(malloc(mcuRowPixels)); + mcuRowBuffer = static_cast(malloc(mcuRowPixels)); if (!mcuRowBuffer) { LOG_ERR("JPG", "Failed to allocate MCU row buffer (%d bytes)", mcuRowPixels); - free(rowBuffer); return false; } // Create ditherer if enabled // Use OUTPUT dimensions for dithering (after prescaling) - AtkinsonDitherer* atkinsonDitherer = nullptr; - FloydSteinbergDitherer* fsDitherer = nullptr; - Atkinson1BitDitherer* atkinson1BitDitherer = nullptr; - if (oneBit) { // For 1-bit output, use Atkinson dithering for better quality atkinson1BitDitherer = new Atkinson1BitDitherer(outWidth); @@ -324,14 +347,12 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm // For scaling: accumulate source rows into scaled output rows // We need to track which source Y maps to which output Y // Using fixed-point: srcY_fp = outY * scaleY_fp (gives source Y in 16.16 format) - uint32_t* rowAccum = nullptr; // Accumulator for each output X (32-bit for larger sums) - uint16_t* rowCount = nullptr; // Count of source pixels accumulated per output X int currentOutY = 0; // Current output row being accumulated uint32_t nextOutY_srcStart = 0; // Source Y where next output row starts (16.16 fixed point) if (needsScaling) { rowAccum = new uint32_t[outWidth](); - rowCount = new uint16_t[outWidth](); + rowCount = new uint32_t[outWidth](); nextOutY_srcStart = scaleY_fp; // First boundary is at scaleY_fp (source Y for outY=1) } @@ -351,8 +372,6 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm } else { LOG_ERR("JPG", "JPEG decode MCU failed at (%d, %d) with error code: %d", mcuX, mcuY, mcuStatus); } - free(mcuRowBuffer); - free(rowBuffer); return false; } @@ -528,31 +547,12 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm } // Moving to next source row - reset accumulators memset(rowAccum, 0, outWidth * sizeof(uint32_t)); - memset(rowCount, 0, outWidth * sizeof(uint16_t)); + memset(rowCount, 0, outWidth * sizeof(uint32_t)); } } } } - // Clean up - if (rowAccum) { - delete[] rowAccum; - } - if (rowCount) { - delete[] rowCount; - } - if (atkinsonDitherer) { - delete atkinsonDitherer; - } - if (fsDitherer) { - delete fsDitherer; - } - if (atkinson1BitDitherer) { - delete atkinson1BitDitherer; - } - free(mcuRowBuffer); - free(rowBuffer); - LOG_DBG("JPG", "Successfully converted JPEG to BMP"); return true; }