Skip to content

Commit

Permalink
Merge pull request OSGeo#11056 from rouault/gpkg_memleak_fix
Browse files Browse the repository at this point in the history 
GPKG: fix memleaks in usage of GetSpatialRef(int, ...)
  • Loading branch information
@rouault
rouault authored Oct 22, 2024
2 parents f05241f + 3bcbf6a commit 0d02779
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 35 deletions.
5 changes: 3 additions & 2 deletions ogr/ogrsf_frmts/gpkg/ogr_geopackage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,8 +372,9 @@ class GDALGeoPackageDataset final : public OGRSQLiteBaseDataSource,

int GetSrsId(const OGRSpatialReference *poSRS);
const char *GetSrsName(const OGRSpatialReference &oSRS);
OGRSpatialReference *GetSpatialRef(int iSrsId, bool bFallbackToEPSG = false,
bool bEmitErrorIfNotFound = true);
std::unique_ptr<OGRSpatialReference, OGRSpatialReferenceReleaser>
GetSpatialRef(int iSrsId, bool bFallbackToEPSG = false,
bool bEmitErrorIfNotFound = true);
OGRErr CreateExtensionsTableIfNecessary();
bool HasExtensionsTable();

Expand Down
47 changes: 23 additions & 24 deletions ogr/ogrsf_frmts/gpkg/ogrgeopackagedatasource.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ static OGRErr GDALGPKGImportFromEPSG(OGRSpatialReference *poSpatialRef,
return eErr;
}

