Skip to content

Commit

Permalink
Addresses various Coverity Scan warnings about performance & bad use …
Browse files Browse the repository at this point in the history 
…of iterators
  • Loading branch information
@rouault
rouault committed May 15, 2023
1 parent 761198a commit db2481a
Show file tree
Hide file tree
Showing 56 changed files with 544 additions and 491 deletions.
2 changes: 0 additions & 2 deletions apps/gdaldem_lib.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1467,7 +1467,6 @@ GDALColorReliefProcessColors(ColorAssociation **ppasColorAssociation,
pasColorAssociation, (nColorAssociation + nAdded) *
sizeof(ColorAssociation)));
pCurrent = &pasColorAssociation[i];
pPrevious = nullptr;
pasColorAssociation[nColorAssociation + nAdded - 1] = sPrevious;
pasColorAssociation[nColorAssociation + nAdded - 1].dfVal =
dfNewValue;
Expand All @@ -1489,7 +1488,6 @@ GDALColorReliefProcessColors(ColorAssociation **ppasColorAssociation,
pasColorAssociation, (nColorAssociation + nAdded) *
sizeof(ColorAssociation)));
pCurrent = &pasColorAssociation[i];
pPrevious = nullptr;
pasColorAssociation[nColorAssociation + nAdded - 1] = sCurrent;
pasColorAssociation[nColorAssociation + nAdded - 1].dfVal =
dfNewValue;
Expand Down
182 changes: 131 additions & 51 deletions autotest/cpp/test_cpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -799,81 +799,161 @@ TEST_F(test_cpl, VSIMalloc)
CPLPushErrorHandler(CPLQuietErrorHandler);

// The following tests will fail because of overflows
CPLErrorReset();
ASSERT_TRUE(VSIMalloc2(~(size_t)0, ~(size_t)0) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() != CE_None);

CPLErrorReset();
ASSERT_TRUE(VSIMalloc3(1, ~(size_t)0, ~(size_t)0) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() != CE_None);
{
CPLErrorReset();
void *ptr = VSIMalloc2(~(size_t)0, ~(size_t)0);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_NE(CPLGetLastErrorType(), CE_None);
}

CPLErrorReset();
ASSERT_TRUE(VSIMalloc3(~(size_t)0, 1, ~(size_t)0) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() != CE_None);
{
CPLErrorReset();
void *ptr = VSIMalloc3(1, ~(size_t)0, ~(size_t)0);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_NE(CPLGetLastErrorType(), CE_None);
}

CPLErrorReset();
ASSERT_TRUE(VSIMalloc3(~(size_t)0, ~(size_t)0, 1) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() != CE_None);
{
CPLErrorReset();
void *ptr = VSIMalloc3(~(size_t)0, 1, ~(size_t)0);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_NE(CPLGetLastErrorType(), CE_None);
}

{
CPLErrorReset();
void *ptr = VSIMalloc3(~(size_t)0, ~(size_t)0, 1);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_NE(CPLGetLastErrorType(), CE_None);
}

