A few more minor fixes geared towards lgtm.com alerts (#6127)

* Remove unfilled formatting placeholders in KLT parser.
* Fix presumably unintentionally missing reference on PlanatPtr and ChunkyPtr call operators.
* Fix catching pointer and make use-after-free of workimg easier to spot.
Make sure all of our thrown exceptions derive from std::exception and then catch
by reference instead of by pointer.
* Fix mismatch between array form new and non-array form delete.
* Simplify memory management of embedded color profiles by unifying allocation to use operator new.
This commit is contained in:
Adam Reichold 2021-03-17 08:37:19 +01:00 committed by GitHub
parent 537fa73d3f
commit fae40a137a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 22 additions and 37 deletions

View File

@ -843,7 +843,7 @@ inline void crxFillBuffer(CrxBitstream* bitStrm)
#endif #endif
if (bitStrm->curBufSize < 1) { // nothing read if (bitStrm->curBufSize < 1) { // nothing read
throw std::exception(); throw std::runtime_error("Unexpected end of file in CRX bitstream");
} }
bitStrm->mdatSize -= bitStrm->curBufSize; bitStrm->mdatSize -= bitStrm->curBufSize;

View File

@ -18,7 +18,7 @@
*/ */
#include "iccjpeg.h" #include "iccjpeg.h"
#include <cstdlib> /* define malloc() */ #include <new>
/* /*
@ -155,12 +155,9 @@ marker_is_icc (jpeg_saved_marker_ptr marker)
* If TRUE is returned, *icc_data_ptr is set to point to the * If TRUE is returned, *icc_data_ptr is set to point to the
* returned data, and *icc_data_len is set to its length. * returned data, and *icc_data_len is set to its length.
* *
* IMPORTANT: the data at **icc_data_ptr has been allocated with malloc() * IMPORTANT: the data at **icc_data_ptr has been allocated with new
* and must be freed by the caller with free() when the caller no longer * and must be freed by the caller with delete[] when the caller no longer
* needs it. (Alternatively, we could write this routine to use the * needs it.
* IJG library's memory allocator, so that the data would be freed implicitly
* at jpeg_finish_decompress() time. But it seems likely that many apps
* will prefer to have the data stick around after decompression finishes.)
* *
* NOTE: if the file contains invalid ICC APP2 markers, we just silently * NOTE: if the file contains invalid ICC APP2 markers, we just silently
* return FALSE. You might want to issue an error message instead. * return FALSE. You might want to issue an error message instead.
@ -235,7 +232,7 @@ read_icc_profile (j_decompress_ptr cinfo,
} }
/* Allocate space for assembled data */ /* Allocate space for assembled data */
icc_data = (JOCTET *) malloc(total_length * sizeof(JOCTET)); icc_data = new (std::nothrow) JOCTET[total_length];
if (icc_data == nullptr) { if (icc_data == nullptr) {
return FALSE; /* oops, out of memory */ return FALSE; /* oops, out of memory */

View File

@ -220,7 +220,7 @@ public:
#endif #endif
return ptrs[row][col]; return ptrs[row][col];
} }
const T operator() (size_t row, size_t col) const const T& operator() (size_t row, size_t col) const
{ {
#if CHECK_BOUNDS #if CHECK_BOUNDS
assert (row < height_ && col < width_); assert (row < height_ && col < width_);
@ -1274,7 +1274,7 @@ public:
#endif #endif
return ptr[3 * (row * width + col)]; return ptr[3 * (row * width + col)];
} }
const T operator() (size_t row, size_t col) const const T& operator() (size_t row, size_t col) const
{ {
#if CHECK_BOUNDS #if CHECK_BOUNDS
assert (row < height_ && col < width_); assert (row < height_ && col < width_);

View File

@ -195,7 +195,6 @@ ImageIO::ImageIO() :
profileData(nullptr), profileData(nullptr),
profileLength(0), profileLength(0),
loadedProfileData(nullptr), loadedProfileData(nullptr),
loadedProfileDataJpg(false),
loadedProfileLength(0), loadedProfileLength(0),
exifChange(new procparams::ExifPairs), exifChange(new procparams::ExifPairs),
iptc(nullptr), iptc(nullptr),
@ -517,7 +516,6 @@ int ImageIO::loadJPEGFromMemory (const char* buffer, int bufsize)
jpeg_read_header(&cinfo, TRUE); jpeg_read_header(&cinfo, TRUE);
deleteLoadedProfileData(); deleteLoadedProfileData();
loadedProfileDataJpg = true;
bool hasprofile = read_icc_profile (&cinfo, (JOCTET**)&loadedProfileData, (unsigned int*)&loadedProfileLength); bool hasprofile = read_icc_profile (&cinfo, (JOCTET**)&loadedProfileData, (unsigned int*)&loadedProfileLength);
if (hasprofile) { if (hasprofile) {
@ -602,7 +600,6 @@ int ImageIO::loadJPEG (const Glib::ustring &fname)
cinfo.out_color_space = JCS_RGB; cinfo.out_color_space = JCS_RGB;
deleteLoadedProfileData(); deleteLoadedProfileData();
loadedProfileDataJpg = true;
bool hasprofile = read_icc_profile (&cinfo, (JOCTET**)&loadedProfileData, (unsigned int*)&loadedProfileLength); bool hasprofile = read_icc_profile (&cinfo, (JOCTET**)&loadedProfileData, (unsigned int*)&loadedProfileLength);
if (hasprofile) { if (hasprofile) {
@ -855,7 +852,6 @@ int ImageIO::loadTIFF (const Glib::ustring &fname)
char* profdata; char* profdata;
deleteLoadedProfileData(); deleteLoadedProfileData();
loadedProfileDataJpg = false;
if (TIFFGetField(in, TIFFTAG_ICCPROFILE, &loadedProfileLength, &profdata)) { if (TIFFGetField(in, TIFFTAG_ICCPROFILE, &loadedProfileLength, &profdata)) {
embProfile = cmsOpenProfileFromMem (profdata, loadedProfileLength); embProfile = cmsOpenProfileFromMem (profdata, loadedProfileLength);
@ -1710,11 +1706,7 @@ MyMutex& ImageIO::mutex ()
void ImageIO::deleteLoadedProfileData( ) void ImageIO::deleteLoadedProfileData( )
{ {
if(loadedProfileData) { if(loadedProfileData) {
if(loadedProfileDataJpg) { delete[] loadedProfileData;
free(loadedProfileData);
} else {
delete[] loadedProfileData;
}
} }
loadedProfileData = nullptr; loadedProfileData = nullptr;

View File

@ -70,7 +70,6 @@ protected:
char* profileData; char* profileData;
int profileLength; int profileLength;
char* loadedProfileData; char* loadedProfileData;
bool loadedProfileDataJpg;
int loadedProfileLength; int loadedProfileLength;
const std::unique_ptr<procparams::ExifPairs> exifChange; const std::unique_ptr<procparams::ExifPairs> exifChange;
IptcData* iptc; IptcData* iptc;

View File

@ -1652,8 +1652,10 @@ void ImProcCoordinator::updatePreviewImage(int todo, bool panningRelatedChange)
// Computing the internal image for analysis, i.e. conversion from WCS->Output profile // Computing the internal image for analysis, i.e. conversion from WCS->Output profile
delete workimg; delete workimg;
workimg = nullptr;
workimg = ipf.lab2rgb(nprevl, 0, 0, pW, pH, params->icm); workimg = ipf.lab2rgb(nprevl, 0, 0, pW, pH, params->icm);
} catch (char * str) { } catch (std::exception&) {
return; return;
} }
} }
@ -1728,6 +1730,7 @@ void ImProcCoordinator::freeAll()
} }
delete workimg; delete workimg;
workimg = nullptr;
} }

View File

@ -5622,14 +5622,8 @@ double ImProcFunctions::getAutoDistor(const Glib::ustring &fname, int thumb_size
rawGray = raw->getGrayscaleHistEQ(width); rawGray = raw->getGrayscaleHistEQ(width);
if (!thumbGray || !rawGray) { if (!thumbGray || !rawGray) {
if (thumbGray) { delete[] thumbGray;
delete thumbGray; delete[] rawGray;
}
if (rawGray) {
delete rawGray;
}
delete thumb; delete thumb;
delete raw; delete raw;
return 0.0; return 0.0;
@ -5642,8 +5636,8 @@ double ImProcFunctions::getAutoDistor(const Glib::ustring &fname, int thumb_size
calcDistortion(thumbGray, rawGray, width, h_thumb, 4, dist_amount); calcDistortion(thumbGray, rawGray, width, h_thumb, 4, dist_amount);
} }
delete thumbGray; delete[] thumbGray;
delete rawGray; delete[] rawGray;
delete thumb; delete thumb;
delete raw; delete raw;
return dist_amount; return dist_amount;

View File

@ -559,19 +559,19 @@ static structureType _readHeader(
if (id == FEATURE_TABLE) { if (id == FEATURE_TABLE) {
fscanf(fp, "%s", line); fscanf(fp, "%s", line);
if (strcmp(line, ",") != 0) { if (strcmp(line, ",") != 0) {
KLTError("(_readFeatures) File '%s' is corrupted -- " KLTError("(_readFeatures) File is corrupted -- "
"(Expected 'comma', found '%s' instead)", line); "(Expected 'comma', found '%s' instead)", line);
exit(1); exit(1);
} }
fscanf(fp, "%s", line); fscanf(fp, "%s", line);
if (strcmp(line, "nFeatures") != 0) { if (strcmp(line, "nFeatures") != 0) {
KLTError("(_readFeatures) File '%s' is corrupted -- " KLTError("(_readFeatures) File is corrupted -- "
"(2 Expected 'nFeatures ', found '%s' instead)", line); "(2 Expected 'nFeatures ', found '%s' instead)", line);
exit(1); exit(1);
} }
fscanf(fp, "%s", line); fscanf(fp, "%s", line);
if (strcmp(line, "=") != 0) { if (strcmp(line, "=") != 0) {
KLTError("(_readFeatures) File '%s' is corrupted -- " KLTError("(_readFeatures) File is corrupted -- "
"(2 Expected '= ', found '%s' instead)", line); "(2 Expected '= ', found '%s' instead)", line);
exit(1); exit(1);
} }

View File

@ -196,7 +196,7 @@ rtengine::LCPProfile::LCPProfile(const Glib::ustring& fname) :
XML_Parser parser = XML_ParserCreate(nullptr); XML_Parser parser = XML_ParserCreate(nullptr);
if (!parser) { if (!parser) {
throw "Couldn't allocate memory for XML parser"; throw std::runtime_error("Couldn't allocate memory for XML parser");
} }
XML_SetElementHandler(parser, XmlStartHandler, XmlEndHandler); XML_SetElementHandler(parser, XmlStartHandler, XmlEndHandler);
@ -216,7 +216,7 @@ rtengine::LCPProfile::LCPProfile(const Glib::ustring& fname) :
if (XML_Parse(parser, buf, bytesRead, done) == XML_STATUS_ERROR) { if (XML_Parse(parser, buf, bytesRead, done) == XML_STATUS_ERROR) {
XML_ParserFree(parser); XML_ParserFree(parser);
throw "Invalid XML in LCP file"; throw std::runtime_error("Invalid XML in LCP file");
} }
} while (!done); } while (!done);