Skip to content

Commit

Permalink
Fix warnings (false positives, or non-important) raised by CSA
Browse files Browse the repository at this point in the history
  • Loading branch information
rouault committed Nov 15, 2023
1 parent 3344a2b commit 7de0934
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 46 deletions.
12 changes: 5 additions & 7 deletions alg/gdalsievefilter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include <cstring>

#include <algorithm>
#include <cassert>
#include <set>
#include <vector>
#include <utility>
Expand Down Expand Up @@ -321,9 +322,9 @@ CPLErr CPL_STDCALL GDALSieveFilter(GDALRasterBandH hSrcBand,
/* Push the sizes of merged polygon fragments into the */
/* merged polygon id's count. */
/* -------------------------------------------------------------------- */
for (int iPoly = 0; oFirstEnum.panPolyIdMap != nullptr && // for Coverity
iPoly < oFirstEnum.nNextPolygonId;
iPoly++)
assert(oFirstEnum.panPolyIdMap != nullptr); // for Coverity

This comment has been minimized.

Copy link
@schwehr

schwehr Nov 19, 2024

Member

These two asserts are removed in a6ffbcb because it triggered rasterio/rasterio#3101

This is already fix, but just leaving documentation since I just ran into the issue as I'm back at this point in the git history.

assert(oFirstEnum.panPolyValue != nullptr); // for Coverity
for (int iPoly = 0; iPoly < oFirstEnum.nNextPolygonId; iPoly++)
{
if (oFirstEnum.panPolyIdMap[iPoly] != iPoly)
{
Expand Down Expand Up @@ -462,10 +463,7 @@ CPLErr CPL_STDCALL GDALSieveFilter(GDALRasterBandH hSrcBand,
int nIsolatedSmall = 0;
int nSieveTargets = 0;

for (int iPoly = 0; oFirstEnum.panPolyIdMap != nullptr && // for Coverity
oFirstEnum.panPolyValue != nullptr && // for Coverity
iPoly < static_cast<int>(anPolySizes.size());
iPoly++)
for (int iPoly = 0; iPoly < static_cast<int>(anPolySizes.size()); iPoly++)
{
if (oFirstEnum.panPolyIdMap[iPoly] != iPoly)
continue;
Expand Down
20 changes: 8 additions & 12 deletions apps/gdalmanage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,33 +105,29 @@ static void Identify(int nArgc, char **papszArgv)
bool bForceRecurse = false;
bool bReportFailures = false;

while (nArgc > 0 && papszArgv[0][0] == '-')
int i = 0;
for (; i < nArgc && papszArgv[i][0] == '-'; ++i)
{
if (EQUAL(papszArgv[0], "-r"))
if (EQUAL(papszArgv[i], "-r"))
bRecursive = true;
else if (EQUAL(papszArgv[0], "-fr"))
else if (EQUAL(papszArgv[i], "-fr"))
{
bForceRecurse = true;
bRecursive = true;
}
else if (EQUAL(papszArgv[0], "-u"))
else if (EQUAL(papszArgv[i], "-u"))
bReportFailures = true;
else
Usage(true);

papszArgv++;
nArgc--;
}

/* -------------------------------------------------------------------- */
/* Process given files. */
/* -------------------------------------------------------------------- */
while (nArgc > 0)
for (; i < nArgc; ++i)
{
ProcessIdentifyTarget(papszArgv[0], nullptr, bRecursive,
ProcessIdentifyTarget(papszArgv[i], nullptr, bRecursive,
bReportFailures, bForceRecurse);
nArgc--;
papszArgv++;
}
}

Expand Down Expand Up @@ -193,7 +189,7 @@ MAIN_START(argc, argv)
if (argc < 1)
exit(-argc);

for (int i = 0; argv != nullptr && argv[i] != nullptr; i++)
for (int i = 0; i < argc; i++)
{
if (EQUAL(argv[i], "--help"))
{
Expand Down
3 changes: 1 addition & 2 deletions apps/test_ogrsf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,7 @@ MAIN_START(nArgc, papszArgv)
{
pszSQLStatement = papszArgv[++iArg];
}
else if (EQUAL(papszArgv[iArg], "-dialect") &&
papszArgv[iArg + 1] != nullptr)
else if (EQUAL(papszArgv[iArg], "-dialect") && iArg + 1 < nArgc)
{
pszDialect = papszArgv[++iArg];
}
Expand Down
2 changes: 1 addition & 1 deletion ci/travis/csa_common/script.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ rm -f filtered_scanbuild.txt
files=$(find scanbuildoutput -name "*.sarif")
for f in $files; do
# CSA 10 uses artifactLocation. Earlier versions used fileLocation
(sed 's/fileLocation/artifactLocation/g' < $f) |jq '.runs[].results[] | (if .locations[].physicalLocation.artifactLocation.uri | (contains("/usr/include") or contains("degrib") or contains("libpng") or contains("libjpeg") or contains("EHapi") or contains("GDapi") or contains("SWapi") or contains("osr_cs_wkt_parser") or contains("ods_formula_parser") or contains("swq_parser") or contains("json_tokener") ) then empty else { "uri": .locations[].physicalLocation.artifactLocation.uri, "msg": .message.text, "location": .codeFlows[-1].threadFlows[-1].locations[-1] } end)' > tmp.txt
(sed 's/fileLocation/artifactLocation/g' < $f) |jq '.runs[].results[] | (if .locations[].physicalLocation.artifactLocation.uri | (contains("/usr/include") or contains("degrib") or contains("libpng") or contains("libjpeg") or contains("EHapi") or contains("GDapi") or contains("SWapi") or contains("osr_cs_wkt_parser") or contains("ods_formula_parser") or contains("swq_parser") or contains("libjson") or contains("flatbuffers") or contains("cpl_minizip_zip.cpp") or contains("gdal_rpc.cpp") or contains("internal_libqhull") ) then empty else { "uri": .locations[].physicalLocation.artifactLocation.uri, "msg": .message.text, "location": .codeFlows[-1].threadFlows[-1].locations[-1] } end)' > tmp.txt
if [ -s tmp.txt ]; then
echo "Errors from $f: "
cat $f
Expand Down
4 changes: 2 additions & 2 deletions frmts/gtiff/gtiffrasterband_read.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1604,15 +1604,15 @@ const char *GTiffRasterBand::GetMetadataItem(const char *pszName,
return CPLSPrintf(CPL_FRMT_GUIB, static_cast<GUIntBig>(nByteCount));
}
}
else if (pszDomain != nullptr && EQUAL(pszDomain, "_DEBUG_"))
else if (pszName && pszDomain && EQUAL(pszDomain, "_DEBUG_"))
{
if (EQUAL(pszName, "HAS_BLOCK_CACHE"))
return HasBlockCache() ? "1" : "0";
}

const char *pszRet = m_oGTiffMDMD.GetMetadataItem(pszName, pszDomain);

if (pszRet == nullptr && eDataType == GDT_Byte && pszDomain != nullptr &&
if (pszRet == nullptr && eDataType == GDT_Byte && pszName && pszDomain &&
EQUAL(pszDomain, "IMAGE_STRUCTURE") && EQUAL(pszName, "PIXELTYPE"))
{
// to get a chance of emitting the warning about this legacy usage
Expand Down
5 changes: 3 additions & 2 deletions frmts/iso8211/ddffield.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ void DDFField::Dump(FILE *fp)
{
int nMaxRepeat = 8;

if (getenv("DDF_MAXDUMP") != nullptr)
nMaxRepeat = atoi(getenv("DDF_MAXDUMP"));
const char *pszDDF_MAXDUMP = getenv("DDF_MAXDUMP");
if (pszDDF_MAXDUMP != nullptr)
nMaxRepeat = atoi(pszDDF_MAXDUMP);

fprintf(fp, " DDFField:\n");
fprintf(fp, " Tag = `%s'\n", poDefn->GetName());
Expand Down
22 changes: 11 additions & 11 deletions frmts/png/filter_sse2_intrinsics.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ static void gdal_png_read_filter_row_sub3_sse2(png_row_infop row_info,
d = _mm_add_epi8(d, a);
store3(row, d);

input += 3;
row += 3;
rb -= 3;
//input += 3;
//row += 3;
//rb -= 3;
}
}

Expand Down Expand Up @@ -188,10 +188,10 @@ static void gdal_png_read_filter_row_avg3_sse2(png_row_infop row_info,
d = _mm_add_epi8(d, avg);
store3(row, d);

input += 3;
prev += 3;
row += 3;
rb -= 3;
// input += 3;
// prev += 3;
// row += 3;
// rb -= 3;
}
}

Expand Down Expand Up @@ -366,10 +366,10 @@ static void gdal_png_read_filter_row_paeth3_sse2(png_row_infop row_info,
d = _mm_add_epi8(d, nearest);
store3(row, _mm_packus_epi16(d, d));

input += 3;
prev += 3;
row += 3;
rb -= 3;
// input += 3;
// prev += 3;
// row += 3;
// rb -= 3;
}
}

Expand Down
17 changes: 10 additions & 7 deletions frmts/png/pngdataset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ CPLErr PNGDataset::LoadWholeImage(void *pSingleBuffer, GSpacing nPixelSpace,

// We try to read the zlib compressed data into pData, if there is
// enough room for that
size_t pDataSize = 0;
size_t nDataSize = 0;
std::vector<GByte> abyCompressedData; // keep in this scope
GByte *pabyCompressedData = static_cast<GByte *>(pSingleBuffer);
size_t nCompressedDataSize = 0;
Expand All @@ -424,14 +424,14 @@ CPLErr PNGDataset::LoadWholeImage(void *pSingleBuffer, GSpacing nPixelSpace,
if (nPixelSpace == nBands && nLineSpace == nPixelSpace * nRasterXSize &&
(nBands == 1 || nBandSpace == 1))
{
pDataSize =
nDataSize =
static_cast<size_t>(nRasterXSize) * nRasterYSize * nBands;
}
else if (nPixelSpace == 1 && nLineSpace == nRasterXSize &&
nBandSpace ==
static_cast<GSpacing>(nRasterXSize) * nRasterYSize)
{
pDataSize =
nDataSize =
static_cast<size_t>(nRasterXSize) * nRasterYSize * nBands;
}
}
Expand Down Expand Up @@ -485,7 +485,7 @@ CPLErr PNGDataset::LoadWholeImage(void *pSingleBuffer, GSpacing nPixelSpace,
}
}

if (nCompressedDataSize + nChunkSize > pDataSize)
if (nCompressedDataSize + nChunkSize > nDataSize)
{
const bool bVectorEmptyBefore = abyCompressedData.empty();
// unlikely situation: would mean that the zlib compressed
Expand All @@ -502,9 +502,12 @@ CPLErr PNGDataset::LoadWholeImage(void *pSingleBuffer, GSpacing nPixelSpace,
bError = true;
break;
}
if (bVectorEmptyBefore && nCompressedDataSize > 0)
if (bVectorEmptyBefore && pSingleBuffer &&
nCompressedDataSize > 0)
{
memcpy(pabyCompressedData, pSingleBuffer,
nCompressedDataSize);
}
}
VSIFReadL(pabyCompressedData + nCompressedDataSize, nChunkSize, 1,
fpImage);
Expand Down Expand Up @@ -1202,7 +1205,8 @@ CPLErr PNGRasterBand::IRasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff,
else if (nBlockYSize > 1)
{
void *apabyBuffers[4];
GDALRasterBlock *apoBlocks[4];
GDALRasterBlock *apoBlocks[4] = {nullptr, nullptr, nullptr,
nullptr};
CPLErr eErr = CE_None;
bool bNeedToUseDefaultCase = true;
for (int i = 0; i < poGDS->nBands; ++i)
Expand All @@ -1212,7 +1216,6 @@ CPLErr PNGRasterBand::IRasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff,
{
bNeedToUseDefaultCase = false;
apabyBuffers[i] = pData;
apoBlocks[i] = nullptr;
}
else
{
Expand Down
4 changes: 2 additions & 2 deletions ogr/ogrsf_frmts/filegdb/FGdbLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ OGRFeature *FGdbBaseLayer::GetNextFeature()

OGRFeature *pOGRFeature = nullptr;

if (!OGRFeatureFromGdbRow(&row, &pOGRFeature))
if (!OGRFeatureFromGdbRow(&row, &pOGRFeature) || !pOGRFeature)
{
int32 oid = -1;
CPL_IGNORE_RET_VAL(row.GetOID(oid));
Expand Down Expand Up @@ -3828,7 +3828,7 @@ OGRFeature *FGdbLayer::GetNextFeature()
OGRFeature *pOGRFeature = nullptr;
Row rowFull;
if (GetRow(enumRows, rowFull, oid) != OGRERR_NONE ||
!OGRFeatureFromGdbRow(&rowFull, &pOGRFeature))
!OGRFeatureFromGdbRow(&rowFull, &pOGRFeature) || !pOGRFeature)
{
GDBErr(hr,
CPLSPrintf(
Expand Down

0 comments on commit 7de0934

Please sign in to comment.