if (!CSLTestBoolean(CPLGetConfigOption("SKIP_MEM_INTENSIVE_TEST", "NO")))
{
// The following tests will fail because such allocations cannot succeed
#if SIZEOF_VOIDP == 8
CPLErrorReset();
ASSERT_TRUE(VSIMalloc(~(size_t)0) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() == CE_None); /* no error reported */
{
CPLErrorReset();
void *ptr = VSIMalloc(~(size_t)0);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_EQ(CPLGetLastErrorType(), CE_None); /* no error reported */
}

CPLErrorReset();
ASSERT_TRUE(VSIMalloc2(~(size_t)0, 1) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() != CE_None);
{
CPLErrorReset();
void *ptr = VSIMalloc2(~(size_t)0, 1);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_NE(CPLGetLastErrorType(), CE_None);
}

CPLErrorReset();
ASSERT_TRUE(VSIMalloc3(~(size_t)0, 1, 1) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() != CE_None);
{
CPLErrorReset();
void *ptr = VSIMalloc3(~(size_t)0, 1, 1);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_NE(CPLGetLastErrorType(), CE_None);
}

CPLErrorReset();
ASSERT_TRUE(VSICalloc(~(size_t)0, 1) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() == CE_None); /* no error reported */
{
CPLErrorReset();
void *ptr = VSICalloc(~(size_t)0, 1);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_EQ(CPLGetLastErrorType(), CE_None); /* no error reported */
}

CPLErrorReset();
ASSERT_TRUE(VSIRealloc(nullptr, ~(size_t)0) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() == CE_None); /* no error reported */
{
CPLErrorReset();
void *ptr = VSIRealloc(nullptr, ~(size_t)0);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_EQ(CPLGetLastErrorType(), CE_None); /* no error reported */
}

CPLErrorReset();
ASSERT_TRUE(VSI_MALLOC_VERBOSE(~(size_t)0) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() != CE_None);
{
CPLErrorReset();
void *ptr = VSI_MALLOC_VERBOSE(~(size_t)0);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_NE(CPLGetLastErrorType(), CE_None);
}

CPLErrorReset();
ASSERT_TRUE(VSI_MALLOC2_VERBOSE(~(size_t)0, 1) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() != CE_None);
{
CPLErrorReset();
void *ptr = VSI_MALLOC2_VERBOSE(~(size_t)0, 1);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_NE(CPLGetLastErrorType(), CE_None);
}

CPLErrorReset();
ASSERT_TRUE(VSI_MALLOC3_VERBOSE(~(size_t)0, 1, 1) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() != CE_None);
{
CPLErrorReset();
void *ptr = VSI_MALLOC3_VERBOSE(~(size_t)0, 1, 1);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_NE(CPLGetLastErrorType(), CE_None);
}

CPLErrorReset();
ASSERT_TRUE(VSI_CALLOC_VERBOSE(~(size_t)0, 1) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() != CE_None);
{
CPLErrorReset();
void *ptr = VSI_CALLOC_VERBOSE(~(size_t)0, 1);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_NE(CPLGetLastErrorType(), CE_None);
}

CPLErrorReset();
ASSERT_TRUE(VSI_REALLOC_VERBOSE(nullptr, ~(size_t)0) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() != CE_None);
{
CPLErrorReset();
void *ptr = VSI_REALLOC_VERBOSE(nullptr, ~(size_t)0);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_NE(CPLGetLastErrorType(), CE_None);
}
#endif
}

CPLPopErrorHandler();

// The following allocs will return NULL because of 0 byte alloc
CPLErrorReset();
ASSERT_TRUE(VSIMalloc2(0, 1) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() == CE_None);
ASSERT_TRUE(VSIMalloc2(1, 0) == nullptr);
{
CPLErrorReset();
void *ptr = VSIMalloc2(0, 1);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_EQ(CPLGetLastErrorType(), CE_None);
}

CPLErrorReset();
ASSERT_TRUE(VSIMalloc3(0, 1, 1) == nullptr);
ASSERT_TRUE(CPLGetLastErrorType() == CE_None);
ASSERT_TRUE(VSIMalloc3(1, 0, 1) == nullptr);
ASSERT_TRUE(VSIMalloc3(1, 1, 0) == nullptr);
{
void *ptr = VSIMalloc2(1, 0);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
}

{
CPLErrorReset();
void *ptr = VSIMalloc3(0, 1, 1);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
EXPECT_EQ(CPLGetLastErrorType(), CE_None);
}

{
void *ptr = VSIMalloc3(1, 0, 1);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
}

{
void *ptr = VSIMalloc3(1, 1, 0);
EXPECT_EQ(ptr, nullptr);
VSIFree(ptr);
}
}

