From 62fc992460537a55a4aa512c7fcc18f7fe522417 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Sun, 23 Jun 2024 23:50:40 +0200 Subject: [PATCH] Fix number of new Coverity Scan warnings, likely due to new version of the tool --- alg/gdalchecksum.cpp | 7 +- alg/gdalrasterize.cpp | 7 +- alg/gdalwarpkernel.cpp | 5 +- alg/internal_libqhull/poly_r.c | 17 ++- apps/argparse/argparse.hpp | 6 +- apps/gdalargumentparser.cpp | 8 +- autotest/cpp/test_gdal.cpp | 2 +- frmts/grib/degrib/degrib/clock.c | 1 + frmts/gsg/gsbgdataset.cpp | 32 ++--- frmts/gtiff/gt_overview.cpp | 7 +- frmts/gtiff/gtiffdataset_read.cpp | 3 +- frmts/gtiff/gtiffdataset_write.cpp | 7 +- frmts/gtiff/gtiffrasterband_read.cpp | 4 + frmts/mrf/marfa.h | 6 +- frmts/netcdf/netcdfdataset.cpp | 122 +++++++++++------- frmts/northwood/northwood.cpp | 2 + frmts/pdf/pdfdataset.cpp | 16 ++- gcore/gdaljp2structure.cpp | 4 +- gcore/gdalmultidim.cpp | 1 + ogr/ogr_p.h | 3 +- ogr/ogrlinestring.cpp | 28 ++-- .../arrow_common/ograrrowlayer.hpp | 2 +- ogr/ogrsf_frmts/geojson/libjson/json_util.c | 4 +- ogr/ogrsf_frmts/gmlas/ogrgmlaslayer.cpp | 2 +- .../gpkg/ogrgeopackagetablelayer.cpp | 11 +- ogr/ogrsf_frmts/mvt/ogrmvtdataset.cpp | 45 ++++--- ogr/ogrsf_frmts/ntf/ntfrecord.cpp | 1 + ogr/ogrsf_frmts/openfilegdb/filegdbtable.cpp | 20 ++- .../openfilegdb/filegdbtable_freelist.cpp | 3 + ogr/ogrsf_frmts/osm/ogrosmdatasource.cpp | 1 + ogr/ogrsf_frmts/pmtiles/pmtiles/pmtiles.hpp | 3 + ogr/ogrsf_frmts/shape/ogrshape.h | 1 + ogr/ogrsf_frmts/shape/ogrshapedatasource.cpp | 7 +- ogr/ogrsf_frmts/vfk/vfkreader.cpp | 4 +- ogr/ogrsf_frmts/vfk/vfkreader.h | 2 +- ogr/ogrsf_frmts/vfk/vfkreaderp.h | 4 +- ogr/ogrsf_frmts/vfk/vfkreadersqlite.cpp | 6 +- ogr/ogrsf_frmts/vrt/ogrvrtdriver.cpp | 11 +- ogr/ogrsf_frmts/wfs/ogrwfsdatasource.cpp | 1 + port/cpl_md5.cpp | 1 + port/cpl_spawn.cpp | 8 +- port/cpl_vsi_mem.cpp | 101 +++++++++------ port/cpl_vsil_s3.cpp | 1 + port/cpl_vsisimple.cpp | 6 +- port/cpl_worker_thread_pool.cpp | 39 +++++- port/cpl_worker_thread_pool.h | 7 +- 46 files changed, 374 insertions(+), 205 deletions(-) diff --git a/alg/gdalchecksum.cpp b/alg/gdalchecksum.cpp index e0f0dcafacf8..64f6158433b8 100644 --- a/alg/gdalchecksum.cpp +++ b/alg/gdalchecksum.cpp @@ -80,12 +80,9 @@ int CPL_STDCALL GDALChecksumImage(GDALRasterBandH hBand, int nXOff, int nYOff, const auto IntFromDouble = [](double dfVal) { int nVal; - if (CPLIsNan(dfVal) || CPLIsInf(dfVal)) + if (!std::isfinite(dfVal)) { - // Most compilers seem to cast NaN or Inf to 0x80000000. - // but VC7 is an exception. So we force the result - // of such a cast. - nVal = 0x80000000; + nVal = INT_MIN; } else { diff --git a/alg/gdalrasterize.cpp b/alg/gdalrasterize.cpp index 16be0bac7c55..6aa6efc998e3 100644 --- a/alg/gdalrasterize.cpp +++ b/alg/gdalrasterize.cpp @@ -1559,11 +1559,8 @@ CPLErr GDALRasterizeLayers(GDALDatasetH hDS, int nBandCount, int *panBandList, if (!(pszYChunkSize && ((nYChunkSize = atoi(pszYChunkSize))) != 0)) { const GIntBig nYChunkSize64 = GDALGetCacheMax64() / nScanlineBytes; - const int knIntMax = std::numeric_limits::max(); - if (nYChunkSize64 > knIntMax) - nYChunkSize = knIntMax; - else - nYChunkSize = static_cast(nYChunkSize64); + nYChunkSize = static_cast( + std::min(nYChunkSize64, std::numeric_limits::max())); } if (nYChunkSize < 1) diff --git a/alg/gdalwarpkernel.cpp b/alg/gdalwarpkernel.cpp index 98b1555c4a1b..51361a45d12c 100644 --- a/alg/gdalwarpkernel.cpp +++ b/alg/gdalwarpkernel.cpp @@ -513,6 +513,7 @@ static CPLErr GWKRun(GDALWarpKernel *poWK, const char *pszFuncName, job.pfnFunc = pfnFunc; } + bool bStopFlag; { std::unique_lock lock(psThreadData->mutex); @@ -550,6 +551,8 @@ static CPLErr GWKRun(GDALWarpKernel *poWK, const char *pszFuncName, } } } + + bStopFlag = psThreadData->stopFlag; } /* -------------------------------------------------------------------- */ @@ -557,7 +560,7 @@ static CPLErr GWKRun(GDALWarpKernel *poWK, const char *pszFuncName, /* -------------------------------------------------------------------- */ psThreadData->poJobQueue->WaitCompletion(); - return psThreadData->stopFlag ? CE_Failure : CE_None; + return bStopFlag ? CE_Failure : CE_None; } /************************************************************************/ diff --git a/alg/internal_libqhull/poly_r.c b/alg/internal_libqhull/poly_r.c index c68596a12bf9..b7e09b54fc8d 100644 --- a/alg/internal_libqhull/poly_r.c +++ b/alg/internal_libqhull/poly_r.c @@ -1137,10 +1137,13 @@ ridgeT *qh_newridge(qhT *qh) { qh_memalloc_(qh, (int)sizeof(ridgeT), freelistp, ridge, ridgeT); memset((char *)ridge, (size_t)0, sizeof(ridgeT)); zinc_(Ztotridges); + ridge->id= qh->ridge_id; if (qh->ridge_id == UINT_MAX) { qh_fprintf(qh, qh->ferr, 7074, "qhull warning: more than 2^32 ridges. Qhull results are OK. Since the ridge ID wraps around to 0, two ridges may have the same identifier.\n"); + qh->ridge_id = 0; + } else { + qh->ridge_id++; } - ridge->id= qh->ridge_id++; trace4((qh, qh->ferr, 4056, "qh_newridge: created ridge r%d\n", ridge->id)); return(ridge); } /* newridge */ @@ -1176,10 +1179,14 @@ int qh_pointid(qhT *qh, pointT *point) { offset= (ptr_intT)(point - qh->first_point); /* coverity[divide_arg] */ id= offset / qh->hull_dim; - }else if ((id= qh_setindex(qh->other_points, point)) != -1) - id += qh->num_points; - else - return qh_IDunknown; + } else { + id = qh_setindex(qh->other_points, point); + if (id >= 0) { + id += qh->num_points; + } else { + return qh_IDunknown; + } + } return (int)id; } /* pointid */ diff --git a/apps/argparse/argparse.hpp b/apps/argparse/argparse.hpp index a29c120e00ab..7b589e7c79c8 100644 --- a/apps/argparse/argparse.hpp +++ b/apps/argparse/argparse.hpp @@ -2041,8 +2041,10 @@ class ArgumentParser { } stream << std::setw(2) << " "; - stream << std::setw(static_cast(longest_arg_length - 2)) - << command; + if (longest_arg_length >= 2) { + stream << std::setw(static_cast(longest_arg_length - 2)) + << command; + } stream << " " << subparser->get().m_description << "\n"; } } diff --git a/apps/gdalargumentparser.cpp b/apps/gdalargumentparser.cpp index c70a939be599..f9992cea7317 100644 --- a/apps/gdalargumentparser.cpp +++ b/apps/gdalargumentparser.cpp @@ -50,10 +50,10 @@ GDALArgumentParser::GDALArgumentParser(const std::string &program_name, add_argument("-h", "--help") .flag() .action( - [this, program_name](const auto &) + [this](const auto &) { std::cout << usage() << std::endl << std::endl; - std::cout << _("Note: ") << program_name + std::cout << _("Note: ") << m_program_name << _(" --long-usage for full help.") << std::endl; std::exit(0); }) @@ -77,11 +77,11 @@ GDALArgumentParser::GDALArgumentParser(const std::string &program_name, .flag() .hidden() .action( - [program_name](const auto &) + [this](const auto &) { printf("%s was compiled against GDAL %s and " "is running against GDAL %s\n", - program_name.c_str(), GDAL_RELEASE_NAME, + m_program_name.c_str(), GDAL_RELEASE_NAME, GDALVersionInfo("RELEASE_NAME")); std::exit(0); }) diff --git a/autotest/cpp/test_gdal.cpp b/autotest/cpp/test_gdal.cpp index 459105bdb9ec..8e8af2c85dd3 100644 --- a/autotest/cpp/test_gdal.cpp +++ b/autotest/cpp/test_gdal.cpp @@ -3413,7 +3413,7 @@ TEST_F(test_gdal, gtiff_ReadCompressedData) CE_None); EXPECT_EQ(nGotSize, nNeededSize); EXPECT_NE(pBuffer, nullptr); - if (pBuffer != nullptr && nGotSize == nNeededSize) + if (pBuffer != nullptr && nGotSize == nNeededSize && nNeededSize >= 2) { const GByte *pabyBuffer = static_cast(pBuffer); EXPECT_EQ(pabyBuffer[0], 0xFF); diff --git a/frmts/grib/degrib/degrib/clock.c b/frmts/grib/degrib/degrib/clock.c index 0f1ea08fcdab..a38543c8df45 100644 --- a/frmts/grib/degrib/degrib/clock.c +++ b/frmts/grib/degrib/degrib/clock.c @@ -729,6 +729,7 @@ sChar Clock_GetTimeZone () #else const struct tm *gmTimePtr = gmtime (&ansTime); #endif + timeZone = 0; if (gmTimePtr) { timeZone = gmTimePtr->tm_hour; diff --git a/frmts/gsg/gsbgdataset.cpp b/frmts/gsg/gsbgdataset.cpp index 2a234236fbb8..41b9d3d25a54 100644 --- a/frmts/gsg/gsbgdataset.cpp +++ b/frmts/gsg/gsbgdataset.cpp @@ -53,7 +53,7 @@ class GSBGDataset final : public GDALPamDataset static const float fNODATA_VALUE; static const size_t nHEADER_SIZE; - static CPLErr WriteHeader(VSILFILE *fp, GInt16 nXSize, GInt16 nYSize, + static CPLErr WriteHeader(VSILFILE *fp, int nXSize, int nYSize, double dfMinX, double dfMaxX, double dfMinY, double dfMaxY, double dfMinZ, double dfMaxZ); @@ -417,9 +417,9 @@ CPLErr GSBGRasterBand::IWriteBlock(int nBlockXOff, int nBlockYOff, void *pImage) if (bHeaderNeedsUpdate && dfMaxZ > dfMinZ) { - CPLErr eErr = poGDS->WriteHeader(poGDS->fp, (GInt16)nRasterXSize, - (GInt16)nRasterYSize, dfMinX, dfMaxX, - dfMinY, dfMaxY, dfMinZ, dfMaxZ); + CPLErr eErr = + poGDS->WriteHeader(poGDS->fp, nRasterXSize, nRasterYSize, dfMinX, + dfMaxX, dfMinY, dfMaxY, dfMinZ, dfMaxZ); return eErr; } @@ -686,9 +686,9 @@ CPLErr GSBGDataset::SetGeoTransform(double *padfGeoTransform) padfGeoTransform[5] * (nRasterYSize - 0.5) + padfGeoTransform[3]; double dfMaxY = padfGeoTransform[3] + padfGeoTransform[5] / 2; - CPLErr eErr = WriteHeader(fp, (GInt16)poGRB->nRasterXSize, - (GInt16)poGRB->nRasterYSize, dfMinX, dfMaxX, - dfMinY, dfMaxY, poGRB->dfMinZ, poGRB->dfMaxZ); + CPLErr eErr = + WriteHeader(fp, poGRB->nRasterXSize, poGRB->nRasterYSize, dfMinX, + dfMaxX, dfMinY, dfMaxY, poGRB->dfMinZ, poGRB->dfMaxZ); if (eErr == CE_None) { @@ -705,7 +705,7 @@ CPLErr GSBGDataset::SetGeoTransform(double *padfGeoTransform) /* WriteHeader() */ /************************************************************************/ -CPLErr GSBGDataset::WriteHeader(VSILFILE *fp, GInt16 nXSize, GInt16 nYSize, +CPLErr GSBGDataset::WriteHeader(VSILFILE *fp, int nXSize, int nYSize, double dfMinX, double dfMaxX, double dfMinY, double dfMaxY, double dfMinZ, double dfMaxZ) @@ -724,7 +724,8 @@ CPLErr GSBGDataset::WriteHeader(VSILFILE *fp, GInt16 nXSize, GInt16 nYSize, return CE_Failure; } - GInt16 nTemp = CPL_LSBWORD16(nXSize); + assert(nXSize >= 0 && nXSize <= std::numeric_limits::max()); + GInt16 nTemp = CPL_LSBWORD16(static_cast(nXSize)); if (VSIFWriteL((void *)&nTemp, 2, 1, fp) != 1) { CPLError(CE_Failure, CPLE_FileIO, @@ -732,7 +733,8 @@ CPLErr GSBGDataset::WriteHeader(VSILFILE *fp, GInt16 nXSize, GInt16 nYSize, return CE_Failure; } - nTemp = CPL_LSBWORD16(nYSize); + assert(nYSize >= 0 && nYSize <= std::numeric_limits::max()); + nTemp = CPL_LSBWORD16(static_cast(nYSize)); if (VSIFWriteL((void *)&nTemp, 2, 1, fp) != 1) { CPLError(CE_Failure, CPLE_FileIO, @@ -847,8 +849,8 @@ GDALDataset *GSBGDataset::Create(const char *pszFilename, int nXSize, return nullptr; } - CPLErr eErr = WriteHeader(fp, (GInt16)nXSize, (GInt16)nYSize, 0.0, nXSize, - 0.0, nYSize, 0.0, 0.0); + CPLErr eErr = + WriteHeader(fp, nXSize, nYSize, 0.0, nXSize, 0.0, nYSize, 0.0, 0.0); if (eErr != CE_None) { VSIFCloseL(fp); @@ -941,8 +943,8 @@ GDALDataset *GSBGDataset::CreateCopy(const char *pszFilename, return nullptr; } - GInt16 nXSize = (GInt16)poSrcBand->GetXSize(); - GInt16 nYSize = (GInt16)poSrcBand->GetYSize(); + const int nXSize = poSrcBand->GetXSize(); + const int nYSize = poSrcBand->GetYSize(); double adfGeoTransform[6]; poSrcDS->GetGeoTransform(adfGeoTransform); @@ -974,7 +976,7 @@ GDALDataset *GSBGDataset::CreateCopy(const char *pszFilename, float fSrcNoDataValue = (float)poSrcBand->GetNoDataValue(&bSrcHasNDValue); double dfMinZ = std::numeric_limits::max(); double dfMaxZ = std::numeric_limits::lowest(); - for (GInt16 iRow = nYSize - 1; iRow >= 0; iRow--) + for (int iRow = nYSize - 1; iRow >= 0; iRow--) { eErr = poSrcBand->RasterIO(GF_Read, 0, iRow, nXSize, 1, pfData, nXSize, 1, GDT_Float32, 0, 0, nullptr); diff --git a/frmts/gtiff/gt_overview.cpp b/frmts/gtiff/gt_overview.cpp index 38ca89e434c2..6405f32261ad 100644 --- a/frmts/gtiff/gt_overview.cpp +++ b/frmts/gtiff/gt_overview.cpp @@ -188,8 +188,11 @@ toff_t GTIFFWriteDirectory(TIFF *hTIFF, int nSubfileType, int nXSize, } TIFFWriteDirectory(hTIFF); - TIFFSetDirectory(hTIFF, - static_cast(TIFFNumberOfDirectories(hTIFF) - 1)); + const tdir_t nNumberOfDirs = TIFFNumberOfDirectories(hTIFF); + if (nNumberOfDirs > 0) // always true, but to please Coverity + { + TIFFSetDirectory(hTIFF, static_cast(nNumberOfDirs - 1)); + } const toff_t nOffset = TIFFCurrentDirOffset(hTIFF); diff --git a/frmts/gtiff/gtiffdataset_read.cpp b/frmts/gtiff/gtiffdataset_read.cpp index 9e602fa2619d..59da8e19c738 100644 --- a/frmts/gtiff/gtiffdataset_read.cpp +++ b/frmts/gtiff/gtiffdataset_read.cpp @@ -106,7 +106,8 @@ int GTiffDataset::GetJPEGOverviewCount() GByte abyFFD8[] = {0xFF, 0xD8}; if (TIFFGetField(m_hTIFF, TIFFTAG_JPEGTABLES, &nJPEGTableSize, &pJPEGTable)) { - if (pJPEGTable == nullptr || nJPEGTableSize > INT_MAX || + if (pJPEGTable == nullptr || nJPEGTableSize < 2 || + nJPEGTableSize > INT_MAX || static_cast(pJPEGTable)[nJPEGTableSize - 1] != 0xD9) { m_nJPEGOverviewCount = 0; diff --git a/frmts/gtiff/gtiffdataset_write.cpp b/frmts/gtiff/gtiffdataset_write.cpp index 062ba5cd1bfc..601ce94e5685 100644 --- a/frmts/gtiff/gtiffdataset_write.cpp +++ b/frmts/gtiff/gtiffdataset_write.cpp @@ -2117,8 +2117,11 @@ void GTiffDataset::Crystalize() } else { - TIFFSetDirectory( - m_hTIFF, static_cast(TIFFNumberOfDirectories(m_hTIFF) - 1)); + const tdir_t nNumberOfDirs = TIFFNumberOfDirectories(m_hTIFF); + if (nNumberOfDirs > 0) + { + TIFFSetDirectory(m_hTIFF, static_cast(nNumberOfDirs - 1)); + } } RestoreVolatileParameters(m_hTIFF); diff --git a/frmts/gtiff/gtiffrasterband_read.cpp b/frmts/gtiff/gtiffrasterband_read.cpp index b16d57b4436a..22b90e51e259 100644 --- a/frmts/gtiff/gtiffrasterband_read.cpp +++ b/frmts/gtiff/gtiffrasterband_read.cpp @@ -32,6 +32,7 @@ #include "gtiffjpegoverviewds.h" #include +#include #include #include #include @@ -509,6 +510,9 @@ CPLVirtualMem *GTiffRasterBand::GetVirtualMemAutoInternal(GDALRWFlag eRWFlag, CPLAssert(panByteCounts[0] == static_cast(nBlockSize)); // Now simulate the writing of other blocks. + assert(nBlocks > 0); + assert(static_cast(nBlockSize) < + std::numeric_limits::max() / nBlocks); const vsi_l_offset nDataSize = static_cast(nBlockSize) * nBlocks; if (VSIFTruncateL(fp, nBaseOffset + nDataSize) != 0) diff --git a/frmts/mrf/marfa.h b/frmts/mrf/marfa.h index 6e0cff3171d2..20c908fff4fc 100644 --- a/frmts/mrf/marfa.h +++ b/frmts/mrf/marfa.h @@ -966,11 +966,11 @@ class LERC_Band final : public MRFRasterBand protected: virtual CPLErr Decompress(buf_mgr &dst, buf_mgr &src) override; virtual CPLErr Compress(buf_mgr &dst, buf_mgr &src) override; - double precision; + double precision = 0; // L1 or L2 - int version; + int version = 0; // L2 version - int l2ver; + int l2ver = 0; // Build a MRF header for a single LERC tile static CPLXMLNode *GetMRFConfig(GDALOpenInfo *poOpenInfo); diff --git a/frmts/netcdf/netcdfdataset.cpp b/frmts/netcdf/netcdfdataset.cpp index 2960870dccf4..ae3dbf3f9e2e 100644 --- a/frmts/netcdf/netcdfdataset.cpp +++ b/frmts/netcdf/netcdfdataset.cpp @@ -7043,7 +7043,9 @@ bool netCDFDataset::CloneGrp(int nOldGrpId, int nNewGrpId, bool bIsNC4, int nDimCount = -1; int status = nc_inq_ndims(nOldGrpId, &nDimCount); NCDF_ERR(status); - int *panDimIds = static_cast(CPLMalloc(sizeof(int) * nDimCount)); + if (nDimCount < 0 || nDimCount > NC_MAX_DIMS) + return false; + int anDimIds[NC_MAX_DIMS]; int nUnlimiDimID = -1; status = nc_inq_unlimdim(nOldGrpId, &nUnlimiDimID); NCDF_ERR(status); @@ -7052,21 +7054,21 @@ bool netCDFDataset::CloneGrp(int nOldGrpId, int nNewGrpId, bool bIsNC4, // In NC4, the dimension ids of a group are not necessarily in // [0,nDimCount-1] range int nDimCount2 = -1; - status = nc_inq_dimids(nOldGrpId, &nDimCount2, panDimIds, FALSE); + status = nc_inq_dimids(nOldGrpId, &nDimCount2, anDimIds, FALSE); NCDF_ERR(status); CPLAssert(nDimCount == nDimCount2); } else { for (int i = 0; i < nDimCount; i++) - panDimIds[i] = i; + anDimIds[i] = i; } for (int i = 0; i < nDimCount; i++) { char szDimName[NC_MAX_NAME + 1]; szDimName[0] = 0; size_t nLen = 0; - const int nDimId = panDimIds[i]; + const int nDimId = anDimIds[i]; status = nc_inq_dim(nOldGrpId, nDimId, szDimName, &nLen); NCDF_ERR(status); if (NCDFIsUnlimitedDim(bIsNC4, nOldGrpId, nDimId)) @@ -7079,11 +7081,9 @@ bool netCDFDataset::CloneGrp(int nOldGrpId, int nNewGrpId, bool bIsNC4, CPLAssert(nDimId == nNewDimId); if (status != NC_NOERR) { - CPLFree(panDimIds); return false; } } - CPLFree(panDimIds); // Clone main attributes if (!CloneAttributes(nOldGrpId, nNewGrpId, NC_GLOBAL, NC_GLOBAL)) @@ -7108,7 +7108,6 @@ bool netCDFDataset::CloneGrp(int nOldGrpId, int nNewGrpId, bool bIsNC4, int nVarDimCount = -1; status = nc_inq_varndims(nOldGrpId, i, &nVarDimCount); NCDF_ERR(status); - int anDimIds[NC_MAX_DIMS]; status = nc_inq_vardimid(nOldGrpId, i, anDimIds); NCDF_ERR(status); int nNewVarId = -1; @@ -10379,7 +10378,7 @@ static CPLErr NCDFGetAttr1(int nCdfId, int nVarId, const char *pszAttrName, NCDFSafeStrcat(&pszAttrValue, "{", &nAttrValueSize); double dfValue = 0.0; - size_t m; + size_t m = 0; char szTemp[256]; bool bSetDoubleFromStr = false; @@ -10398,10 +10397,13 @@ static CPLErr NCDFGetAttr1(int nCdfId, int nVarId, const char *pszAttrName, CPLCalloc(nAttrLen, sizeof(signed char))); nc_get_att_schar(nCdfId, nVarId, pszAttrName, pscTemp); dfValue = static_cast(pscTemp[0]); - for (m = 0; m < nAttrLen - 1; m++) + if (nAttrLen > 1) { - snprintf(szTemp, sizeof(szTemp), "%d,", pscTemp[m]); - NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + for (m = 0; m < nAttrLen - 1; m++) + { + snprintf(szTemp, sizeof(szTemp), "%d,", pscTemp[m]); + NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + } } snprintf(szTemp, sizeof(szTemp), "%d", pscTemp[m]); NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); @@ -10414,10 +10416,13 @@ static CPLErr NCDFGetAttr1(int nCdfId, int nVarId, const char *pszAttrName, static_cast(CPLCalloc(nAttrLen, sizeof(short))); nc_get_att_short(nCdfId, nVarId, pszAttrName, psTemp); dfValue = static_cast(psTemp[0]); - for (m = 0; m < nAttrLen - 1; m++) + if (nAttrLen > 1) { - snprintf(szTemp, sizeof(szTemp), "%d,", psTemp[m]); - NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + for (m = 0; m < nAttrLen - 1; m++) + { + snprintf(szTemp, sizeof(szTemp), "%d,", psTemp[m]); + NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + } } snprintf(szTemp, sizeof(szTemp), "%d", psTemp[m]); NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); @@ -10429,10 +10434,13 @@ static CPLErr NCDFGetAttr1(int nCdfId, int nVarId, const char *pszAttrName, int *pnTemp = static_cast(CPLCalloc(nAttrLen, sizeof(int))); nc_get_att_int(nCdfId, nVarId, pszAttrName, pnTemp); dfValue = static_cast(pnTemp[0]); - for (m = 0; m < nAttrLen - 1; m++) + if (nAttrLen > 1) { - snprintf(szTemp, sizeof(szTemp), "%d,", pnTemp[m]); - NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + for (m = 0; m < nAttrLen - 1; m++) + { + snprintf(szTemp, sizeof(szTemp), "%d,", pnTemp[m]); + NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + } } snprintf(szTemp, sizeof(szTemp), "%d", pnTemp[m]); NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); @@ -10445,10 +10453,13 @@ static CPLErr NCDFGetAttr1(int nCdfId, int nVarId, const char *pszAttrName, static_cast(CPLCalloc(nAttrLen, sizeof(float))); nc_get_att_float(nCdfId, nVarId, pszAttrName, pfTemp); dfValue = static_cast(pfTemp[0]); - for (m = 0; m < nAttrLen - 1; m++) + if (nAttrLen > 1) { - CPLsnprintf(szTemp, sizeof(szTemp), "%.8g,", pfTemp[m]); - NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + for (m = 0; m < nAttrLen - 1; m++) + { + CPLsnprintf(szTemp, sizeof(szTemp), "%.8g,", pfTemp[m]); + NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + } } CPLsnprintf(szTemp, sizeof(szTemp), "%.8g", pfTemp[m]); NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); @@ -10461,10 +10472,13 @@ static CPLErr NCDFGetAttr1(int nCdfId, int nVarId, const char *pszAttrName, static_cast(CPLCalloc(nAttrLen, sizeof(double))); nc_get_att_double(nCdfId, nVarId, pszAttrName, pdfTemp); dfValue = pdfTemp[0]; - for (m = 0; m < nAttrLen - 1; m++) + if (nAttrLen > 1) { - CPLsnprintf(szTemp, sizeof(szTemp), "%.16g,", pdfTemp[m]); - NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + for (m = 0; m < nAttrLen - 1; m++) + { + CPLsnprintf(szTemp, sizeof(szTemp), "%.16g,", pdfTemp[m]); + NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + } } CPLsnprintf(szTemp, sizeof(szTemp), "%.16g", pdfTemp[m]); NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); @@ -10478,12 +10492,15 @@ static CPLErr NCDFGetAttr1(int nCdfId, int nVarId, const char *pszAttrName, nc_get_att_string(nCdfId, nVarId, pszAttrName, ppszTemp); bSetDoubleFromStr = true; dfValue = 0.0; - for (m = 0; m < nAttrLen - 1; m++) + if (nAttrLen > 1) { - NCDFSafeStrcat(&pszAttrValue, - ppszTemp[m] ? ppszTemp[m] : "{NULL}", - &nAttrValueSize); - NCDFSafeStrcat(&pszAttrValue, ",", &nAttrValueSize); + for (m = 0; m < nAttrLen - 1; m++) + { + NCDFSafeStrcat(&pszAttrValue, + ppszTemp[m] ? ppszTemp[m] : "{NULL}", + &nAttrValueSize); + NCDFSafeStrcat(&pszAttrValue, ",", &nAttrValueSize); + } } NCDFSafeStrcat(&pszAttrValue, ppszTemp[m] ? ppszTemp[m] : "{NULL}", &nAttrValueSize); @@ -10497,10 +10514,13 @@ static CPLErr NCDFGetAttr1(int nCdfId, int nVarId, const char *pszAttrName, CPLCalloc(nAttrLen, sizeof(unsigned char))); nc_get_att_uchar(nCdfId, nVarId, pszAttrName, pucTemp); dfValue = static_cast(pucTemp[0]); - for (m = 0; m < nAttrLen - 1; m++) + if (nAttrLen > 1) { - CPLsnprintf(szTemp, sizeof(szTemp), "%u,", pucTemp[m]); - NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + for (m = 0; m < nAttrLen - 1; m++) + { + CPLsnprintf(szTemp, sizeof(szTemp), "%u,", pucTemp[m]); + NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + } } CPLsnprintf(szTemp, sizeof(szTemp), "%u", pucTemp[m]); NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); @@ -10514,10 +10534,13 @@ static CPLErr NCDFGetAttr1(int nCdfId, int nVarId, const char *pszAttrName, CPLCalloc(nAttrLen, sizeof(unsigned short))); nc_get_att_ushort(nCdfId, nVarId, pszAttrName, pusTemp); dfValue = static_cast(pusTemp[0]); - for (m = 0; m < nAttrLen - 1; m++) + if (nAttrLen > 1) { - CPLsnprintf(szTemp, sizeof(szTemp), "%u,", pusTemp[m]); - NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + for (m = 0; m < nAttrLen - 1; m++) + { + CPLsnprintf(szTemp, sizeof(szTemp), "%u,", pusTemp[m]); + NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + } } CPLsnprintf(szTemp, sizeof(szTemp), "%u", pusTemp[m]); NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); @@ -10530,10 +10553,13 @@ static CPLErr NCDFGetAttr1(int nCdfId, int nVarId, const char *pszAttrName, static_cast(CPLCalloc(nAttrLen, sizeof(int))); nc_get_att_uint(nCdfId, nVarId, pszAttrName, punTemp); dfValue = static_cast(punTemp[0]); - for (m = 0; m < nAttrLen - 1; m++) + if (nAttrLen > 1) { - CPLsnprintf(szTemp, sizeof(szTemp), "%u,", punTemp[m]); - NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + for (m = 0; m < nAttrLen - 1; m++) + { + CPLsnprintf(szTemp, sizeof(szTemp), "%u,", punTemp[m]); + NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + } } CPLsnprintf(szTemp, sizeof(szTemp), "%u", punTemp[m]); NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); @@ -10546,11 +10572,14 @@ static CPLErr NCDFGetAttr1(int nCdfId, int nVarId, const char *pszAttrName, static_cast(CPLCalloc(nAttrLen, sizeof(GIntBig))); nc_get_att_longlong(nCdfId, nVarId, pszAttrName, panTemp); dfValue = static_cast(panTemp[0]); - for (m = 0; m < nAttrLen - 1; m++) + if (nAttrLen > 1) { - CPLsnprintf(szTemp, sizeof(szTemp), CPL_FRMT_GIB ",", - panTemp[m]); - NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + for (m = 0; m < nAttrLen - 1; m++) + { + CPLsnprintf(szTemp, sizeof(szTemp), CPL_FRMT_GIB ",", + panTemp[m]); + NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + } } CPLsnprintf(szTemp, sizeof(szTemp), CPL_FRMT_GIB, panTemp[m]); NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); @@ -10563,11 +10592,14 @@ static CPLErr NCDFGetAttr1(int nCdfId, int nVarId, const char *pszAttrName, static_cast(CPLCalloc(nAttrLen, sizeof(GUIntBig))); nc_get_att_ulonglong(nCdfId, nVarId, pszAttrName, panTemp); dfValue = static_cast(panTemp[0]); - for (m = 0; m < nAttrLen - 1; m++) + if (nAttrLen > 1) { - CPLsnprintf(szTemp, sizeof(szTemp), CPL_FRMT_GUIB ",", - panTemp[m]); - NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + for (m = 0; m < nAttrLen - 1; m++) + { + CPLsnprintf(szTemp, sizeof(szTemp), CPL_FRMT_GUIB ",", + panTemp[m]); + NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); + } } CPLsnprintf(szTemp, sizeof(szTemp), CPL_FRMT_GUIB, panTemp[m]); NCDFSafeStrcat(&pszAttrValue, szTemp, &nAttrValueSize); diff --git a/frmts/northwood/northwood.cpp b/frmts/northwood/northwood.cpp index 1917e154f2fd..a12d8a849a8e 100644 --- a/frmts/northwood/northwood.cpp +++ b/frmts/northwood/northwood.cpp @@ -32,6 +32,7 @@ #include "northwood.h" #include +#include #include #include @@ -79,6 +80,7 @@ int nwt_ParseHeader(NWT_GRID *pGrd, const unsigned char *nwtHeader) memcpy(&pGrd->dfMaxY, &nwtHeader[37], sizeof(pGrd->dfMaxY)); CPL_LSBPTR64(&pGrd->dfMaxY); + assert(pGrd->nXSide > 1); pGrd->dfStepSize = (pGrd->dfMaxX - pGrd->dfMinX) / (pGrd->nXSide - 1); /* dfTmp = (pGrd->dfMaxY - pGrd->dfMinY) / (pGrd->nYSide - 1); */ diff --git a/frmts/pdf/pdfdataset.cpp b/frmts/pdf/pdfdataset.cpp index 33bc89b0f49d..1f6b63961887 100644 --- a/frmts/pdf/pdfdataset.cpp +++ b/frmts/pdf/pdfdataset.cpp @@ -2480,7 +2480,7 @@ GDALPDFObject *PDFDataset::GetCatalog() return m_poCatalogObject; #ifdef HAVE_POPPLER - if (m_bUseLib.test(PDFLIB_POPPLER)) + if (m_bUseLib.test(PDFLIB_POPPLER) && m_poDocPoppler) { m_poCatalogObjectPoppler = std::make_unique(m_poDocPoppler->getXRef()->getCatalog()); @@ -2491,7 +2491,7 @@ GDALPDFObject *PDFDataset::GetCatalog() #endif #ifdef HAVE_PODOFO - if (m_bUseLib.test(PDFLIB_PODOFO)) + if (m_bUseLib.test(PDFLIB_PODOFO) && m_poDocPodofo) { int nCatalogNum = 0; int nCatalogGen = 0; @@ -2517,7 +2517,7 @@ GDALPDFObject *PDFDataset::GetCatalog() #endif #ifdef HAVE_PDFIUM - if (m_bUseLib.test(PDFLIB_PDFIUM)) + if (m_bUseLib.test(PDFLIB_PDFIUM) && m_poDocPdfium) { const CPDF_Dictionary *catalog = m_poDocPdfium->doc->GetRoot(); if (catalog) @@ -4957,11 +4957,11 @@ PDFDataset *PDFDataset::Open(GDALOpenInfo *poOpenInfo) return nullptr; } poPageObj = GDALPDFObjectPdfium::Build(pageObj); - if (poPageObj == nullptr) - return nullptr; } #endif // ~ HAVE_PDFIUM + if (poPageObj == nullptr) + return nullptr; GDALPDFDictionary *poPageDict = poPageObj->GetDictionary(); if (poPageDict == nullptr) { @@ -5032,7 +5032,9 @@ PDFDataset *PDFDataset::Open(GDALOpenInfo *poOpenInfo) if (pszDumpCatalog != nullptr) { GDALPDFDumper oDumper(pszFilename, pszDumpCatalog); - oDumper.Dump(poDS->GetCatalog()); + auto poCatalog = poDS->GetCatalog(); + if (poCatalog) + oDumper.Dump(poCatalog); } int nBandsGuessed = 0; @@ -5090,6 +5092,7 @@ PDFDataset *PDFDataset::Open(GDALOpenInfo *poOpenInfo) #ifdef HAVE_PDFIUM if (bUseLib.test(PDFLIB_PDFIUM)) { + CPLAssert(poPagePdfium); CFX_FloatRect rect = poPagePdfium->page->GetBBox(); dfX1 = rect.left; dfX2 = rect.right; @@ -5134,6 +5137,7 @@ PDFDataset *PDFDataset::Open(GDALOpenInfo *poOpenInfo) #ifdef HAVE_PDFIUM if (bUseLib.test(PDFLIB_PDFIUM)) { + CPLAssert(poPagePdfium); dfRotation = poPagePdfium->page->GetPageRotation() * 90; } #endif diff --git a/gcore/gdaljp2structure.cpp b/gcore/gdaljp2structure.cpp index b3ae58eba05f..e55f0787b93a 100644 --- a/gcore/gdaljp2structure.cpp +++ b/gcore/gdaljp2structure.cpp @@ -29,6 +29,7 @@ #include "cpl_port.h" #include "gdaljp2metadata.h" +#include #include #include #if HAVE_FCNTL_H @@ -2177,7 +2178,8 @@ static void GDALGetJPEG2000StructureInternal(CPLXMLNode *psParent, VSILFILE *fp, CPLXMLNode *psBinaryContent = CPLCreateXMLNode(nullptr, CXT_Element, "BinaryContent"); GByte *pabyBoxData = oBox.ReadBoxData(); - int nBoxLength = static_cast(nBoxDataLength); + const int nBoxLength = static_cast( + std::min(nBoxDataLength, INT_MAX / 2 - 1)); char *pszBinaryContent = static_cast(VSIMalloc(2 * nBoxLength + 1)); if (pabyBoxData && pszBinaryContent) diff --git a/gcore/gdalmultidim.cpp b/gcore/gdalmultidim.cpp index 2e380faaafd1..ab7ece1b95f1 100644 --- a/gcore/gdalmultidim.cpp +++ b/gcore/gdalmultidim.cpp @@ -3116,6 +3116,7 @@ bool GDALAbstractMDArray::ProcessPerChunk(const GUInt64 *arrayStartIdx, goto end; } + assert(dimIdx > 0); dimIdx--; // cppcheck-suppress negativeContainerIndex switch (stack[dimIdx].return_point) diff --git a/ogr/ogr_p.h b/ogr/ogr_p.h index 6fde155e9c1f..f4ee626786d1 100644 --- a/ogr/ogr_p.h +++ b/ogr/ogr_p.h @@ -276,7 +276,8 @@ inline uint64_t OGRRoundValueIEEE754(uint64_t nVal, int nBitsPrecision) return nVal; if (nNullifiedBits >= MANTISSA_SIZE) nNullifiedBits = MANTISSA_SIZE; - nVal &= std::numeric_limits::max() << nNullifiedBits; + nVal >>= nNullifiedBits; + nVal <<= nNullifiedBits; return nVal; } diff --git a/ogr/ogrlinestring.cpp b/ogr/ogrlinestring.cpp index 8d99615b4cec..9d59218fb9a4 100644 --- a/ogr/ogrlinestring.cpp +++ b/ogr/ogrlinestring.cpp @@ -2528,6 +2528,8 @@ void OGRSimpleCurve::segmentize(double dfMaxLength) const double dfSquareMaxLength = dfMaxLength * dfMaxLength; // First pass to compute new number of points + constexpr double REL_EPSILON_LENGTH_SQUARE = 1e-5; + constexpr double REL_EPSILON_ROUND = 1e-2; for (int i = 0; i < nPointCount; i++) { nNewPointCount++; @@ -2539,10 +2541,11 @@ void OGRSimpleCurve::segmentize(double dfMaxLength) const double dfX = paoPoints[i + 1].x - paoPoints[i].x; const double dfY = paoPoints[i + 1].y - paoPoints[i].y; const double dfSquareDist = dfX * dfX + dfY * dfY; - if (dfSquareDist - dfSquareMaxLength > 1e-5 * dfSquareMaxLength) + if (dfSquareDist - dfSquareMaxLength > + REL_EPSILON_LENGTH_SQUARE * dfSquareMaxLength) { - const double dfIntermediatePoints = - floor(sqrt(dfSquareDist / dfSquareMaxLength) - 1e-2); + const double dfIntermediatePoints = floor( + sqrt(dfSquareDist / dfSquareMaxLength) - REL_EPSILON_ROUND); const int nIntermediatePoints = DoubleToIntClamp(dfIntermediatePoints); @@ -2620,28 +2623,33 @@ void OGRSimpleCurve::segmentize(double dfMaxLength) const double dfX = paoPoints[i + 1].x - paoPoints[i].x; const double dfY = paoPoints[i + 1].y - paoPoints[i].y; const double dfSquareDist = dfX * dfX + dfY * dfY; - if (dfSquareDist - dfSquareMaxLength > 1e-5 * dfSquareMaxLength) + + // Must be kept in sync with the initial pass loop + if (dfSquareDist - dfSquareMaxLength > + REL_EPSILON_LENGTH_SQUARE * dfSquareMaxLength) { - const double dfIntermediatePoints = - floor(sqrt(dfSquareDist / dfSquareMaxLength) - 1e-2); + const double dfIntermediatePoints = floor( + sqrt(dfSquareDist / dfSquareMaxLength) - REL_EPSILON_ROUND); const int nIntermediatePoints = DoubleToIntClamp(dfIntermediatePoints); for (int j = 1; j <= nIntermediatePoints; j++) { - paoNewPoints[nNewPointCount + j - 1].x = + // coverity[overflow_const] + const int newI = nNewPointCount + j - 1; + paoNewPoints[newI].x = paoPoints[i].x + j * dfX / (nIntermediatePoints + 1); - paoNewPoints[nNewPointCount + j - 1].y = + paoNewPoints[newI].y = paoPoints[i].y + j * dfY / (nIntermediatePoints + 1); if (padfZ != nullptr) { // No interpolation. - padfNewZ[nNewPointCount + j - 1] = padfZ[i]; + padfNewZ[newI] = padfZ[i]; } if (padfM != nullptr) { // No interpolation. - padfNewM[nNewPointCount + j - 1] = padfM[i]; + padfNewM[newI] = padfM[i]; } } diff --git a/ogr/ogrsf_frmts/arrow_common/ograrrowlayer.hpp b/ogr/ogrsf_frmts/arrow_common/ograrrowlayer.hpp index ac5d7d96b256..b364925ea9a9 100644 --- a/ogr/ogrsf_frmts/arrow_common/ograrrowlayer.hpp +++ b/ogr/ogrsf_frmts/arrow_common/ograrrowlayer.hpp @@ -2385,7 +2385,7 @@ inline OGRFeature *OGRArrowLayer::ReadFeature( arrow::LargeBinaryArray::offset_type out_length = 0; const uint8_t *data = castArray->GetValue(nIdxInBatch, &out_length); - if (out_length <= INT_MAX) + if (out_length <= INT_MAX - 1) { poFeature->SetField(i, static_cast(out_length), data); } diff --git a/ogr/ogrsf_frmts/geojson/libjson/json_util.c b/ogr/ogrsf_frmts/geojson/libjson/json_util.c index 0fcb0d4298d3..505834d83ce6 100644 --- a/ogr/ogrsf_frmts/geojson/libjson/json_util.c +++ b/ogr/ogrsf_frmts/geojson/libjson/json_util.c @@ -188,7 +188,7 @@ int json_object_to_fd(int fd, struct json_object *obj, int flags) } static int _json_object_to_fd(int fd, struct json_object *obj, int flags, const char *filename) { - int ret; + ssize_t ret; const char *json_str; unsigned int wpos, wsize; @@ -204,7 +204,7 @@ static int _json_object_to_fd(int fd, struct json_object *obj, int flags, const wpos = 0; while (wpos < wsize) { - if ((ret = (int)write(fd, json_str + wpos, wsize - wpos)) < 0) + if ((ret = write(fd, json_str + wpos, wsize - wpos)) < 0) { _json_c_set_last_err("json_object_to_file: error writing file %s: %s\n", filename, strerror(errno)); diff --git a/ogr/ogrsf_frmts/gmlas/ogrgmlaslayer.cpp b/ogr/ogrsf_frmts/gmlas/ogrgmlaslayer.cpp index 4635659c873c..dccaf32d23d1 100644 --- a/ogr/ogrsf_frmts/gmlas/ogrgmlaslayer.cpp +++ b/ogr/ogrsf_frmts/gmlas/ogrgmlaslayer.cpp @@ -1128,7 +1128,7 @@ void OGRGMLASLayer::InsertNewField(int nInsertPos, if (strcmp(poFeature->GetFieldAsString(szLAYER_NAME), GetName()) == 0) { int nFieldIndex = poFeature->GetFieldAsInteger(szFIELD_INDEX); - if (nFieldIndex >= nInsertPos) + if (nFieldIndex >= nInsertPos && nFieldIndex < INT_MAX) { poFeature->SetField(szFIELD_INDEX, nFieldIndex + 1); CPL_IGNORE_RET_VAL( diff --git a/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp b/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp index ad16578e40d5..be706dbebb4a 100644 --- a/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp +++ b/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp @@ -8231,7 +8231,10 @@ void OGR_GPKG_FillArrowArray_Step(sqlite3_context *pContext, int /*argc*/, } if (iField == psHelper->m_nFieldCount) + { + std::unique_lock oLock(psFillArrowArray->oMutex); psFillArrowArray->nCountRows++; + } return; error: @@ -8655,7 +8658,13 @@ int OGRGeoPackageTableLayer::GetNextArrowArray(struct ArrowArrayStream *stream, sizeof(struct ArrowArray)); memset(task->m_psArrowArray.get(), 0, sizeof(struct ArrowArray)); - if (task->m_bMemoryLimitReached) + const bool bMemoryLimitReached = [&task]() + { + std::unique_lock oLock(task->m_oMutex); + return task->m_bMemoryLimitReached; + }(); + + if (bMemoryLimitReached) { m_nIsCompatOfOptimizedGetNextArrowArray = false; stopThread(); diff --git a/ogr/ogrsf_frmts/mvt/ogrmvtdataset.cpp b/ogr/ogrsf_frmts/mvt/ogrmvtdataset.cpp index 3302b5d938d9..fadf1938b908 100644 --- a/ogr/ogrsf_frmts/mvt/ogrmvtdataset.cpp +++ b/ogr/ogrsf_frmts/mvt/ogrmvtdataset.cpp @@ -4265,24 +4265,35 @@ OGRErr OGRMVTWriterDataset::PreGenerateForTileReal( oBuffer.assign(static_cast(pCompressed), nCompressedSize); CPLFree(pCompressed); - std::unique_ptr> poLockGuard; + const auto InsertIntoDb = [&]() + { + m_nTempTiles++; + sqlite3_bind_int(m_hInsertStmt, 1, nZ); + sqlite3_bind_int(m_hInsertStmt, 2, nTileX); + sqlite3_bind_int(m_hInsertStmt, 3, nTileY); + sqlite3_bind_text(m_hInsertStmt, 4, osTargetName.c_str(), -1, + SQLITE_STATIC); + sqlite3_bind_int64(m_hInsertStmt, 5, nSerial); + sqlite3_bind_blob(m_hInsertStmt, 6, oBuffer.data(), + static_cast(oBuffer.size()), SQLITE_STATIC); + sqlite3_bind_int(m_hInsertStmt, 7, + static_cast(poGPBFeature->getType())); + sqlite3_bind_double(m_hInsertStmt, 8, dfAreaOrLength); + int rc = sqlite3_step(m_hInsertStmt); + sqlite3_reset(m_hInsertStmt); + return rc; + }; + + int rc; if (m_bThreadPoolOK) - poLockGuard = std::make_unique>(m_oDBMutex); - - m_nTempTiles++; - sqlite3_bind_int(m_hInsertStmt, 1, nZ); - sqlite3_bind_int(m_hInsertStmt, 2, nTileX); - sqlite3_bind_int(m_hInsertStmt, 3, nTileY); - sqlite3_bind_text(m_hInsertStmt, 4, osTargetName.c_str(), -1, - SQLITE_STATIC); - sqlite3_bind_int64(m_hInsertStmt, 5, nSerial); - sqlite3_bind_blob(m_hInsertStmt, 6, oBuffer.data(), - static_cast(oBuffer.size()), SQLITE_STATIC); - sqlite3_bind_int(m_hInsertStmt, 7, - static_cast(poGPBFeature->getType())); - sqlite3_bind_double(m_hInsertStmt, 8, dfAreaOrLength); - int rc = sqlite3_step(m_hInsertStmt); - sqlite3_reset(m_hInsertStmt); + { + std::lock_guard oLock(m_oDBMutex); + rc = InsertIntoDb(); + } + else + { + rc = InsertIntoDb(); + } if (!(rc == SQLITE_OK || rc == SQLITE_DONE)) { diff --git a/ogr/ogrsf_frmts/ntf/ntfrecord.cpp b/ogr/ogrsf_frmts/ntf/ntfrecord.cpp index 0bf6bebec369..1fe02fb7ed63 100644 --- a/ogr/ogrsf_frmts/ntf/ntfrecord.cpp +++ b/ogr/ogrsf_frmts/ntf/ntfrecord.cpp @@ -74,6 +74,7 @@ NTFRecord::NTFRecord(VSILFILE *fp) : nType(99), nLength(0), pszData(nullptr) if (pszData == nullptr) { nLength = nNewLength - 2; + // coverity[overflow_sink] pszData = static_cast(VSI_MALLOC_VERBOSE(nLength + 1)); if (pszData == nullptr) { diff --git a/ogr/ogrsf_frmts/openfilegdb/filegdbtable.cpp b/ogr/ogrsf_frmts/openfilegdb/filegdbtable.cpp index 20d715ea3eef..18936e188acb 100644 --- a/ogr/ogrsf_frmts/openfilegdb/filegdbtable.cpp +++ b/ogr/ogrsf_frmts/openfilegdb/filegdbtable.cpp @@ -30,6 +30,7 @@ #include "filegdbtable.h" #include +#include #include #include #include @@ -3664,6 +3665,7 @@ OGRGeometry *FileGDBOGRGeometryConverterImpl::CreateCurveGeometry( returnError(); } const int nMaxSize = static_cast(nMaxSize64); + // coverity[overflow_sink] GByte *pabyExtShapeBuffer = static_cast(VSI_MALLOC_VERBOSE(nMaxSize)); if (pabyExtShapeBuffer == nullptr) @@ -3860,11 +3862,19 @@ FileGDBOGRGeometryConverterImpl::GetAsGeometry(const OGRField *psField) ReadVarUInt64NoCheck(pabyCur, m); const double dfMScale = SanitizeScale(poGeomField->GetMScale()); - const double dfM = - m == 0 - ? std::numeric_limits::quiet_NaN() - : (m - 1U) / dfMScale + poGeomField->GetMOrigin(); - return new OGRPoint(dfX, dfY, dfZ, dfM); + if (m == 0) + { + return new OGRPoint( + dfX, dfY, dfZ, + std::numeric_limits::quiet_NaN()); + } + else + { + assert(m >= 1U); + const double dfM = + (m - 1U) / dfMScale + poGeomField->GetMOrigin(); + return new OGRPoint(dfX, dfY, dfZ, dfM); + } } return new OGRPoint(dfX, dfY, dfZ); } diff --git a/ogr/ogrsf_frmts/openfilegdb/filegdbtable_freelist.cpp b/ogr/ogrsf_frmts/openfilegdb/filegdbtable_freelist.cpp index c666c1bbce44..56768752d458 100644 --- a/ogr/ogrsf_frmts/openfilegdb/filegdbtable_freelist.cpp +++ b/ogr/ogrsf_frmts/openfilegdb/filegdbtable_freelist.cpp @@ -32,6 +32,7 @@ #include "filegdbtable_priv.h" #include +#include #include #include @@ -140,6 +141,7 @@ void FileGDBTable::AddEntryToFreelist(uint64_t nOffset, uint32_t nSize) VSIFCloseL(fp); return; } + assert(iSlot < 100); // Read the last page index of the identified slot uint32_t nPageIdx = @@ -284,6 +286,7 @@ uint64_t FileGDBTable::GetOffsetOfFreeAreaFromFreeList(uint32_t nSize) VSIFCloseL(fp); return OFFSET_MINUS_ONE; } + assert(iSlot < 100); // Read the last page index of the identified slot uint32_t nPageIdx = diff --git a/ogr/ogrsf_frmts/osm/ogrosmdatasource.cpp b/ogr/ogrsf_frmts/osm/ogrosmdatasource.cpp index 53226246e321..895574462710 100644 --- a/ogr/ogrsf_frmts/osm/ogrosmdatasource.cpp +++ b/ogr/ogrsf_frmts/osm/ogrosmdatasource.cpp @@ -925,6 +925,7 @@ void OGROSMDataSource::LookupNodesSQLite() if (nToQuery > static_cast(LIMIT_IDS_PER_REQUEST)) nToQuery = static_cast(LIMIT_IDS_PER_REQUEST); + assert(nToQuery > 0); sqlite3_stmt *hStmt = m_pahSelectNodeStmt[nToQuery - 1]; for (unsigned int i = iCur; i < iCur + nToQuery; i++) { diff --git a/ogr/ogrsf_frmts/pmtiles/pmtiles/pmtiles.hpp b/ogr/ogrsf_frmts/pmtiles/pmtiles/pmtiles.hpp index 82ed8c56f5d0..c09f44d02020 100644 --- a/ogr/ogrsf_frmts/pmtiles/pmtiles/pmtiles.hpp +++ b/ogr/ogrsf_frmts/pmtiles/pmtiles/pmtiles.hpp @@ -576,6 +576,9 @@ inline std::tuple make_root_leaves(const std::fun if (root_bytes.length() < 16384 - 127) { return std::make_tuple(root_bytes, leaves_bytes, num_leaves); } + if (leaf_size > std::numeric_limits::max() / 2) { + return std::make_tuple(compressed, "", 0); + } leaf_size *= 2; } } diff --git a/ogr/ogrsf_frmts/shape/ogrshape.h b/ogr/ogrsf_frmts/shape/ogrshape.h index 471471f5f1a7..b907abff3bab 100644 --- a/ogr/ogrsf_frmts/shape/ogrshape.h +++ b/ogr/ogrsf_frmts/shape/ogrshape.h @@ -339,6 +339,7 @@ class OGRShapeDataSource final : public OGRDataSource VSILFILE *m_psLockFile = nullptr; CPLJoinableThread *m_hRefreshLockFileThread = nullptr; bool m_bExitRefreshLockFileThread = false; + bool m_bRefreshLockFileThreadStarted = false; double m_dfRefreshLockDelay = 0; std::vector GetLayerNames() const; diff --git a/ogr/ogrsf_frmts/shape/ogrshapedatasource.cpp b/ogr/ogrsf_frmts/shape/ogrshapedatasource.cpp index 8b8d432e1373..69e45b7d4b50 100644 --- a/ogr/ogrsf_frmts/shape/ogrshapedatasource.cpp +++ b/ogr/ogrsf_frmts/shape/ogrshapedatasource.cpp @@ -1403,6 +1403,7 @@ void OGRShapeDataSource::RefreshLockFile(void *_self) OGRShapeDataSource *self = static_cast(_self); CPLAssert(self->m_psLockFile); CPLAcquireMutex(self->m_poRefreshLockFileMutex, 1000); + self->m_bRefreshLockFileThreadStarted = true; CPLCondSignal(self->m_poRefreshLockFileCond); unsigned int nInc = 0; while (!(self->m_bExitRefreshLockFileThread)) @@ -1500,6 +1501,7 @@ bool OGRShapeDataSource::UncompressIfNeeded() } m_psLockFile = f; m_bExitRefreshLockFileThread = false; + m_bRefreshLockFileThreadStarted = false; // Config option mostly for testing purposes // coverity[tainted_data] m_dfRefreshLockDelay = CPLAtof(CPLGetConfigOption( @@ -1516,7 +1518,10 @@ bool OGRShapeDataSource::UncompressIfNeeded() else { CPLAcquireMutex(m_poRefreshLockFileMutex, 1000); - CPLCondWait(m_poRefreshLockFileCond, m_poRefreshLockFileMutex); + while (!m_bRefreshLockFileThreadStarted) + { + CPLCondWait(m_poRefreshLockFileCond, m_poRefreshLockFileMutex); + } CPLReleaseMutex(m_poRefreshLockFileMutex); } } diff --git a/ogr/ogrsf_frmts/vfk/vfkreader.cpp b/ogr/ogrsf_frmts/vfk/vfkreader.cpp index 16fbdc85e0ff..05f61e78ce08 100644 --- a/ogr/ogrsf_frmts/vfk/vfkreader.cpp +++ b/ogr/ogrsf_frmts/vfk/vfkreader.cpp @@ -314,7 +314,7 @@ int VFKReader::ReadDataBlocks(bool bSuppressGeometry) \return number of data records or -1 on error */ -int VFKReader::ReadDataRecords(IVFKDataBlock *poDataBlock) +int64_t VFKReader::ReadDataRecords(IVFKDataBlock *poDataBlock) { const char *pszName = nullptr; IVFKDataBlock *poDataBlockCurrent = nullptr; @@ -342,7 +342,7 @@ int VFKReader::ReadDataRecords(IVFKDataBlock *poDataBlock) int iLine = 0; int nSkipped = 0; int nDupl = 0; - int nRecords = 0; + int64_t nRecords = 0; bool bInHeader = true; CPLString osBlockNameLast; char *pszLine = nullptr; diff --git a/ogr/ogrsf_frmts/vfk/vfkreader.h b/ogr/ogrsf_frmts/vfk/vfkreader.h index 6d3898350017..7ce3fbfe2399 100644 --- a/ogr/ogrsf_frmts/vfk/vfkreader.h +++ b/ogr/ogrsf_frmts/vfk/vfkreader.h @@ -470,7 +470,7 @@ class IVFKReader virtual bool IsValid() const = 0; virtual bool HasFileField() const = 0; virtual int ReadDataBlocks(bool = false) = 0; - virtual int ReadDataRecords(IVFKDataBlock * = nullptr) = 0; + virtual int64_t ReadDataRecords(IVFKDataBlock * = nullptr) = 0; virtual int LoadGeometry() = 0; virtual int GetDataBlockCount() const = 0; diff --git a/ogr/ogrsf_frmts/vfk/vfkreaderp.h b/ogr/ogrsf_frmts/vfk/vfkreaderp.h index 735ecece239c..8891158fa392 100644 --- a/ogr/ogrsf_frmts/vfk/vfkreaderp.h +++ b/ogr/ogrsf_frmts/vfk/vfkreaderp.h @@ -108,7 +108,7 @@ class VFKReader : public IVFKReader } int ReadDataBlocks(bool = false) override; - int ReadDataRecords(IVFKDataBlock * = nullptr) override; + int64_t ReadDataRecords(IVFKDataBlock * = nullptr) override; int LoadGeometry() override; int GetDataBlockCount() const override @@ -166,7 +166,7 @@ class VFKReaderSQLite : public VFKReader } int ReadDataBlocks(bool = false) override; - int ReadDataRecords(IVFKDataBlock * = nullptr) override; + int64_t ReadDataRecords(IVFKDataBlock * = nullptr) override; sqlite3_stmt *PrepareStatement(const char *); OGRErr ExecuteSQL(const char *, CPLErr = CE_Failure); diff --git a/ogr/ogrsf_frmts/vfk/vfkreadersqlite.cpp b/ogr/ogrsf_frmts/vfk/vfkreadersqlite.cpp index 84c71e5326a5..6a49db943a5e 100644 --- a/ogr/ogrsf_frmts/vfk/vfkreadersqlite.cpp +++ b/ogr/ogrsf_frmts/vfk/vfkreadersqlite.cpp @@ -338,13 +338,13 @@ int VFKReaderSQLite::ReadDataBlocks(bool bSuppressGeometry) \return number of data records or -1 on error */ -int VFKReaderSQLite::ReadDataRecords(IVFKDataBlock *poDataBlock) +int64_t VFKReaderSQLite::ReadDataRecords(IVFKDataBlock *poDataBlock) { CPLString osSQL; IVFKDataBlock *poDataBlockCurrent = nullptr; sqlite3_stmt *hStmt = nullptr; const char *pszName = nullptr; - int nDataRecords = 0; + int64_t nDataRecords = 0; bool bReadVfk = !m_bDbSource; bool bReadDb = false; @@ -360,7 +360,7 @@ int VFKReaderSQLite::ReadDataRecords(IVFKDataBlock *poDataBlock) hStmt = PrepareStatement(osSQL.c_str()); if (ExecuteSQL(hStmt) == OGRERR_NONE) { - nDataRecords = sqlite3_column_int(hStmt, 0); + nDataRecords = sqlite3_column_int64(hStmt, 0); if (nDataRecords > 0) bReadDb = true; /* -> read from DB */ else diff --git a/ogr/ogrsf_frmts/vrt/ogrvrtdriver.cpp b/ogr/ogrsf_frmts/vrt/ogrvrtdriver.cpp index 65bfa072e3a1..f86a32a817e8 100644 --- a/ogr/ogrsf_frmts/vrt/ogrvrtdriver.cpp +++ b/ogr/ogrsf_frmts/vrt/ogrvrtdriver.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -130,9 +131,14 @@ static GDALDataset *OGRVRTDriverOpen(GDALOpenInfo *poOpenInfo) "configuration option"); return nullptr; } + if (static_cast(sStatBuf.st_size) > + std::numeric_limits::max() - 1) + { + return nullptr; + } // It is the right file, now load the full XML definition. - const int nLen = static_cast(sStatBuf.st_size); + const size_t nLen = static_cast(sStatBuf.st_size); pszXML = static_cast(VSI_MALLOC_VERBOSE(nLen + 1)); if (pszXML == nullptr) @@ -140,8 +146,7 @@ static GDALDataset *OGRVRTDriverOpen(GDALOpenInfo *poOpenInfo) pszXML[nLen] = '\0'; VSIFSeekL(poOpenInfo->fpL, 0, SEEK_SET); - if (static_cast(VSIFReadL(pszXML, 1, nLen, poOpenInfo->fpL)) != - nLen) + if (VSIFReadL(pszXML, 1, nLen, poOpenInfo->fpL) != nLen) { CPLFree(pszXML); return nullptr; diff --git a/ogr/ogrsf_frmts/wfs/ogrwfsdatasource.cpp b/ogr/ogrsf_frmts/wfs/ogrwfsdatasource.cpp index 3bb8ef68f0b2..cbfcec134998 100644 --- a/ogr/ogrsf_frmts/wfs/ogrwfsdatasource.cpp +++ b/ogr/ogrsf_frmts/wfs/ogrwfsdatasource.cpp @@ -1376,6 +1376,7 @@ int OGRWFSDataSource::Open(const char *pszFilename, int bUpdateIn, OGRLayer::GetSupportedSRSListRetType apoSupportedCRSList; if (psOtherSRS) { + if (pszDefaultSRS) { auto poSRS = std::unique_ptrbits[0]; + // coverity[overflow_const] if ((context->bits[0] = (t + (static_cast(len) << 3)) & 0xffffffff) < t) context->bits[1]++; /* Carry from low to high */ diff --git a/port/cpl_spawn.cpp b/port/cpl_spawn.cpp index 3fb75d7274ea..64b7a3bd1a38 100644 --- a/port/cpl_spawn.cpp +++ b/port/cpl_spawn.cpp @@ -474,7 +474,7 @@ int CPLPipeRead(CPL_FILE_HANDLE fin, void *data, int length) { while (true) { - const int n = static_cast(read(fin, pabyData, nRemain)); + const auto n = read(fin, pabyData, nRemain); if (n < 0) { if (errno == EINTR) @@ -485,7 +485,7 @@ int CPLPipeRead(CPL_FILE_HANDLE fin, void *data, int length) else if (n == 0) return FALSE; pabyData += n; - nRemain -= n; + nRemain -= static_cast(n); break; } } @@ -515,7 +515,7 @@ int CPLPipeWrite(CPL_FILE_HANDLE fout, const void *data, int length) { while (true) { - const int n = static_cast(write(fout, pabyData, nRemain)); + const auto n = write(fout, pabyData, nRemain); if (n < 0) { if (errno == EINTR) @@ -524,7 +524,7 @@ int CPLPipeWrite(CPL_FILE_HANDLE fout, const void *data, int length) return FALSE; } pabyData += n; - nRemain -= n; + nRemain -= static_cast(n); break; } } diff --git a/port/cpl_vsi_mem.cpp b/port/cpl_vsi_mem.cpp index caf3a5436c22..17f57ef9fa88 100644 --- a/port/cpl_vsi_mem.cpp +++ b/port/cpl_vsi_mem.cpp @@ -344,7 +344,11 @@ int VSIMemHandle::Close() int VSIMemHandle::Seek(vsi_l_offset nOffset, int nWhence) { - CPL_SHARED_LOCK oLock(poFile->m_oMutex); + vsi_l_offset nLength; + { + CPL_SHARED_LOCK oLock(poFile->m_oMutex); + nLength = poFile->nLength; + } bExtendFileAtNextWrite = false; if (nWhence == SEEK_CUR) @@ -361,7 +365,7 @@ int VSIMemHandle::Seek(vsi_l_offset nOffset, int nWhence) } else if (nWhence == SEEK_END) { - m_nOffset = poFile->nLength + nOffset; + m_nOffset = nLength + nOffset; } else { @@ -371,7 +375,7 @@ int VSIMemHandle::Seek(vsi_l_offset nOffset, int nWhence) bEOF = false; - if (m_nOffset > poFile->nLength) + if (m_nOffset > nLength) { if (bUpdate) // Writable files are zero-extended by seek past end. { @@ -399,13 +403,7 @@ vsi_l_offset VSIMemHandle::Tell() size_t VSIMemHandle::Read(void *pBuffer, size_t nSize, size_t nCount) { - CPL_SHARED_LOCK oLock(poFile->m_oMutex); - - if (!m_bReadAllowed) - { - m_bError = true; - return 0; - } + const vsi_l_offset nOffset = m_nOffset; size_t nBytesToRead = nSize * nCount; if (nBytesToRead == 0) @@ -417,21 +415,32 @@ size_t VSIMemHandle::Read(void *pBuffer, size_t nSize, size_t nCount) return 0; } - if (poFile->nLength <= m_nOffset || nBytesToRead + m_nOffset < nBytesToRead) + if (!m_bReadAllowed) { - bEOF = true; + m_bError = true; return 0; } - if (nBytesToRead + m_nOffset > poFile->nLength) + { - nBytesToRead = static_cast(poFile->nLength - m_nOffset); - nCount = nBytesToRead / nSize; - bEOF = true; + CPL_SHARED_LOCK oLock(poFile->m_oMutex); + + if (poFile->nLength <= nOffset || nBytesToRead + nOffset < nBytesToRead) + { + bEOF = true; + return 0; + } + if (nBytesToRead + nOffset > poFile->nLength) + { + nBytesToRead = static_cast(poFile->nLength - nOffset); + nCount = nBytesToRead / nSize; + bEOF = true; + } + + if (nBytesToRead) + memcpy(pBuffer, poFile->pabyData + nOffset, + static_cast(nBytesToRead)); } - if (nBytesToRead) - memcpy(pBuffer, poFile->pabyData + m_nOffset, - static_cast(nBytesToRead)); m_nOffset += nBytesToRead; return nCount; @@ -465,46 +474,50 @@ size_t VSIMemHandle::PRead(void *pBuffer, size_t nSize, size_t VSIMemHandle::Write(const void *pBuffer, size_t nSize, size_t nCount) { - CPL_EXCLUSIVE_LOCK oLock(poFile->m_oMutex); + const vsi_l_offset nOffset = m_nOffset; if (!bUpdate) { errno = EACCES; return 0; } + + const size_t nBytesToWrite = nSize * nCount; + if (bExtendFileAtNextWrite) { bExtendFileAtNextWrite = false; - if (!poFile->SetLength(m_nOffset)) + CPL_EXCLUSIVE_LOCK oLock(poFile->m_oMutex); + if (!poFile->SetLength(nOffset)) return 0; } - const size_t nBytesToWrite = nSize * nCount; - if (nCount > 0 && nBytesToWrite / nCount != nSize) - { - return 0; - } - if (nBytesToWrite + m_nOffset < nBytesToWrite) { - return 0; - } + CPL_EXCLUSIVE_LOCK oLock(poFile->m_oMutex); - if (nBytesToWrite + m_nOffset > poFile->nLength) - { - if (!poFile->SetLength(nBytesToWrite + m_nOffset)) + if (nCount > 0 && nBytesToWrite / nCount != nSize) + { return 0; + } + if (nBytesToWrite + nOffset < nBytesToWrite) + { + return 0; + } + + if (nBytesToWrite + nOffset > poFile->nLength) + { + if (!poFile->SetLength(nBytesToWrite + nOffset)) + return 0; + } + + if (nBytesToWrite) + memcpy(poFile->pabyData + nOffset, pBuffer, nBytesToWrite); + + time(&poFile->mTime); } - if (nBytesToWrite) - memcpy(poFile->pabyData + m_nOffset, pBuffer, nBytesToWrite); - // Coverity seems to be confused by the fact that we access m_nOffset - // under a shared lock in most places, except here under an exclusive lock - // which is fine - // coverity[missing_lock] m_nOffset += nBytesToWrite; - time(&poFile->mTime); - return nCount; } @@ -684,8 +697,12 @@ VSIVirtualHandle *VSIMemFilesystemHandler::Open(const char *pszFilename, #endif if (strchr(pszAccess, 'a')) { - CPL_SHARED_LOCK oLock(poFile->m_oMutex); - poHandle->m_nOffset = poFile->nLength; + vsi_l_offset nOffset; + { + CPL_SHARED_LOCK oLock(poFile->m_oMutex); + nOffset = poFile->nLength; + } + poHandle->m_nOffset = nOffset; } return poHandle; diff --git a/port/cpl_vsil_s3.cpp b/port/cpl_vsil_s3.cpp index 6be5177127e0..5225e45b9f1e 100644 --- a/port/cpl_vsil_s3.cpp +++ b/port/cpl_vsil_s3.cpp @@ -3814,6 +3814,7 @@ int IVSIS3LikeFSHandlerWithMultipartUpload::CopyFileRestartable( while (!bStop) { oCV.wait(oLock); + // coverity[ uninit_use_in_call] oLock.unlock(); const bool bInterrupt = !pProgressFunc(double(iCurChunk) / nChunkCount, diff --git a/port/cpl_vsisimple.cpp b/port/cpl_vsisimple.cpp index 744b540a08cd..a8d545ab2bb8 100644 --- a/port/cpl_vsisimple.cpp +++ b/port/cpl_vsisimple.cpp @@ -56,6 +56,7 @@ #include #include #include +#include #if HAVE_SYS_STAT_H #include #endif @@ -1401,8 +1402,11 @@ GIntBig CPLGetPhysicalRAM(void) { const long nPhysPages = sysconf(_SC_PHYS_PAGES); const long nPageSize = sysconf(_SC_PAGESIZE); - if (nPhysPages < 0 || nPageSize < 0) + if (nPhysPages <= 0 || nPageSize <= 0 || + nPhysPages > std::numeric_limits::max() / nPageSize) + { return 0; + } GIntBig nVal = static_cast(nPhysPages) * nPageSize; #ifdef __linux diff --git a/port/cpl_worker_thread_pool.cpp b/port/cpl_worker_thread_pool.cpp index 9087cf0270f7..444055a1879b 100644 --- a/port/cpl_worker_thread_pool.cpp +++ b/port/cpl_worker_thread_pool.cpp @@ -86,6 +86,16 @@ CPLWorkerThreadPool::~CPLWorkerThreadPool() CPLListDestroy(psWaitingWorkerThreadsList); } +/************************************************************************/ +/* GetThreadCount() */ +/************************************************************************/ + +int CPLWorkerThreadPool::GetThreadCount() const +{ + std::unique_lock oGuard(m_mutex); + return m_nMaxThreads; +} + /************************************************************************/ /* WorkerThreadFunction() */ /************************************************************************/ @@ -130,7 +140,12 @@ void CPLWorkerThreadPool::WorkerThreadFunction(void *user_data) */ bool CPLWorkerThreadPool::SubmitJob(CPLThreadFunc pfnFunc, void *pData) { - CPLAssert(m_nMaxThreads > 0); +#ifdef DEBUG + { + std::unique_lock oGuard(m_mutex); + CPLAssert(m_nMaxThreads > 0); + } +#endif bool bMustIncrementWaitingWorkerThreadsAfterSubmission = false; if (threadLocalCurrentThreadPool == this) @@ -234,6 +249,7 @@ bool CPLWorkerThreadPool::SubmitJob(CPLThreadFunc pfnFunc, void *pData) { std::lock_guard oGuardWT(psWorkerThread->m_mutex); + // coverity[ uninit_use_in_call] oGuard.unlock(); psWorkerThread->m_cv.notify_one(); } @@ -257,7 +273,12 @@ bool CPLWorkerThreadPool::SubmitJob(CPLThreadFunc pfnFunc, void *pData) bool CPLWorkerThreadPool::SubmitJobs(CPLThreadFunc pfnFunc, const std::vector &apData) { - CPLAssert(m_nMaxThreads > 0); +#ifdef DEBUG + { + std::unique_lock oGuard(m_mutex); + CPLAssert(m_nMaxThreads > 0); + } +#endif if (threadLocalCurrentThreadPool == this) { @@ -361,6 +382,7 @@ bool CPLWorkerThreadPool::SubmitJobs(CPLThreadFunc pfnFunc, #endif { std::lock_guard oGuardWT(psWorkerThread->m_mutex); + // coverity[ uninit_use_in_call] oGuard.unlock(); psWorkerThread->m_cv.notify_one(); } @@ -467,11 +489,14 @@ bool CPLWorkerThreadPool::Setup(int nThreads, CPLThreadFunc pfnInitFunc, bool bRet = true; for (int i = static_cast(aWT.size()); i < nThreads; i++) { - std::unique_ptr wt(new CPLWorkerThread); + auto wt = std::make_unique(); wt->pfnInitFunc = pfnInitFunc; wt->pInitData = pasInitData ? pasInitData[i] : nullptr; wt->poTP = this; - wt->bMarkedAsWaiting = false; + { + std::lock_guard oGuard(wt->m_mutex); + wt->bMarkedAsWaiting = false; + } wt->hThread = CPLCreateJoinableThread(WorkerThreadFunction, wt.get()); if (wt->hThread == nullptr) { @@ -575,8 +600,12 @@ CPLWorkerThreadPool::GetNextJob(CPLWorkerThread *psWorkerThread) #endif std::unique_lock oGuardThisThread(psWorkerThread->m_mutex); + // coverity[uninit_use_in_call] oGuard.unlock(); - psWorkerThread->m_cv.wait(oGuardThisThread); + while (psWorkerThread->bMarkedAsWaiting && eState != CPLWTS_STOP) + { + psWorkerThread->m_cv.wait(oGuardThisThread); + } } } diff --git a/port/cpl_worker_thread_pool.h b/port/cpl_worker_thread_pool.h index a6a0519220f2..77ecdcb2518c 100644 --- a/port/cpl_worker_thread_pool.h +++ b/port/cpl_worker_thread_pool.h @@ -80,7 +80,7 @@ class CPL_DLL CPLWorkerThreadPool CPL_DISALLOW_COPY_ASSIGN(CPLWorkerThreadPool) std::vector> aWT{}; - std::mutex m_mutex{}; + mutable std::mutex m_mutex{}; std::condition_variable m_cv{}; volatile CPLWorkerThreadState eState = CPLWTS_OK; CPLList *psJobQueue = nullptr; @@ -112,10 +112,7 @@ class CPL_DLL CPLWorkerThreadPool void WaitEvent(); /** Return the number of threads setup */ - int GetThreadCount() const - { - return m_nMaxThreads; - } + int GetThreadCount() const; }; /** Job queue */