[geos-commits] [SCM] GEOS branch master updated. 5f92026c20de11c4c0f46c5feb9665523c2922cf
git at osgeo.org
git at osgeo.org
Tue Nov 20 15:54:37 PST 2018
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GEOS".
The branch, master has been updated
via 5f92026c20de11c4c0f46c5feb9665523c2922cf (commit)
via fe5f16c8f00dad2709a6dd0306b5831b686d905c (commit)
from 81ff4c0359100dcdb8c89109e65f0c265b451ca6 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 5f92026c20de11c4c0f46c5feb9665523c2922cf
Merge: 81ff4c0 fe5f16c
Author: Daniel Baston <dbaston at gmail.com>
Date: Tue Nov 20 18:54:24 2018 -0500
Merge branch 'trac-912'
commit fe5f16c8f00dad2709a6dd0306b5831b686d905c
Author: Daniel Baston <dbaston at gmail.com>
Date: Mon Nov 19 20:17:42 2018 -0500
Resolve segfault in BinTree
Take things a bit farther than needed to strictly fix the bug, by using
std::unique_ptr to clearly define the ownership of objects returned by
MonotoneChainBuilder.
diff --git a/include/geos/algorithm/MCPointInRing.h b/include/geos/algorithm/MCPointInRing.h
index 872cf44..61ebd59 100644
--- a/include/geos/algorithm/MCPointInRing.h
+++ b/include/geos/algorithm/MCPointInRing.h
@@ -22,6 +22,7 @@
#include <geos/geom/Coordinate.h> // for composition
#include <geos/index/bintree/Interval.h> // for composition
+#include <memory>
#include <vector>
// Forward declarations
@@ -38,9 +39,6 @@ namespace geos {
class Bintree;
class Interval;
}
- namespace chain {
- class MonotoneChain;
- }
}
}
@@ -69,8 +67,9 @@ public:
private:
const geom::LinearRing *ring;
index::bintree::Interval interval;
- geom::CoordinateSequence *pts;
- index::bintree::Bintree *tree;
+ std::unique_ptr<geom::CoordinateSequence> pts;
+ std::unique_ptr<index::bintree::Bintree> tree;
+ std::unique_ptr<std::vector<std::unique_ptr<index::chain::MonotoneChain>>> chains;
int crossings; // number of segment/ray crossings
void buildIndex();
void testMonotoneChain(geom::Envelope *rayEnv,
diff --git a/include/geos/index/chain/MonotoneChainBuilder.h b/include/geos/index/chain/MonotoneChainBuilder.h
index 6e5ff52..8ac9060 100644
--- a/include/geos/index/chain/MonotoneChainBuilder.h
+++ b/include/geos/index/chain/MonotoneChainBuilder.h
@@ -20,6 +20,7 @@
#define GEOS_IDX_CHAIN_MONOTONECHAINBUILDER_H
#include <geos/export.h>
+#include <memory>
#include <vector>
#include <cstddef>
@@ -56,7 +57,7 @@ public:
* MonotoneChain objects for the given CoordinateSequence.
* Remember to deep-delete the result.
*/
- static std::vector<MonotoneChain*>* getChains(
+ static std::unique_ptr<std::vector<std::unique_ptr<MonotoneChain>>> getChains(
const geom::CoordinateSequence *pts,
void* context);
@@ -67,9 +68,9 @@ public:
*/
static void getChains(const geom::CoordinateSequence *pts,
void* context,
- std::vector<MonotoneChain*>& mcList);
+ std::vector<std::unique_ptr<MonotoneChain>> & mcList);
- static std::vector<MonotoneChain*>* getChains(const geom::CoordinateSequence *pts)
+ static std::unique_ptr<std::vector<std::unique_ptr<MonotoneChain>>> getChains(const geom::CoordinateSequence *pts)
{
return getChains(pts, nullptr);
}
diff --git a/include/geos/noding/MCIndexSegmentSetMutualIntersector.h b/include/geos/noding/MCIndexSegmentSetMutualIntersector.h
index 0b36d27..05a0c96 100644
--- a/include/geos/noding/MCIndexSegmentSetMutualIntersector.h
+++ b/include/geos/noding/MCIndexSegmentSetMutualIntersector.h
@@ -59,15 +59,6 @@ public:
~MCIndexSegmentSetMutualIntersector() override;
- /* Returns a reference to a vector of MonotoneChain objects owned
- * by this class and destroyed on next call to ::process.
- * Copy them if you need them alive for longer.
- */
- std::vector<index::chain::MonotoneChain *>& getMonotoneChains()
- {
- return monoChains;
- }
-
index::SpatialIndex* getIndex()
{
return index;
@@ -98,7 +89,7 @@ public:
private:
- typedef std::vector<index::chain::MonotoneChain *> MonoChains;
+ typedef std::vector<std::unique_ptr<index::chain::MonotoneChain>> MonoChains;
MonoChains monoChains;
/*
diff --git a/src/algorithm/MCPointInRing.cpp b/src/algorithm/MCPointInRing.cpp
index 74904d8..f098575 100644
--- a/src/algorithm/MCPointInRing.cpp
+++ b/src/algorithm/MCPointInRing.cpp
@@ -60,44 +60,23 @@ MCPointInRing::MCPointInRing(const LinearRing *newRing)
buildIndex();
}
-MCPointInRing::~MCPointInRing()
-{
- delete tree;
- delete pts;
-}
+MCPointInRing::~MCPointInRing() = default;
void
MCPointInRing::buildIndex()
{
- //using namespace geos::index;
-
-// Envelope *env=ring->getEnvelopeInternal();
- tree=new bintree::Bintree();
- pts=CoordinateSequence::removeRepeatedPoints(ring->getCoordinatesRO());
+ tree.reset(new bintree::Bintree());
+ pts.reset(CoordinateSequence::removeRepeatedPoints(ring->getCoordinatesRO()));
+ chains = chain::MonotoneChainBuilder::getChains(pts.get());
- // NOTE: we take ownership of mcList and it's elements
- vector<chain::MonotoneChain*> *mcList =
- chain::MonotoneChainBuilder::getChains(pts);
-
- for(size_t i=0, n=mcList->size(); i<n; ++i)
+ for(const auto& mc : *chains)
{
- chain::MonotoneChain *mc=(*mcList)[i];
const Envelope& mcEnv = mc->getEnvelope();
interval.min = mcEnv.getMinY();
interval.max = mcEnv.getMaxY();
- // TODO: is 'mc' ownership transferred here ? (see below)
- // by documentation SpatialIndex does NOT take
- // ownership of the items, so unless we query it
- // all later we've a leak problem here..
- // Need a focused testcase.
- //
- tree->insert(&interval, mc);
+ tree->insert(&interval, mc.get());
}
-
- // TODO: mcList elements ownership went to tree or what ?
-
- delete mcList;
}
bool
diff --git a/src/index/bintree/NodeBase.cpp b/src/index/bintree/NodeBase.cpp
index ee89d93..b801b43 100644
--- a/src/index/bintree/NodeBase.cpp
+++ b/src/index/bintree/NodeBase.cpp
@@ -16,14 +16,11 @@
#include <geos/index/bintree/NodeBase.h>
#include <geos/index/bintree/Interval.h>
#include <geos/index/bintree/Node.h>
-#include <geos/index/chain/MonotoneChain.h> // FIXME: split
#include <vector>
using namespace std;
-using namespace geos::index::chain;
-
namespace geos {
namespace index { // geos.index
namespace bintree { // geos.index.bintree
@@ -49,9 +46,6 @@ NodeBase::NodeBase()
}
NodeBase::~NodeBase() {
- for(int i=0;i<(int)items->size();i++) {
- delete (MonotoneChain*)(*items)[i];
- }
delete items;
delete subnode[0];
delete subnode[1];
diff --git a/src/index/chain/MonotoneChainBuilder.cpp b/src/index/chain/MonotoneChainBuilder.cpp
index 81b5c52..2e30a63 100644
--- a/src/index/chain/MonotoneChainBuilder.cpp
+++ b/src/index/chain/MonotoneChainBuilder.cpp
@@ -42,10 +42,11 @@ namespace index { // geos.index
namespace chain { // geos.index.chain
/* static public */
-vector<MonotoneChain*>*
+std::unique_ptr<std::vector<std::unique_ptr<MonotoneChain>>>
MonotoneChainBuilder::getChains(const CoordinateSequence* pts, void* context)
{
- vector<MonotoneChain*>* mcList = new vector<MonotoneChain*>();
+ // TODO clean this up with std::make_unique (C++14)
+ std::unique_ptr<std::vector<std::unique_ptr<MonotoneChain>>> mcList{new vector<std::unique_ptr<MonotoneChain>>()};
getChains(pts, context, *mcList);
return mcList;
}
@@ -53,7 +54,7 @@ MonotoneChainBuilder::getChains(const CoordinateSequence* pts, void* context)
/* static public */
void
MonotoneChainBuilder::getChains(const CoordinateSequence* pts, void* context,
- vector<MonotoneChain*>& mcList)
+ vector<std::unique_ptr<MonotoneChain>>& mcList)
{
vector<std::size_t> startIndex;
getChainStartIndices(*pts, startIndex);
@@ -63,8 +64,7 @@ MonotoneChainBuilder::getChains(const CoordinateSequence* pts, void* context,
std::size_t n = nindexes - 1;
for(std::size_t i = 0; i < n; i++)
{
- MonotoneChain* mc = new MonotoneChain(*pts, startIndex[i], startIndex[i+1], context);
- mcList.push_back(mc);
+ mcList.emplace_back(new MonotoneChain(*pts, startIndex[i], startIndex[i+1], context));
}
}
}
diff --git a/src/noding/MCIndexNoder.cpp b/src/noding/MCIndexNoder.cpp
index aefae13..aa79c65 100644
--- a/src/noding/MCIndexNoder.cpp
+++ b/src/noding/MCIndexNoder.cpp
@@ -105,24 +105,21 @@ MCIndexNoder::intersectChains()
void
MCIndexNoder::add(SegmentString* segStr)
{
- vector<MonotoneChain*> segChains;
+ vector<std::unique_ptr<MonotoneChain>> segChains;
// segChains will contain nelwy allocated MonotoneChain objects
MonotoneChainBuilder::getChains(segStr->getCoordinates(),
segStr, segChains);
- for(vector<MonotoneChain*>::iterator
- it=segChains.begin(), iEnd=segChains.end();
- it!=iEnd; ++it)
+ for(auto& mc : segChains)
{
- MonotoneChain* mc = *it;
assert(mc);
mc->setId(idCounter++);
- index.insert(&(mc->getEnvelope()), mc);
+ index.insert(&(mc->getEnvelope()), mc.get());
// MonotoneChain objects deletion delegated to destructor
- monoChains.push_back(mc);
+ monoChains.push_back(mc.release());
}
}
diff --git a/src/noding/MCIndexSegmentSetMutualIntersector.cpp b/src/noding/MCIndexSegmentSetMutualIntersector.cpp
index e837b5c..f35718a 100644
--- a/src/noding/MCIndexSegmentSetMutualIntersector.cpp
+++ b/src/noding/MCIndexSegmentSetMutualIntersector.cpp
@@ -43,12 +43,11 @@ MCIndexSegmentSetMutualIntersector::addToIndex(SegmentString* segStr)
MonoChains::size_type n = segChains.size();
chainStore.reserve(chainStore.size() + n);
- for (MonoChains::size_type i = 0; i < n; i++)
+ for (auto& mc : segChains)
{
- MonotoneChain * mc = segChains[i];
mc->setId(indexCounter++);
- index->insert(&(mc->getEnvelope()), mc);
- chainStore.push_back(mc);
+ index->insert(&(mc->getEnvelope()), mc.get());
+ chainStore.push_back(std::move(mc));
}
}
@@ -59,10 +58,8 @@ MCIndexSegmentSetMutualIntersector::intersectChains()
{
MCIndexSegmentSetMutualIntersector::SegmentOverlapAction overlapAction( *segInt);
- for (MonoChains::size_type i = 0, ni = monoChains.size(); i < ni; ++i)
+ for (const auto& queryChain : monoChains)
{
- MonotoneChain * queryChain = (MonotoneChain *)monoChains[i];
-
std::vector<void*> overlapChains;
index->query( &(queryChain->getEnvelope()), overlapChains);
@@ -88,11 +85,10 @@ MCIndexSegmentSetMutualIntersector::addToMonoChains(SegmentString* segStr)
MonoChains::size_type n = segChains.size();
monoChains.reserve(monoChains.size() + n);
- for (MonoChains::size_type i = 0; i < n; i++)
+ for (auto& mc : segChains)
{
- MonotoneChain* mc = segChains[i];
mc->setId( processCounter++ );
- monoChains.push_back(mc);
+ monoChains.push_back(std::move(mc));
}
}
@@ -112,14 +108,6 @@ MCIndexSegmentSetMutualIntersector::~MCIndexSegmentSetMutualIntersector()
delete index;
MonoChains::iterator i, e;
-
- for (i = chainStore.begin(), e = chainStore.end(); i != e; ++i) {
- delete *i;
- }
-
- for (i = monoChains.begin(), e = monoChains.end(); i != e; i++) {
- delete *i;
- }
}
/* public */
@@ -143,11 +131,6 @@ MCIndexSegmentSetMutualIntersector::process(SegmentString::ConstVect * segString
processCounter = indexCounter + 1;
nOverlaps = 0;
- for (MonoChains::iterator i = monoChains.begin(), e = monoChains.end();
- i != e; i++)
- {
- delete *i;
- }
monoChains.clear();
for (SegmentString::ConstVect::size_type i = 0, n = segStrings->size(); i < n; i++)
-----------------------------------------------------------------------
Summary of changes:
include/geos/algorithm/MCPointInRing.h | 9 +++---
include/geos/index/chain/MonotoneChainBuilder.h | 7 +++--
.../noding/MCIndexSegmentSetMutualIntersector.h | 11 +-------
src/algorithm/MCPointInRing.cpp | 33 ++++------------------
src/index/bintree/NodeBase.cpp | 6 ----
src/index/chain/MonotoneChainBuilder.cpp | 10 +++----
src/noding/MCIndexNoder.cpp | 11 +++-----
src/noding/MCIndexSegmentSetMutualIntersector.cpp | 29 ++++---------------
8 files changed, 30 insertions(+), 86 deletions(-)
hooks/post-receive
--
GEOS
More information about the geos-commits
mailing list