TEST_F(test_cpl, CPLFormFilename)
Expand Down
20 changes: 16 additions & 4 deletions autotest/cpp/test_marching_squares_tile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,14 @@ struct Writer
// check if a segment is in a set of borders
bool segmentInBorders(int levelIdx, const Segment &segmentToTest) const
{
std::vector<Segment> segments = borders.find(levelIdx)->second;
for (Segment &s : segments)
const auto iter = borders.find(levelIdx);
if (iter == borders.end())
{
CPLAssert(false);
return false;
}
const auto &segments = iter->second;
for (const Segment &s : segments)
{
// (A,B) == (A,B) || (A,B) == (B,A)
if (((coordEquals(s.first.x, segmentToTest.first.x)) &&
Expand All @@ -81,8 +87,14 @@ struct Writer
// check if a segment is in a set of contours
bool segmentInContours(int levelIdx, const Segment &segmentToTest) const
{
std::vector<Segment> segments = contours.find(levelIdx)->second;
for (Segment &s : segments)
const auto iter = contours.find(levelIdx);
if (iter == contours.end())
{
CPLAssert(false);
return false;
}
const auto &segments = iter->second;
for (const Segment &s : segments)
{
// (A,B) == (A,B) || (A,B) == (B,A)
if (((coordEquals(s.first.x, segmentToTest.first.x)) &&
Expand Down
18 changes: 10 additions & 8 deletions frmts/arg/argdataset.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,18 +131,18 @@ CPLErr ARGDataset::GetGeoTransform(double *padfTransform)
/************************************************************************/
/* GetJsonFilename() */
/************************************************************************/
static CPLString GetJsonFilename(CPLString pszFilename)
static std::string GetJsonFilename(const std::string &pszFilename)
{
return CPLSPrintf("%s/%s.json", CPLGetDirname(pszFilename),
CPLGetBasename(pszFilename));
return CPLSPrintf("%s/%s.json", CPLGetDirname(pszFilename.c_str()),
CPLGetBasename(pszFilename.c_str()));
}

/************************************************************************/
/* GetJsonObject() */
/************************************************************************/
static json_object *GetJsonObject(CPLString pszFilename)
static json_object *GetJsonObject(const std::string &pszFilename)
{
CPLString osJSONFilename = GetJsonFilename(pszFilename);
const std::string osJSONFilename = GetJsonFilename(pszFilename);

json_object *pJSONObject =
json_object_from_file(const_cast<char *>(osJSONFilename.c_str()));
Expand All @@ -158,7 +158,8 @@ static json_object *GetJsonObject(CPLString pszFilename)
/************************************************************************/
/* GetJsonValueStr() */
/************************************************************************/
static const char *GetJsonValueStr(json_object *pJSONObject, CPLString pszKey)
static const char *GetJsonValueStr(json_object *pJSONObject,
const std::string &pszKey)
{
json_object *pJSONItem =
CPL_json_object_object_get(pJSONObject, pszKey.c_str());
Expand All @@ -177,7 +178,8 @@ static const char *GetJsonValueStr(json_object *pJSONObject, CPLString pszKey)
/************************************************************************/
/* GetJsonValueDbl() */
/************************************************************************/
static double GetJsonValueDbl(json_object *pJSONObject, CPLString pszKey)
static double GetJsonValueDbl(json_object *pJSONObject,
const std::string &pszKey)
{
const char *pszJSONStr = GetJsonValueStr(pJSONObject, pszKey.c_str());
if (pszJSONStr == nullptr)
Expand All @@ -201,7 +203,7 @@ static double GetJsonValueDbl(json_object *pJSONObject, CPLString pszKey)
/************************************************************************/
/* GetJsonValueInt() */
/************************************************************************/
static int GetJsonValueInt(json_object *pJSONObject, CPLString pszKey)
static int GetJsonValueInt(json_object *pJSONObject, const std::string &pszKey)
{
const double dfTmp = GetJsonValueDbl(pJSONObject, pszKey.c_str());
if (CPLIsNan(dfTmp))
Expand Down
2 changes: 1 addition & 1 deletion frmts/daas/daasdataset.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1192,7 +1192,7 @@ bool GDALDAASDataset::SetupServerSideReprojection(const char *pszTargetSRS)
std::copy_n(adfGeoTransform, 6, m_adfGeoTransform.begin());
m_bRequestInGeoreferencedCoordinates = true;
m_osSRSType = "epsg";
m_osSRSValue = osTargetEPSGCode;
m_osSRSValue = std::move(osTargetEPSGCode);
m_oSRS = oSRS;
nRasterXSize = nXSize;
nRasterYSize = nYSize;
Expand Down
2 changes: 1 addition & 1 deletion frmts/eeda/eedacommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ BuildBandDescArray(json_object *poBands,
oDesc.osName = pszBandId;
oDesc.osWKT = osWKT;
oDesc.eDT = eDT;
oDesc.adfGeoTransform = adfGeoTransform;
oDesc.adfGeoTransform = std::move(adfGeoTransform);
oDesc.nWidth = nWidth;
oDesc.nHeight = nHeight;
aoBandDesc.push_back(oDesc);
Expand Down
11 changes: 9 additions & 2 deletions frmts/mem/memdataset.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1647,8 +1647,15 @@ std::shared_ptr<GDALMDArray> MEMGroupCreateMDArray(
const GDALExtendedDataType &oDataType, void *pData,
CSLConstList papszOptions)
{
return dynamic_cast<MEMGroup *>(poGroup)->CreateMDArray(
osName, aoDimensions, oDataType, pData, papszOptions);
auto poMemGroup = dynamic_cast<MEMGroup *>(poGroup);
if (!poMemGroup)
{
CPLError(CE_Failure, CPLE_AppDefined,
"MEMGroupCreateMDArray(): poGroup not of type MEMGroup");
return nullptr;
}
return poMemGroup->CreateMDArray(osName, aoDimensions, oDataType, pData,
papszOptions);
}

/************************************************************************/
Expand Down
2 changes: 1 addition & 1 deletion frmts/nitf/nitfdataset.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2291,7 +2291,7 @@ CPLErr NITFDataset::SetGCPs(int nGCPCountIn, const GDAL_GCP *pasGCPListIn,
/* To recompute the zone */
OGRSpatialReference oSRSBackup = m_oSRS;
CPLErr eErr = SetSpatialRef(&m_oGCPSRS);
m_oSRS = oSRSBackup;
m_oSRS = std::move(oSRSBackup);

if (eErr != CE_None)
return eErr;
Expand Down
2 changes: 1 addition & 1 deletion frmts/pcidsk/sdk/core/cpcidskfile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ using namespace PCIDSK;
/* CPCIDSKFile() */
/************************************************************************/

CPCIDSKFile::CPCIDSKFile( std::string filename )
CPCIDSKFile::CPCIDSKFile( const std::string& filename )

{
io_handle = nullptr;
Expand Down
2 changes: 1 addition & 1 deletion frmts/pcidsk/sdk/core/cpcidskfile.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ namespace PCIDSK
std::string access, const PCIDSKInterfaces *interfaces, int max_channel_count_allowed );
public:

CPCIDSKFile( std::string filename );
CPCIDSKFile( const std::string& filename );
virtual ~CPCIDSKFile();

virtual PCIDSKInterfaces *GetInterfaces() override { return &interfaces; }
Expand Down
2 changes: 1 addition & 1 deletion frmts/pcidsk/sdk/core/pcidskcreate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ PCIDSK::Create( std::string filename, int pixels, int lines,
int tilesize = PCIDSK_DEFAULT_TILE_SIZE;
std::string oLinkFilename;

std::string oOrigOptions = options;
const std::string oOrigOptions = options;
UCaseStr( options );
for(auto & c : options)
{
Expand Down
Loading

0 comments on commit db2481a

Please sign in to comment.