Skip to content

Commit 9f95020

Browse files
Dutchman101Dutchman101
committed
Reduce a specific, high-hitting crash caused by mem abuse
1 parent 58877e6 commit 9f95020

File tree

2 files changed

+185
-106
lines changed

2 files changed

+185
-106
lines changed

‎Client/core/CFileFormatJpeg.cpp‎

Lines changed: 69 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -80,37 +80,45 @@ bool JpegDecode(const void* pData, uint uiDataSize, CBuffer* pOutBuffer, uint& u
8080
uiOutWidth = uiWidth;
8181
uiOutHeight = uiHeight;
8282

83-
if (pOutBuffer)
83+
try
8484
{
85-
pOutBuffer->SetSize(uiWidth * uiHeight * 4);
86-
char* pOutData = pOutBuffer->GetData();
87-
88-
/* Process data */
89-
JSAMPROW row_pointer[1];
90-
CBuffer rowBuffer;
91-
rowBuffer.SetSize(uiWidth * 3);
92-
char* pRowTemp = rowBuffer.GetData();
93-
94-
while (cinfo.output_scanline < cinfo.output_height)
85+
if (pOutBuffer)
9586
{
96-
BYTE* pRowDest = (BYTE*)pOutData + cinfo.output_scanline * uiWidth * 4;
97-
row_pointer[0] = (JSAMPROW)pRowTemp;
87+
pOutBuffer->SetSize(uiWidth * uiHeight * 4);
88+
char* pOutData = pOutBuffer->GetData();
9889

99-
JDIMENSION num_scanlines = jpeg_read_scanlines(&cinfo, row_pointer, 1);
90+
/* Process data */
91+
JSAMPROW row_pointer[1];
92+
CBuffer rowBuffer;
93+
rowBuffer.SetSize(uiWidth * 3);
94+
char* pRowTemp = rowBuffer.GetData();
10095

101-
for (uint i = 0; i < uiWidth; i++)
96+
while (cinfo.output_scanline < cinfo.output_height)
10297
{
103-
pRowDest[i * 4 + 0] = pRowTemp[i * 3 + 2];
104-
pRowDest[i * 4 + 1] = pRowTemp[i * 3 + 1];
105-
pRowDest[i * 4 + 2] = pRowTemp[i * 3 + 0];
106-
pRowDest[i * 4 + 3] = 255;
98+
BYTE* pRowDest = (BYTE*)pOutData + cinfo.output_scanline * uiWidth * 4;
99+
row_pointer[0] = (JSAMPROW)pRowTemp;
100+
101+
JDIMENSION num_scanlines = jpeg_read_scanlines(&cinfo, row_pointer, 1);
102+
103+
for (uint i = 0; i < uiWidth; i++)
104+
{
105+
pRowDest[i * 4 + 0] = pRowTemp[i * 3 + 2];
106+
pRowDest[i * 4 + 1] = pRowTemp[i * 3 + 1];
107+
pRowDest[i * 4 + 2] = pRowTemp[i * 3 + 0];
108+
pRowDest[i * 4 + 3] = 255;
109+
}
107110
}
108111
}
109-
110-
// Finish decompression and release memory.
111-
jpeg_finish_decompress(&cinfo);
112+
}
113+
catch (...)
114+
{
115+
jpeg_destroy_decompress(&cinfo);
116+
throw;
112117
}
113118

119+
// Finish decompression and release memory.
120+
// Must always be called after jpeg_start_decompress()
121+
jpeg_finish_decompress(&cinfo);
114122
jpeg_destroy_decompress(&cinfo);
115123

116124
if (jerr.num_warnings)
@@ -151,34 +159,52 @@ bool JpegEncode(uint uiWidth, uint uiHeight, uint uiQuality, const void* pData,
151159
/* Start compressor */
152160
jpeg_start_compress(&cinfo, TRUE);
153161

154-
/* Process data */
155-
CBuffer rowBuffer;
156-
rowBuffer.SetSize(uiWidth * 3);
157-
char* pRowTemp = rowBuffer.GetData();
158-
159-
JSAMPROW row_pointer[1];
160-
while (cinfo.next_scanline < cinfo.image_height)
162+
bool bSuccess = false;
163+
try
161164
{
162-
BYTE* pRowSrc = (BYTE*)pData + cinfo.next_scanline * uiWidth * 4;
163-
for (uint i = 0; i < uiWidth; i++)
165+
/* Process data */
166+
CBuffer rowBuffer;
167+
rowBuffer.SetSize(uiWidth * 3);
168+
char* pRowTemp = rowBuffer.GetData();
169+
170+
JSAMPROW row_pointer[1];
171+
while (cinfo.next_scanline < cinfo.image_height)
172+
{
173+
BYTE* pRowSrc = (BYTE*)pData + cinfo.next_scanline * uiWidth * 4;
174+
for (uint i = 0; i < uiWidth; i++)
175+
{
176+
pRowTemp[i * 3 + 0] = pRowSrc[i * 4 + 2];
177+
pRowTemp[i * 3 + 1] = pRowSrc[i * 4 + 1];
178+
pRowTemp[i * 3 + 2] = pRowSrc[i * 4 + 0];
179+
}
180+
row_pointer[0] = (JSAMPROW)pRowTemp;
181+
jpeg_write_scanlines(&cinfo, row_pointer, 1);
182+
}
183+
184+
/* Finish compression and release memory */
185+
jpeg_finish_compress(&cinfo);
186+
187+
// Copy out data and free memory
188+
if (membuffer)
164189
{
165-
pRowTemp[i * 3 + 0] = pRowSrc[i * 4 + 2];
166-
pRowTemp[i * 3 + 1] = pRowSrc[i * 4 + 1];
167-
pRowTemp[i * 3 + 2] = pRowSrc[i * 4 + 0];
190+
outBuffer = CBuffer(membuffer, memlen);
191+
free(membuffer);
192+
membuffer = NULL;
193+
bSuccess = true;
168194
}
169-
row_pointer[0] = (JSAMPROW)pRowTemp;
170-
jpeg_write_scanlines(&cinfo, row_pointer, 1);
195+
}
196+
catch (...)
197+
{
198+
// Note: Do not call jpeg_finish_compress() on error - it would write garbage
199+
jpeg_destroy_compress(&cinfo);
200+
if (membuffer)
201+
free(membuffer);
202+
throw;
171203
}
172204

173-
/* Finish compression and release memory */
174-
jpeg_finish_compress(&cinfo);
175205
jpeg_destroy_compress(&cinfo);
176206

177-
// Copy out data and free memory
178-
outBuffer = CBuffer(membuffer, memlen);
179-
free(membuffer);
180-
181-
return true;
207+
return bSuccess;
182208
}
183209

184210
///////////////////////////////////////////////////////////////

‎Client/core/CFileFormatPng.cpp‎

Lines changed: 116 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -46,23 +46,31 @@ void ParseRGBA(CBuffer& outImage, const png_structp& png_ptr, const png_infop& i
4646
const png_uint_32 bytesPerRow = png_get_rowbytes(png_ptr, info_ptr);
4747
byte* rowData = new byte[bytesPerRow];
4848

49-
// read single row at a time
50-
for (uint rowIdx = 0; rowIdx < height; ++rowIdx)
49+
try
5150
{
52-
png_read_row(png_ptr, (png_bytep)rowData, NULL);
53-
54-
const uint rowOffset = rowIdx * width;
55-
56-
uint byteIndex = 0;
57-
for (uint colIdx = 0; colIdx < width; ++colIdx)
51+
// read single row at a time
52+
for (uint rowIdx = 0; rowIdx < height; ++rowIdx)
5853
{
59-
pData->R = rowData[byteIndex++];
60-
pData->G = rowData[byteIndex++];
61-
pData->B = rowData[byteIndex++];
62-
pData->A = rowData[byteIndex++];
63-
pData++;
54+
png_read_row(png_ptr, (png_bytep)rowData, NULL);
55+
56+
const uint rowOffset = rowIdx * width;
57+
58+
uint byteIndex = 0;
59+
for (uint colIdx = 0; colIdx < width; ++colIdx)
60+
{
61+
pData->R = rowData[byteIndex++];
62+
pData->G = rowData[byteIndex++];
63+
pData->B = rowData[byteIndex++];
64+
pData->A = rowData[byteIndex++];
65+
pData++;
66+
}
67+
assert(byteIndex == bytesPerRow);
6468
}
65-
assert(byteIndex == bytesPerRow);
69+
}
70+
catch (...)
71+
{
72+
delete[] rowData;
73+
throw;
6674
}
6775

6876
delete[] rowData;
@@ -81,23 +89,31 @@ void ParseRGB(CBuffer& outImage, const png_structp& png_ptr, const png_infop& in
8189
const png_uint_32 bytesPerRow = png_get_rowbytes(png_ptr, info_ptr);
8290
byte* rowData = new byte[bytesPerRow];
8391

84-
// read single row at a time
85-
for (uint rowIdx = 0; rowIdx < height; ++rowIdx)
92+
try
8693
{
87-
png_read_row(png_ptr, (png_bytep)rowData, NULL);
88-
89-
const uint rowOffset = rowIdx * width;
90-
91-
uint byteIndex = 0;
92-
for (uint colIdx = 0; colIdx < width; ++colIdx)
94+
// read single row at a time
95+
for (uint rowIdx = 0; rowIdx < height; ++rowIdx)
9396
{
94-
pData->R = rowData[byteIndex++];
95-
pData->G = rowData[byteIndex++];
96-
pData->B = rowData[byteIndex++];
97-
pData->A = 255;
98-
pData++;
97+
png_read_row(png_ptr, (png_bytep)rowData, NULL);
98+
99+
const uint rowOffset = rowIdx * width;
100+
101+
uint byteIndex = 0;
102+
for (uint colIdx = 0; colIdx < width; ++colIdx)
103+
{
104+
pData->R = rowData[byteIndex++];
105+
pData->G = rowData[byteIndex++];
106+
pData->B = rowData[byteIndex++];
107+
pData->A = 255;
108+
pData++;
109+
}
110+
assert(byteIndex == bytesPerRow);
99111
}
100-
assert(byteIndex == bytesPerRow);
112+
}
113+
catch (...)
114+
{
115+
delete[] rowData;
116+
throw;
101117
}
102118

103119
delete[] rowData;
@@ -192,21 +208,29 @@ bool PngDecode(const void* pData, uint uiDataSize, CBuffer* pOutBuffer, uint& ui
192208

193209
if (pOutBuffer)
194210
{
195-
pOutBuffer->SetSize(width * height * 4);
196-
197-
switch (colorType)
211+
try
198212
{
199-
case PNG_COLOR_TYPE_RGB:
200-
ParseRGB(*pOutBuffer, png_ptr, info_ptr, width, height);
201-
break;
202-
203-
case PNG_COLOR_TYPE_RGB_ALPHA:
204-
ParseRGBA(*pOutBuffer, png_ptr, info_ptr, width, height);
205-
break;
206-
207-
default:
208-
png_destroy_read_struct(&png_ptr, &info_ptr, NULL);
209-
return false;
213+
pOutBuffer->SetSize(width * height * 4);
214+
215+
switch (colorType)
216+
{
217+
case PNG_COLOR_TYPE_RGB:
218+
ParseRGB(*pOutBuffer, png_ptr, info_ptr, width, height);
219+
break;
220+
221+
case PNG_COLOR_TYPE_RGB_ALPHA:
222+
ParseRGBA(*pOutBuffer, png_ptr, info_ptr, width, height);
223+
break;
224+
225+
default:
226+
png_destroy_read_struct(&png_ptr, &info_ptr, NULL);
227+
return false;
228+
}
229+
}
230+
catch (...)
231+
{
232+
png_destroy_read_struct(&png_ptr, &info_ptr, NULL);
233+
throw;
210234
}
211235
}
212236

@@ -244,38 +268,67 @@ void my_png_write_data(png_structp png_ptr, png_bytep data, png_size_t length)
244268
///////////////////////////////////////////////////////////////
245269
bool PngEncode(uint uiWidth, uint uiHeight, const void* pData, uint uiDataSize, CBuffer& outBuffer)
246270
{
247-
// Create the screen data buffer
248271
BYTE** ppScreenData = NULL;
249-
ppScreenData = new BYTE*[uiHeight];
250-
for (unsigned short y = 0; y < uiHeight; y++)
272+
png_struct* png_ptr = NULL;
273+
png_info* info_ptr = NULL;
274+
275+
try
251276
{
252-
ppScreenData[y] = new BYTE[uiWidth * 4];
253-
}
277+
// Create the screen data buffer
278+
ppScreenData = new BYTE*[uiHeight];
279+
memset(ppScreenData, 0, sizeof(BYTE*) * uiHeight);
280+
for (uint y = 0; y < uiHeight; y++)
281+
{
282+
ppScreenData[y] = new BYTE[uiWidth * 4];
283+
}
284+
285+
// Copy the surface data into a row-based buffer for libpng
286+
unsigned long ulLineWidth = uiWidth * 4;
287+
for (unsigned int i = 0; i < uiHeight; i++)
288+
{
289+
memcpy(ppScreenData[i], (BYTE*)pData + i * ulLineWidth, ulLineWidth);
290+
}
291+
292+
CBufferWriteStream stream(outBuffer);
254293

255-
// Copy the surface data into a row-based buffer for libpng
256-
unsigned long ulLineWidth = uiWidth * 4;
257-
for (unsigned int i = 0; i < uiHeight; i++)
294+
png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
295+
if (!png_ptr)
296+
throw std::bad_alloc();
297+
298+
info_ptr = png_create_info_struct(png_ptr);
299+
if (!info_ptr)
300+
throw std::bad_alloc();
301+
302+
png_set_write_fn(png_ptr, &stream, my_png_write_data, NULL);
303+
png_set_filter(png_ptr, 0, PNG_FILTER_NONE);
304+
png_set_compression_level(png_ptr, 1);
305+
png_set_IHDR(png_ptr, info_ptr, uiWidth, uiHeight, 8, PNG_COLOR_TYPE_RGB_ALPHA, PNG_INTERLACE_NONE, PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
306+
png_set_rows(png_ptr, info_ptr, ppScreenData);
307+
png_write_png(png_ptr, info_ptr, PNG_TRANSFORM_BGR /*| PNG_TRANSFORM_STRIP_ALPHA*/, NULL);
308+
png_write_end(png_ptr, info_ptr);
309+
}
310+
catch (...)
258311
{
259-
memcpy(ppScreenData[i], (BYTE*)pData + i * ulLineWidth, ulLineWidth);
312+
if (png_ptr)
313+
png_destroy_write_struct(&png_ptr, info_ptr ? &info_ptr : NULL);
314+
315+
if (ppScreenData)
316+
{
317+
for (uint y = 0; y < uiHeight; y++)
318+
{
319+
if (ppScreenData[y]) delete[] ppScreenData[y];
320+
}
321+
delete[] ppScreenData;
322+
}
323+
throw;
260324
}
261325

262-
CBufferWriteStream stream(outBuffer);
263-
264-
png_struct* png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
265-
png_info* info_ptr = png_create_info_struct(png_ptr);
266-
png_set_write_fn(png_ptr, &stream, my_png_write_data, NULL);
267-
png_set_filter(png_ptr, 0, PNG_FILTER_NONE);
268-
png_set_compression_level(png_ptr, 1);
269-
png_set_IHDR(png_ptr, info_ptr, uiWidth, uiHeight, 8, PNG_COLOR_TYPE_RGB_ALPHA, PNG_INTERLACE_NONE, PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
270-
png_set_rows(png_ptr, info_ptr, ppScreenData);
271-
png_write_png(png_ptr, info_ptr, PNG_TRANSFORM_BGR /*| PNG_TRANSFORM_STRIP_ALPHA*/, NULL);
272-
png_write_end(png_ptr, info_ptr);
273326
png_destroy_write_struct(&png_ptr, &info_ptr);
274327

275328
// Clean up the screen data buffer
276329
if (ppScreenData)
277330
{
278-
for (unsigned short y = 0; y < uiHeight; y++)
331+
for (uint y = 0; y < uiHeight; y++)
279332
{
280333
delete[] ppScreenData[y];
281334
}

0 commit comments

Comments
 (0)