OGRSpatialReference *
std::unique_ptr<OGRSpatialReference, OGRSpatialReferenceReleaser>
GDALGeoPackageDataset::GetSpatialRef(int iSrsId, bool bFallbackToEPSG,
bool bEmitErrorIfNotFound)
{
Expand All @@ -245,7 +245,8 @@ GDALGeoPackageDataset::GetSpatialRef(int iSrsId, bool bFallbackToEPSG,
if (oIter->second == nullptr)
return nullptr;
oIter->second->Reference();
return oIter->second;
return std::unique_ptr<OGRSpatialReference,
OGRSpatialReferenceReleaser>(oIter->second);
}

if (iSrsId == 0 || iSrsId == -1)
Expand All @@ -268,7 +269,8 @@ GDALGeoPackageDataset::GetSpatialRef(int iSrsId, bool bFallbackToEPSG,

m_oMapSrsIdToSrs[iSrsId] = poSpatialRef;
poSpatialRef->Reference();
return poSpatialRef;
return std::unique_ptr<OGRSpatialReference,
OGRSpatialReferenceReleaser>(poSpatialRef);
}

CPLString oSQL;
Expand All @@ -292,7 +294,8 @@ GDALGeoPackageDataset::GetSpatialRef(int iSrsId, bool bFallbackToEPSG,
if (poSRS->importFromEPSG(iSrsId) == OGRERR_NONE)
{
poSRS->SetAxisMappingStrategy(OAMS_TRADITIONAL_GIS_ORDER);
return poSRS;
return std::unique_ptr<OGRSpatialReference,
OGRSpatialReferenceReleaser>(poSRS);
}
poSRS->Release();
}
Expand Down Expand Up @@ -349,7 +352,8 @@ GDALGeoPackageDataset::GetSpatialRef(int iSrsId, bool bFallbackToEPSG,
poSpatialRef->SetCoordinateEpoch(dfCoordinateEpoch);
m_oMapSrsIdToSrs[iSrsId] = poSpatialRef;
poSpatialRef->Reference();
return poSpatialRef;
return std::unique_ptr<OGRSpatialReference, OGRSpatialReferenceReleaser>(
poSpatialRef);
}

const char *GDALGeoPackageDataset::GetSrsName(const OGRSpatialReference &oSRS)
Expand Down Expand Up @@ -679,10 +683,8 @@ int GDALGeoPackageDataset::GetSrsId(const OGRSpatialReference *poSRSIn)
auto poRefSRS = GetSpatialRef(nSRSId);
bool bOK =
(poRefSRS == nullptr ||
poSRS->IsSame(poRefSRS, apszIsSameOptions) ||
poSRS->IsSame(poRefSRS.get(), apszIsSameOptions) ||
!CPLTestBool(CPLGetConfigOption("OGR_GPKG_CHECK_SRS", "YES")));
if (poRefSRS)
poRefSRS->Release();
if (bOK)
{
return nSRSId;
Expand Down Expand Up @@ -2858,11 +2860,9 @@ bool GDALGeoPackageDataset::OpenRaster(
m_bRecordInsertedInGPKGContent = true;
m_nSRID = nSRSId;

OGRSpatialReference *poSRS = GetSpatialRef(nSRSId);
if (poSRS)
if (auto poSRS = GetSpatialRef(nSRSId))
{
m_oSRS = *poSRS;
poSRS->Release();
m_oSRS = *(poSRS.get());
}

/* Various sanity checks added in the SELECT */
Expand Down Expand Up @@ -8704,7 +8704,8 @@ static void OGRGeoPackageGeodesicArea(sqlite3_context *pContext, int argc,
GDALGeoPackageDataset *poDS =
static_cast<GDALGeoPackageDataset *>(sqlite3_user_data(pContext));

OGRSpatialReference *poSrcSRS = poDS->GetSpatialRef(sHeader.iSrsId, true);
std::unique_ptr<OGRSpatialReference, OGRSpatialReferenceReleaser> poSrcSRS(
poDS->GetSpatialRef(sHeader.iSrsId, true));
if (poSrcSRS == nullptr)
{
CPLError(CE_Failure, CPLE_AppDefined,
Expand All @@ -8729,7 +8730,7 @@ static void OGRGeoPackageGeodesicArea(sqlite3_context *pContext, int argc,
poGeom.reset(poGeomSpatialite);
}

poGeom->assignSpatialReference(poSrcSRS);
poGeom->assignSpatialReference(poSrcSRS.get());
sqlite3_result_double(
pContext, OGR_G_GeodesicArea(OGRGeometry::ToHandle(poGeom.get())));
}
Expand Down Expand Up @@ -8767,7 +8768,7 @@ static void OGRGeoPackageLengthOrGeodesicLength(sqlite3_context *pContext,
GDALGeoPackageDataset *poDS =
static_cast<GDALGeoPackageDataset *>(sqlite3_user_data(pContext));

OGRSpatialReference *poSrcSRS = nullptr;
std::unique_ptr<OGRSpatialReference, OGRSpatialReferenceReleaser> poSrcSRS;
if (argc == 2)
{
poSrcSRS = poDS->GetSpatialRef(sHeader.iSrsId, true);
Expand Down Expand Up @@ -8797,7 +8798,7 @@ static void OGRGeoPackageLengthOrGeodesicLength(sqlite3_context *pContext,
}

if (argc == 2)
poGeom->assignSpatialReference(poSrcSRS);
poGeom->assignSpatialReference(poSrcSRS.get());

sqlite3_result_double(
pContext,
Expand Down Expand Up @@ -8853,8 +8854,8 @@ void OGRGeoPackageTransform(sqlite3_context *pContext, int argc,
}
else
{
OGRSpatialReference *poSrcSRS =
poDS->GetSpatialRef(sHeader.iSrsId, true);
std::unique_ptr<OGRSpatialReference, OGRSpatialReferenceReleaser>
poSrcSRS(poDS->GetSpatialRef(sHeader.iSrsId, true));
if (poSrcSRS == nullptr)
{
CPLError(CE_Failure, CPLE_AppDefined,
Expand All @@ -8863,19 +8864,17 @@ void OGRGeoPackageTransform(sqlite3_context *pContext, int argc,
return;
}

OGRSpatialReference *poDstSRS = poDS->GetSpatialRef(nDestSRID, true);
std::unique_ptr<OGRSpatialReference, OGRSpatialReferenceReleaser>
poDstSRS(poDS->GetSpatialRef(nDestSRID, true));
if (poDstSRS == nullptr)
{
CPLError(CE_Failure, CPLE_AppDefined, "Target SRID (%d) is invalid",
nDestSRID);
sqlite3_result_blob(pContext, nullptr, 0, nullptr);
poSrcSRS->Release();
return;
}
poCT = OGRCreateCoordinateTransformation(poSrcSRS, poDstSRS);
poSrcSRS->Release();
poDstSRS->Release();

poCT =
OGRCreateCoordinateTransformation(poSrcSRS.get(), poDstSRS.get());
if (poCT == nullptr)
{
sqlite3_result_blob(pContext, nullptr, 0, nullptr);
Expand Down
6 changes: 2 additions & 4 deletions ogr/ogrsf_frmts/gpkg/ogrgeopackagelayer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1144,12 +1144,10 @@ void OGRGeoPackageLayer::BuildFeatureDefn(const char *pszLayerName,
wkbUnknown);

/* Read the SRS */
OGRSpatialReference *poSRS =
m_poDS->GetSpatialRef(nSRID, true);
auto poSRS = m_poDS->GetSpatialRef(nSRID, true);
if (poSRS)
{
oGeomField.SetSpatialRef(poSRS);
poSRS->Dereference();
oGeomField.SetSpatialRef(poSRS.get());
}

OGRwkbGeometryType eGeomType = poGeom->getGeometryType();
Expand Down
8 changes: 3 additions & 5 deletions ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1202,12 +1202,11 @@ OGRErr OGRGeoPackageTableLayer::ReadTableDefinition()
FALSE);

/* Read the SRS */
OGRSpatialReference *poSRS = m_poDS->GetSpatialRef(m_iSrs);
auto poSRS = m_poDS->GetSpatialRef(m_iSrs);
if (poSRS)
{
m_poFeatureDefn->GetGeomFieldDefn(0)->SetSpatialRef(
poSRS);
poSRS->Dereference();
poSRS.get());
}
}
else if (!STARTS_WITH(
Expand Down Expand Up @@ -5660,8 +5659,7 @@ void OGRGeoPackageTableLayer::SetCreationParameters(
/* bEmitErrorIfNotFound = */ false);
if (poGotSRS)
{
oGeomFieldDefn.SetSpatialRef(poGotSRS);
poGotSRS->Release();
oGeomFieldDefn.SetSpatialRef(poGotSRS.get());
}
else
{
Expand Down

0 comments on commit 0d02779

Please sign in to comment.