Skip to content

Commit

Permalink
Merge pull request #10379 from rouault/coverity_fixes
Browse files Browse the repository at this point in the history
Coverity Scan fixes
  • Loading branch information
rouault authored Jul 8, 2024
2 parents 22caab4 + f1fd72c commit 40bdb97
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 33 deletions.
1 change: 1 addition & 0 deletions alg/gdalchecksum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -368,5 +368,6 @@ int CPL_STDCALL GDALChecksumImage(GDALRasterBandH hBand, int nXOff, int nYOff,
CPLFree(panLineData);
}

// coverity[return_overflow]
return nChecksum;
}
4 changes: 2 additions & 2 deletions gcore/gdalmultidim_gltorthorectification.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,14 +129,14 @@ class GLTOrthoRectifiedArray final : public GDALPamMDArray
{/*latIdx = */ 1, /* lonIdx = */ 2});
if (CPLTestBool(CSLFetchNameValueDef(papszOptions,
"USE_GOOD_WAVELENGTHS", "YES")) &&
poParent->GetDimensionCount() == 3)
newAr->m_poParent->GetDimensionCount() == 3)
{
const auto poGoodWaveLengths = poRootGroup->OpenMDArrayFromFullname(
"/sensor_band_parameters/good_wavelengths");
if (poGoodWaveLengths &&
poGoodWaveLengths->GetDimensionCount() == 1 &&
poGoodWaveLengths->GetDimensions()[0]->GetSize() ==
poParent->GetDimensions()[2]->GetSize() &&
newAr->m_poParent->GetDimensions()[2]->GetSize() &&
poGoodWaveLengths->GetDimensions()[0]->GetSize() <
1000 * 1000 &&
poGoodWaveLengths->GetDataType().GetClass() == GEDTC_NUMERIC)
Expand Down
3 changes: 2 additions & 1 deletion ogr/ogrsf_frmts/arrow_common/ograrrowlayer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2385,8 +2385,9 @@ 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 - 1)
if (out_length >= 0 && out_length <= INT_MAX - 1)
{
// coverity[overflow_sink]
poFeature->SetField(i, static_cast<int>(out_length), data);
}
else
Expand Down
55 changes: 29 additions & 26 deletions ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7771,33 +7771,36 @@ void OGR_GPKG_FillArrowArray_Step(sqlite3_context *pContext, int /*argc*/,
auto psFillArrowArray = static_cast<OGRGPKGTableLayerFillArrowArray *>(
sqlite3_user_data(pContext));

if (psFillArrowArray->nCountRows >=
psFillArrowArray->psHelper->m_nMaxBatchSize)
{
if (psFillArrowArray->bAsynchronousMode)
std::unique_lock<std::mutex> oLock(psFillArrowArray->oMutex);
if (psFillArrowArray->nCountRows >=
psFillArrowArray->psHelper->m_nMaxBatchSize)
{
std::unique_lock<std::mutex> oLock(psFillArrowArray->oMutex);
psFillArrowArray->psHelper->Shrink(psFillArrowArray->nCountRows);
psFillArrowArray->oCV.notify_one();
while (psFillArrowArray->nCountRows > 0)
if (psFillArrowArray->bAsynchronousMode)
{
psFillArrowArray->oCV.wait(oLock);
psFillArrowArray->psHelper->Shrink(
psFillArrowArray->nCountRows);
psFillArrowArray->oCV.notify_one();
while (psFillArrowArray->nCountRows > 0)
{
psFillArrowArray->oCV.wait(oLock);
}
// Note that psFillArrowArray->psHelper.get() will generally now be
// different from before the wait()
}
else
{
// should not happen !
psFillArrowArray->osErrorMsg = "OGR_GPKG_FillArrowArray_Step() "
"got more rows than expected!";
sqlite3_interrupt(psFillArrowArray->hDB);
psFillArrowArray->bErrorOccurred = true;
return;
}
// Note that psFillArrowArray->psHelper.get() will generally now be
// different from before the wait()
}
else
{
// should not happen !
psFillArrowArray->osErrorMsg =
"OGR_GPKG_FillArrowArray_Step() got more rows than expected!";
sqlite3_interrupt(psFillArrowArray->hDB);
psFillArrowArray->bErrorOccurred = true;
if (psFillArrowArray->nCountRows < 0)
return;
}
}
if (psFillArrowArray->nCountRows < 0)
return;

if (psFillArrowArray->nMemLimit == 0)
psFillArrowArray->nMemLimit = OGRArrowArrayHelper::GetMemLimit();
Expand Down Expand Up @@ -7943,7 +7946,7 @@ void OGR_GPKG_FillArrowArray_Step(sqlite3_context *pContext, int /*argc*/,
}
}

