mirror of
https://github.com/luanti-org/luanti.git
synced 2025-06-27 16:36:03 +00:00
Less explicit memory management in Irrlicht image writer classes (#15493)
CImageWriterPNG::writeImage() and writeJPEGFile() explicitly allocate and deallocate memory with `new` and `delete`, which is prone to programming errors. The code also has non-functional error handling: When memory allocation fails, `new` throws an `std::bad_alloc` exception and never returns `nullptr`, so the check for `nullptr` is always false. Co-authored-by: DS <ds.desour@proton.me>
This commit is contained in:
parent
03813a5b5e
commit
a45b04ffb4
2 changed files with 34 additions and 46 deletions
|
@ -10,6 +10,7 @@
|
||||||
#include "os.h"
|
#include "os.h"
|
||||||
|
|
||||||
#include <cstdio> // IWYU pragma: keep (required for jpeglib.h)
|
#include <cstdio> // IWYU pragma: keep (required for jpeglib.h)
|
||||||
|
#include <memory>
|
||||||
#include <jpeglib.h>
|
#include <jpeglib.h>
|
||||||
#include <jerror.h>
|
#include <jerror.h>
|
||||||
|
|
||||||
|
@ -130,32 +131,28 @@ static bool writeJPEGFile(io::IWriteFile *file, IImage *image, u32 quality)
|
||||||
jpeg_set_quality(&cinfo, quality, TRUE);
|
jpeg_set_quality(&cinfo, quality, TRUE);
|
||||||
jpeg_start_compress(&cinfo, TRUE);
|
jpeg_start_compress(&cinfo, TRUE);
|
||||||
|
|
||||||
u8 *dest = new u8[dim.Width * 3];
|
std::unique_ptr<u8[]> dest{new u8[dim.Width * 3]};
|
||||||
|
|
||||||
if (dest) {
|
const u32 pitch = image->getPitch();
|
||||||
const u32 pitch = image->getPitch();
|
JSAMPROW row_pointer[1]; /* pointer to JSAMPLE row[s] */
|
||||||
JSAMPROW row_pointer[1]; /* pointer to JSAMPLE row[s] */
|
row_pointer[0] = dest.get();
|
||||||
row_pointer[0] = dest;
|
|
||||||
|
|
||||||
u8 *src = (u8 *)image->getData();
|
u8 *src = (u8 *)image->getData();
|
||||||
|
|
||||||
while (cinfo.next_scanline < cinfo.image_height) {
|
while (cinfo.next_scanline < cinfo.image_height) {
|
||||||
// convert next line
|
// convert next line
|
||||||
format(src, dim.Width, dest);
|
format(src, dim.Width, dest.get());
|
||||||
src += pitch;
|
src += pitch;
|
||||||
jpeg_write_scanlines(&cinfo, row_pointer, 1);
|
jpeg_write_scanlines(&cinfo, row_pointer, 1);
|
||||||
}
|
|
||||||
|
|
||||||
delete[] dest;
|
|
||||||
|
|
||||||
/* Step 6: Finish compression */
|
|
||||||
jpeg_finish_compress(&cinfo);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Step 6: Finish compression */
|
||||||
|
jpeg_finish_compress(&cinfo);
|
||||||
|
|
||||||
/* Step 7: Destroy */
|
/* Step 7: Destroy */
|
||||||
jpeg_destroy_compress(&cinfo);
|
jpeg_destroy_compress(&cinfo);
|
||||||
|
|
||||||
return (dest != 0);
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
} // namespace video
|
} // namespace video
|
||||||
|
|
|
@ -10,6 +10,7 @@
|
||||||
#include "os.h" // for logging
|
#include "os.h" // for logging
|
||||||
|
|
||||||
#include <png.h> // use system lib png
|
#include <png.h> // use system lib png
|
||||||
|
#include <memory>
|
||||||
|
|
||||||
namespace irr
|
namespace irr
|
||||||
{
|
{
|
||||||
|
@ -94,22 +95,23 @@ bool CImageWriterPNG::writeImage(io::IWriteFile *file, IImage *image, u32 param)
|
||||||
png_set_write_fn(png_ptr, file, user_write_data_fcn, NULL);
|
png_set_write_fn(png_ptr, file, user_write_data_fcn, NULL);
|
||||||
|
|
||||||
// Set info
|
// Set info
|
||||||
|
core::dimension2d<u32> img_dim = image->getDimension();
|
||||||
switch (image->getColorFormat()) {
|
switch (image->getColorFormat()) {
|
||||||
case ECF_A8R8G8B8:
|
case ECF_A8R8G8B8:
|
||||||
case ECF_A1R5G5B5:
|
case ECF_A1R5G5B5:
|
||||||
png_set_IHDR(png_ptr, info_ptr,
|
png_set_IHDR(png_ptr, info_ptr,
|
||||||
image->getDimension().Width, image->getDimension().Height,
|
img_dim.Width, img_dim.Height,
|
||||||
8, PNG_COLOR_TYPE_RGB_ALPHA, PNG_INTERLACE_NONE,
|
8, PNG_COLOR_TYPE_RGB_ALPHA, PNG_INTERLACE_NONE,
|
||||||
PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
|
PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
|
||||||
break;
|
break;
|
||||||
default:
|
default:
|
||||||
png_set_IHDR(png_ptr, info_ptr,
|
png_set_IHDR(png_ptr, info_ptr,
|
||||||
image->getDimension().Width, image->getDimension().Height,
|
img_dim.Width, img_dim.Height,
|
||||||
8, PNG_COLOR_TYPE_RGB, PNG_INTERLACE_NONE,
|
8, PNG_COLOR_TYPE_RGB, PNG_INTERLACE_NONE,
|
||||||
PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
|
PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
|
||||||
}
|
}
|
||||||
|
|
||||||
s32 lineWidth = image->getDimension().Width;
|
s32 lineWidth = img_dim.Width;
|
||||||
switch (image->getColorFormat()) {
|
switch (image->getColorFormat()) {
|
||||||
case ECF_R8G8B8:
|
case ECF_R8G8B8:
|
||||||
case ECF_R5G6B5:
|
case ECF_R5G6B5:
|
||||||
|
@ -123,61 +125,52 @@ bool CImageWriterPNG::writeImage(io::IWriteFile *file, IImage *image, u32 param)
|
||||||
default:
|
default:
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
u8 *tmpImage = new u8[image->getDimension().Height * lineWidth];
|
std::unique_ptr<u8[]> tmpImage{new u8[img_dim.Height * lineWidth]};
|
||||||
if (!tmpImage) {
|
|
||||||
os::Printer::log("PNGWriter: Internal PNG create image failure", file->getFileName(), ELL_ERROR);
|
|
||||||
png_destroy_write_struct(&png_ptr, &info_ptr);
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
|
|
||||||
|
auto num_pixels = img_dim.Height * img_dim.Width;
|
||||||
u8 *data = (u8 *)image->getData();
|
u8 *data = (u8 *)image->getData();
|
||||||
switch (image->getColorFormat()) {
|
switch (image->getColorFormat()) {
|
||||||
case ECF_R8G8B8:
|
case ECF_R8G8B8:
|
||||||
CColorConverter::convert_R8G8B8toR8G8B8(data, image->getDimension().Height * image->getDimension().Width, tmpImage);
|
CColorConverter::convert_R8G8B8toR8G8B8(data, num_pixels,
|
||||||
|
tmpImage.get());
|
||||||
break;
|
break;
|
||||||
case ECF_A8R8G8B8:
|
case ECF_A8R8G8B8:
|
||||||
CColorConverter::convert_A8R8G8B8toA8R8G8B8(data, image->getDimension().Height * image->getDimension().Width, tmpImage);
|
CColorConverter::convert_A8R8G8B8toA8R8G8B8(data, num_pixels,
|
||||||
|
tmpImage.get());
|
||||||
break;
|
break;
|
||||||
case ECF_R5G6B5:
|
case ECF_R5G6B5:
|
||||||
CColorConverter::convert_R5G6B5toR8G8B8(data, image->getDimension().Height * image->getDimension().Width, tmpImage);
|
CColorConverter::convert_R5G6B5toR8G8B8(data, num_pixels,
|
||||||
|
tmpImage.get());
|
||||||
break;
|
break;
|
||||||
case ECF_A1R5G5B5:
|
case ECF_A1R5G5B5:
|
||||||
CColorConverter::convert_A1R5G5B5toA8R8G8B8(data, image->getDimension().Height * image->getDimension().Width, tmpImage);
|
CColorConverter::convert_A1R5G5B5toA8R8G8B8(data, num_pixels,
|
||||||
|
tmpImage.get());
|
||||||
break;
|
break;
|
||||||
// TODO: Error handling in case of unsupported color format
|
// TODO: Error handling in case of unsupported color format
|
||||||
default:
|
default:
|
||||||
os::Printer::log("CImageWriterPNG does not support image format", ColorFormatNames[image->getColorFormat()], ELL_WARNING);
|
os::Printer::log("CImageWriterPNG does not support image format", ColorFormatNames[image->getColorFormat()], ELL_WARNING);
|
||||||
png_destroy_write_struct(&png_ptr, &info_ptr);
|
png_destroy_write_struct(&png_ptr, &info_ptr);
|
||||||
delete[] tmpImage;
|
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Create array of pointers to rows in image data
|
// Create array of pointers to rows in image data
|
||||||
|
|
||||||
// Used to point to image rows
|
// Used to point to image rows
|
||||||
u8 **RowPointers = new png_bytep[image->getDimension().Height];
|
std::unique_ptr<u8*[]> RowPointers{new u8*[img_dim.Height]};
|
||||||
if (!RowPointers) {
|
|
||||||
os::Printer::log("PNGWriter: Internal PNG create row pointers failure", file->getFileName(), ELL_ERROR);
|
|
||||||
png_destroy_write_struct(&png_ptr, &info_ptr);
|
|
||||||
delete[] tmpImage;
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
|
|
||||||
data = tmpImage;
|
data = tmpImage.get();
|
||||||
// Fill array of pointers to rows in image data
|
// Fill array of pointers to rows in image data
|
||||||
for (u32 i = 0; i < image->getDimension().Height; ++i) {
|
for (u32 i = 0; i < img_dim.Height; ++i) {
|
||||||
RowPointers[i] = data;
|
RowPointers[i] = data;
|
||||||
data += lineWidth;
|
data += lineWidth;
|
||||||
}
|
}
|
||||||
// for proper error handling
|
// for proper error handling
|
||||||
if (setjmp(png_jmpbuf(png_ptr))) {
|
if (setjmp(png_jmpbuf(png_ptr))) {
|
||||||
png_destroy_write_struct(&png_ptr, &info_ptr);
|
png_destroy_write_struct(&png_ptr, &info_ptr);
|
||||||
delete[] RowPointers;
|
|
||||||
delete[] tmpImage;
|
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
png_set_rows(png_ptr, info_ptr, RowPointers);
|
png_set_rows(png_ptr, info_ptr, RowPointers.get());
|
||||||
|
|
||||||
if (image->getColorFormat() == ECF_A8R8G8B8 || image->getColorFormat() == ECF_A1R5G5B5)
|
if (image->getColorFormat() == ECF_A8R8G8B8 || image->getColorFormat() == ECF_A1R5G5B5)
|
||||||
png_write_png(png_ptr, info_ptr, PNG_TRANSFORM_BGR, NULL);
|
png_write_png(png_ptr, info_ptr, PNG_TRANSFORM_BGR, NULL);
|
||||||
|
@ -185,8 +178,6 @@ bool CImageWriterPNG::writeImage(io::IWriteFile *file, IImage *image, u32 param)
|
||||||
png_write_png(png_ptr, info_ptr, PNG_TRANSFORM_IDENTITY, NULL);
|
png_write_png(png_ptr, info_ptr, PNG_TRANSFORM_IDENTITY, NULL);
|
||||||
}
|
}
|
||||||
|
|
||||||
delete[] RowPointers;
|
|
||||||
delete[] tmpImage;
|
|
||||||
png_destroy_write_struct(&png_ptr, &info_ptr);
|
png_destroy_write_struct(&png_ptr, &info_ptr);
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue