Merge pull request OSGeo#10285 from rouault/coverity_fixes

Fix number of new Coverity Scan warnings, likely due to new version of the tool
OSGeo-jp · Jul 1, 2024 · e179609 · e179609
2 parents 3132992 + 62fc992
commit e179609
Show file tree

Hide file tree

Showing 46 changed files with 374 additions and 205 deletions.
diff --git 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
@@ -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<int>::max();
-        if (nYChunkSize64 > knIntMax)
-            nYChunkSize = knIntMax;
-        else
-            nYChunkSize = static_cast<int>(nYChunkSize64);
+        nYChunkSize = static_cast<int>(
+            std::min<GIntBig>(nYChunkSize64, std::numeric_limits<int>::max()));
     }
 
     if (nYChunkSize < 1)

diff --git 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<std::mutex> lock(psThreadData->mutex);
 
@@ -550,14 +551,16 @@ static CPLErr GWKRun(GDALWarpKernel *poWK, const char *pszFuncName,
                 }
             }
         }
+
+        bStopFlag = psThreadData->stopFlag;
     }
 
     /* -------------------------------------------------------------------- */
     /*      Wait for all jobs to complete.                                  */
     /* -------------------------------------------------------------------- */
     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
@@ -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
@@ -2041,8 +2041,10 @@ class ArgumentParser {
         }
 
         stream << std::setw(2) << " ";
-        stream << std::setw(static_cast<int>(longest_arg_length - 2))
-               << command;
+        if (longest_arg_length >= 2) {
+          stream << std::setw(static_cast<int>(longest_arg_length - 2))
+                 << command;
+        }
         stream << " " << subparser->get().m_description << "\n";
       }
     }

diff --git 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
@@ -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<GByte *>(pBuffer);
             EXPECT_EQ(pabyBuffer[0], 0xFF);

diff --git 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
@@ -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,15 +724,17 @@ CPLErr GSBGDataset::WriteHeader(VSILFILE *fp, GInt16 nXSize, GInt16 nYSize,
         return CE_Failure;
     }
 
-    GInt16 nTemp = CPL_LSBWORD16(nXSize);
+    assert(nXSize >= 0 && nXSize <= std::numeric_limits<int16_t>::max());
+    GInt16 nTemp = CPL_LSBWORD16(static_cast<int16_t>(nXSize));
     if (VSIFWriteL((void *)&nTemp, 2, 1, fp) != 1)
     {
         CPLError(CE_Failure, CPLE_FileIO,
                  "Unable to write raster X size to grid file.\n");
         return CE_Failure;
     }
 
-    nTemp = CPL_LSBWORD16(nYSize);
+    assert(nYSize >= 0 && nYSize <= std::numeric_limits<int16_t>::max());
+    nTemp = CPL_LSBWORD16(static_cast<int16_t>(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<double>::max();
     double dfMaxZ = std::numeric_limits<double>::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
@@ -188,8 +188,11 @@ toff_t GTIFFWriteDirectory(TIFF *hTIFF, int nSubfileType, int nXSize,
     }
 
     TIFFWriteDirectory(hTIFF);
-    TIFFSetDirectory(hTIFF,
-                     static_cast<tdir_t>(TIFFNumberOfDirectories(hTIFF) - 1));
+    const tdir_t nNumberOfDirs = TIFFNumberOfDirectories(hTIFF);
+    if (nNumberOfDirs > 0)  // always true, but to please Coverity
+    {
+        TIFFSetDirectory(hTIFF, static_cast<tdir_t>(nNumberOfDirs - 1));
+    }
 
     const toff_t nOffset = TIFFCurrentDirOffset(hTIFF);
 

diff --git 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<GByte *>(pJPEGTable)[nJPEGTableSize - 1] != 0xD9)
         {
             m_nJPEGOverviewCount = 0;

diff --git 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<tdir_t>(TIFFNumberOfDirectories(m_hTIFF) - 1));
+        const tdir_t nNumberOfDirs = TIFFNumberOfDirectories(m_hTIFF);
+        if (nNumberOfDirs > 0)
+        {
+            TIFFSetDirectory(m_hTIFF, static_cast<tdir_t>(nNumberOfDirs - 1));
+        }
     }
 
     RestoreVolatileParameters(m_hTIFF);

diff --git a/frmts/gtiff/gtiffrasterband_read.cpp b/frmts/gtiff/gtiffrasterband_read.cpp
@@ -32,6 +32,7 @@
 #include "gtiffjpegoverviewds.h"
 
 #include <algorithm>
+#include <cassert>
 #include <limits>
 #include <map>
 #include <set>
@@ -509,6 +510,9 @@ CPLVirtualMem *GTiffRasterBand::GetVirtualMemAutoInternal(GDALRWFlag eRWFlag,
             CPLAssert(panByteCounts[0] == static_cast<toff_t>(nBlockSize));
 
             // Now simulate the writing of other blocks.
+            assert(nBlocks > 0);
+            assert(static_cast<vsi_l_offset>(nBlockSize) <
+                   std::numeric_limits<vsi_l_offset>::max() / nBlocks);
             const vsi_l_offset nDataSize =
                 static_cast<vsi_l_offset>(nBlockSize) * nBlocks;
             if (VSIFTruncateL(fp, nBaseOffset + nDataSize) != 0)

diff --git a/frmts/mrf/marfa.h b/frmts/mrf/marfa.h
@@ -965,11 +965,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);