if (psFillArrowArray->nCountRows > 0)
if (iFeat > 0)
{
auto panOffsets = static_cast<int32_t *>(
const_cast<void *>(psArray->buffers[1]));
Expand All @@ -7956,7 +7959,7 @@ void OGR_GPKG_FillArrowArray_Step(sqlite3_context *pContext, int /*argc*/,
"OGR_GPKG_FillArrowArray_Step(): premature "
"notification of %d features to consumer due "
"to too big array",
psFillArrowArray->nCountRows);
iFeat);
psFillArrowArray->bMemoryLimitReached = true;
if (psFillArrowArray->bAsynchronousMode)
{
Expand Down Expand Up @@ -8081,7 +8084,7 @@ void OGR_GPKG_FillArrowArray_Step(sqlite3_context *pContext, int /*argc*/,
const void *pabyData = sqlite3_value_blob(argv[iCol]);
if (pabyData != nullptr || nBytes == 0)
{
if (psFillArrowArray->nCountRows > 0)
if (iFeat > 0)
{
auto panOffsets = static_cast<int32_t *>(
const_cast<void *>(psArray->buffers[1]));
Expand All @@ -8094,7 +8097,7 @@ void OGR_GPKG_FillArrowArray_Step(sqlite3_context *pContext, int /*argc*/,
"OGR_GPKG_FillArrowArray_Step(): "
"premature notification of %d features to "
"consumer due to too big array",
psFillArrowArray->nCountRows);
iFeat);
psFillArrowArray->bMemoryLimitReached = true;
if (psFillArrowArray->bAsynchronousMode)
{
Expand Down Expand Up @@ -8171,7 +8174,7 @@ void OGR_GPKG_FillArrowArray_Step(sqlite3_context *pContext, int /*argc*/,
if (pszTxt != nullptr)
{
const size_t nBytes = strlen(pszTxt);
if (psFillArrowArray->nCountRows > 0)
if (iFeat > 0)
{
auto panOffsets = static_cast<int32_t *>(
const_cast<void *>(psArray->buffers[1]));
Expand All @@ -8184,7 +8187,7 @@ void OGR_GPKG_FillArrowArray_Step(sqlite3_context *pContext, int /*argc*/,
"OGR_GPKG_FillArrowArray_Step(): "
"premature notification of %d features to "
"consumer due to too big array",
psFillArrowArray->nCountRows);
iFeat);
psFillArrowArray->bMemoryLimitReached = true;
if (psFillArrowArray->bAsynchronousMode)
{
Expand Down
2 changes: 2 additions & 0 deletions ogr/ogrsf_frmts/shape/ogrshapedatasource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1500,8 +1500,10 @@ bool OGRShapeDataSource::UncompressIfNeeded()
return false;
}
m_psLockFile = f;
CPLAcquireMutex(m_poRefreshLockFileMutex, 1000);
m_bExitRefreshLockFileThread = false;
m_bRefreshLockFileThreadStarted = false;
CPLReleaseMutex(m_poRefreshLockFileMutex);
// Config option mostly for testing purposes
// coverity[tainted_data]
m_dfRefreshLockDelay = CPLAtof(CPLGetConfigOption(
Expand Down
19 changes: 15 additions & 4 deletions port/cpl_vsi_mem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,25 +421,36 @@ size_t VSIMemHandle::Read(void *pBuffer, size_t nSize, size_t nCount)
return 0;
}

bool bEOFTmp = bEOF;
// Do not access/modify bEOF under the lock to avoid confusing Coverity
// Scan since we access it in other methods outside of the lock.
const auto DoUnderLock =
[this, nOffset, pBuffer, nSize, &nBytesToRead, &nCount, &bEOFTmp]
{
CPL_SHARED_LOCK oLock(poFile->m_oMutex);

if (poFile->nLength <= nOffset || nBytesToRead + nOffset < nBytesToRead)
{
bEOF = true;
return 0;
bEOFTmp = true;
return false;
}
if (nBytesToRead + nOffset > poFile->nLength)
{
nBytesToRead = static_cast<size_t>(poFile->nLength - nOffset);
nCount = nBytesToRead / nSize;
bEOF = true;
bEOFTmp = true;
}

if (nBytesToRead)
memcpy(pBuffer, poFile->pabyData + nOffset,
static_cast<size_t>(nBytesToRead));
}
return true;
};

bool bRet = DoUnderLock();
bEOF = bEOFTmp;
if (!bRet)
return 0;

m_nOffset += nBytesToRead;

Expand Down

0 comments on commit 40bdb97

Please sign in to comment.