[geos-commits] [SCM] GEOS branch master updated. 50f558821f63ef7f8dd41b4ef3bdb581763936d6

git at osgeo.org git at osgeo.org
Mon Aug 26 11:46:09 PDT 2019


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  50f558821f63ef7f8dd41b4ef3bdb581763936d6 (commit)
       via  0bf5167794c90984a056d7b5b6a7f5cb4ecaedc9 (commit)
       via  c487768ede97ea3d4ab78e16f67bb37f9dd3c09d (commit)
      from  2cd5f5be1685ee46f444308fa63b41becf967b16 (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 50f558821f63ef7f8dd41b4ef3bdb581763936d6
Merge: 2cd5f5b 0bf5167
Author: Daniel Baston <dbaston at gmail.com>
Date:   Mon Aug 26 14:45:35 2019 -0400

    Merge remote-tracking branch 'dbaston/trac-858'


commit 0bf5167794c90984a056d7b5b6a7f5cb4ecaedc9
Author: Daniel Baston <dbaston at gmail.com>
Date:   Sun Aug 25 11:51:14 2019 -0400

    Fix invalid read on exception in IsValidOp
    
    Fixes #858

diff --git a/include/geos/geomgraph/GeometryGraph.h b/include/geos/geomgraph/GeometryGraph.h
index a33b4d5..00bd4df 100644
--- a/include/geos/geomgraph/GeometryGraph.h
+++ b/include/geos/geomgraph/GeometryGraph.h
@@ -30,6 +30,7 @@
 #include <geos/geom/Coordinate.h>
 #include <geos/geom/CoordinateSequence.h> // for unique_ptr<CoordinateSequence>
 #include <geos/geomgraph/PlanarGraph.h>
+#include <geos/geomgraph/index/SegmentIntersector.h>
 #include <geos/geom/LineString.h> // for LineStringLT
 
 #include <geos/inline.h>
@@ -58,7 +59,6 @@ namespace geomgraph {
 class Edge;
 class Node;
 namespace index {
-class SegmentIntersector;
 class EdgeSetIntersector;
 }
 }
@@ -209,7 +209,7 @@ public:
      * @return the SegmentIntersector used, containing information about
      *         the intersections found
      */
-    index::SegmentIntersector*
+    std::unique_ptr<index::SegmentIntersector>
     computeSelfNodes(
         algorithm::LineIntersector* li,
         bool computeRingSelfNodes,
@@ -218,7 +218,7 @@ public:
         return computeSelfNodes(*li, computeRingSelfNodes, env);
     }
 
-    index::SegmentIntersector*
+    std::unique_ptr<index::SegmentIntersector>
     computeSelfNodes(
         algorithm::LineIntersector* li,
         bool computeRingSelfNodes,
@@ -230,15 +230,15 @@ public:
 
     // Quick inline calling the function above, the above should probably
     // be deprecated.
-    index::SegmentIntersector* computeSelfNodes(
+    std::unique_ptr<index::SegmentIntersector> computeSelfNodes(
         algorithm::LineIntersector& li,
         bool computeRingSelfNodes, const geom::Envelope* env = nullptr);
 
-    index::SegmentIntersector* computeSelfNodes(
+    std::unique_ptr<index::SegmentIntersector> computeSelfNodes(
         algorithm::LineIntersector& li,
         bool computeRingSelfNodes, bool isDoneIfProperInt, const geom::Envelope* env = nullptr);
 
-    index::SegmentIntersector* computeEdgeIntersections(GeometryGraph* g,
+    std::unique_ptr<index::SegmentIntersector> computeEdgeIntersections(GeometryGraph* g,
             algorithm::LineIntersector* li, bool includeProper,
             const geom::Envelope* env = nullptr);
 
diff --git a/include/geos/geomgraph/index/SimpleMCSweepLineIntersector.h b/include/geos/geomgraph/index/SimpleMCSweepLineIntersector.h
index 4b3dcee..b92be69 100644
--- a/include/geos/geomgraph/index/SimpleMCSweepLineIntersector.h
+++ b/include/geos/geomgraph/index/SimpleMCSweepLineIntersector.h
@@ -17,10 +17,12 @@
 #define GEOS_GEOMGRAPH_INDEX_SIMPLEMCSWEEPLINEINTERSECTOR_H
 
 #include <geos/export.h>
+#include <memory>
 #include <vector>
 
 #include <geos/geomgraph/index/EdgeSetIntersector.h> // for inheritance
 #include <geos/geomgraph/index/SegmentIntersector.h>
+#include <geos/geomgraph/index/MonotoneChain.h>
 
 #ifdef _MSC_VER
 #pragma warning(push)
@@ -68,7 +70,8 @@ public:
 
 protected:
 
-    std::vector<SweepLineEvent*> events;
+    std::vector<std::unique_ptr<SweepLineEvent>> events;
+    std::vector<std::unique_ptr<MonotoneChain>> chains;
 
     // statistics information
     int nOverlaps;
diff --git a/include/geos/geomgraph/index/SweepLineEvent.h b/include/geos/geomgraph/index/SweepLineEvent.h
index 3f5c6ba..f23b85c 100644
--- a/include/geos/geomgraph/index/SweepLineEvent.h
+++ b/include/geos/geomgraph/index/SweepLineEvent.h
@@ -110,8 +110,9 @@ private:
 
 class GEOS_DLL SweepLineEventLessThen {
 public:
+    template<typename T>
     bool
-    operator()(const SweepLineEvent* f, const SweepLineEvent* s) const
+    operator()(const T& f, const T& s) const
     {
         if(f->xValue < s->xValue) {
             return true;
diff --git a/src/geomgraph/GeometryGraph.cpp b/src/geomgraph/GeometryGraph.cpp
index c5b14c3..9630379 100644
--- a/src/geomgraph/GeometryGraph.cpp
+++ b/src/geomgraph/GeometryGraph.cpp
@@ -22,6 +22,7 @@
 #include <geos/algorithm/BoundaryNodeRule.h>
 
 #include <geos/util/UnsupportedOperationException.h>
+#include <geos/util.h>
 
 #include <geos/geomgraph/GeometryGraph.h>
 #include <geos/geomgraph/Node.h>
@@ -351,18 +352,18 @@ collect_intersecting_edges(const Envelope* env, T start, T end, C& to)
 }
 
 /*public*/
-SegmentIntersector*
+std::unique_ptr<SegmentIntersector>
 GeometryGraph::computeSelfNodes(LineIntersector& li,
                                 bool computeRingSelfNodes, const Envelope* env)
 {
     return computeSelfNodes(li, computeRingSelfNodes, false, env);
 }
 
-SegmentIntersector*
+std::unique_ptr<SegmentIntersector>
 GeometryGraph::computeSelfNodes(LineIntersector& li,
                                 bool computeRingSelfNodes, bool isDoneIfProperInt, const Envelope* env)
 {
-    SegmentIntersector* si = new SegmentIntersector(&li, true, false);
+    auto si = detail::make_unique<SegmentIntersector>(&li, true, false);
     si->setIsDoneIfProperInt(isDoneIfProperInt);
     unique_ptr<EdgeSetIntersector> esi(createEdgeSetIntersector());
 
@@ -382,7 +383,7 @@ GeometryGraph::computeSelfNodes(LineIntersector& li,
 
     bool computeAllSegments = computeRingSelfNodes || ! isRings;
 
-    esi->computeIntersections(se, si, computeAllSegments);
+    esi->computeIntersections(se, si.get(), computeAllSegments);
 
 #if GEOS_DEBUG
     cerr << "SegmentIntersector # tests = " << si->numTests << endl;
@@ -392,7 +393,7 @@ GeometryGraph::computeSelfNodes(LineIntersector& li,
     return si;
 }
 
-SegmentIntersector*
+std::unique_ptr<SegmentIntersector>
 GeometryGraph::computeEdgeIntersections(GeometryGraph* g,
                                         LineIntersector* li, bool includeProper, const Envelope* env)
 {
@@ -425,7 +426,7 @@ GeometryGraph::computeEdgeIntersections(GeometryGraph* g,
 #if GEOS_DEBUG
     cerr << "GeometryGraph::computeEdgeIntersections returns" << endl;
 #endif
-    return si.release();
+    return si;
 }
 
 void
diff --git a/src/geomgraph/index/SimpleMCSweepLineIntersector.cpp b/src/geomgraph/index/SimpleMCSweepLineIntersector.cpp
index f97938a..6e93ead 100644
--- a/src/geomgraph/index/SimpleMCSweepLineIntersector.cpp
+++ b/src/geomgraph/index/SimpleMCSweepLineIntersector.cpp
@@ -33,15 +33,7 @@ SimpleMCSweepLineIntersector::SimpleMCSweepLineIntersector()
 {
 }
 
-SimpleMCSweepLineIntersector::~SimpleMCSweepLineIntersector()
-{
-    for(size_t i = 0; i < events.size(); ++i) {
-        SweepLineEvent* sle = events[i];
-        if(sle->isDelete()) {
-            delete sle;
-        }
-    }
-}
+SimpleMCSweepLineIntersector::~SimpleMCSweepLineIntersector() = default;
 
 void
 SimpleMCSweepLineIntersector::computeIntersections(vector<Edge*>* edges,
@@ -91,12 +83,14 @@ SimpleMCSweepLineIntersector::add(Edge* edge, void* edgeSet)
     auto& startIndex = mce->getStartIndexes();
     size_t n = startIndex.size() - 1;
     events.reserve(events.size() + (n * 2));
+    chains.reserve(chains.size() + n);
     for(size_t i = 0; i < n; ++i) {
         GEOS_CHECK_FOR_INTERRUPTS();
         MonotoneChain* mc = new MonotoneChain(mce, i);
+        chains.emplace_back(mc);
         SweepLineEvent* insertEvent = new SweepLineEvent(edgeSet, mce->getMinX(i), nullptr, mc);
-        events.push_back(insertEvent);
-        events.push_back(new SweepLineEvent(edgeSet, mce->getMaxX(i), insertEvent, mc));
+        events.emplace_back(insertEvent);
+        events.emplace_back(new SweepLineEvent(edgeSet, mce->getMaxX(i), insertEvent, mc));
     }
 }
 
@@ -111,7 +105,7 @@ SimpleMCSweepLineIntersector::prepareEvents()
     sort(events.begin(), events.end(), SweepLineEventLessThen());
     for(size_t i = 0; i < events.size(); ++i) {
         GEOS_CHECK_FOR_INTERRUPTS();
-        SweepLineEvent* ev = events[i];
+        auto& ev = events[i];
         if(ev->isDelete()) {
             ev->getInsertEvent()->setDeleteEventIndex(i);
         }
@@ -125,9 +119,9 @@ SimpleMCSweepLineIntersector::computeIntersections(SegmentIntersector* si)
     prepareEvents();
     for(size_t i = 0; i < events.size(); ++i) {
         GEOS_CHECK_FOR_INTERRUPTS();
-        SweepLineEvent* ev = events[i];
+        auto& ev = events[i];
         if(ev->isInsert()) {
-            processOverlaps(i, ev->getDeleteEventIndex(), ev, si);
+            processOverlaps(i, ev->getDeleteEventIndex(), ev.get(), si);
         }
         if(si->getIsDone()) {
             break;
@@ -147,7 +141,7 @@ SimpleMCSweepLineIntersector::processOverlaps(size_t start, size_t end,
      * Last index can be skipped, because it must be a Delete event.
      */
     for(auto i = start; i < end; ++i) {
-        SweepLineEvent* ev1 = events[i];
+        auto& ev1 = events[i];
         if(ev1->isInsert()) {
             MonotoneChain* mc1 = (MonotoneChain*) ev1->getObject();
             // don't compare edges in same group
diff --git a/src/geomgraph/index/SweepLineEvent.cpp b/src/geomgraph/index/SweepLineEvent.cpp
index 81001cb..a8aacf5 100644
--- a/src/geomgraph/index/SweepLineEvent.cpp
+++ b/src/geomgraph/index/SweepLineEvent.cpp
@@ -40,13 +40,7 @@ SweepLineEvent::SweepLineEvent(void* newEdgeSet, double x,
     }
 }
 
-SweepLineEvent::~SweepLineEvent()
-{
-    if(eventType == DELETE_EVENT) {
-        delete insertEvent;
-        delete obj;
-    }
-}
+SweepLineEvent::~SweepLineEvent() = default;
 
 /**
  * ProjectionEvents are ordered first by their x-value, and then by their
diff --git a/src/operation/overlay/OverlayOp.cpp b/src/operation/overlay/OverlayOp.cpp
index a708b23..b623f03 100644
--- a/src/operation/overlay/OverlayOp.cpp
+++ b/src/operation/overlay/OverlayOp.cpp
@@ -758,9 +758,9 @@ OverlayOp::computeOverlay(OverlayOp::OpCode opCode)
     GEOS_CHECK_FOR_INTERRUPTS();
 
     // node the input Geometries
-    delete arg[0]->computeSelfNodes(li, false, env);
+    arg[0]->computeSelfNodes(li, false, env);
     GEOS_CHECK_FOR_INTERRUPTS();
-    delete arg[1]->computeSelfNodes(li, false, env);
+    arg[1]->computeSelfNodes(li, false, env);
 
 #if GEOS_DEBUG
     cerr << "OverlayOp::computeOverlay: computed SelfNodes" << endl;
@@ -769,7 +769,7 @@ OverlayOp::computeOverlay(OverlayOp::OpCode opCode)
     GEOS_CHECK_FOR_INTERRUPTS();
 
     // compute intersections between edges of the two input geometries
-    delete arg[0]->computeEdgeIntersections(arg[1], &li, true, env);
+    arg[0]->computeEdgeIntersections(arg[1], &li, true, env);
 
 #if GEOS_DEBUG
     cerr << "OverlayOp::computeOverlay: computed EdgeIntersections" << endl;
diff --git a/src/operation/valid/IsValidOp.cpp b/src/operation/valid/IsValidOp.cpp
index c3c1489..c6b705f 100644
--- a/src/operation/valid/IsValidOp.cpp
+++ b/src/operation/valid/IsValidOp.cpp
@@ -215,7 +215,7 @@ IsValidOp::checkValid(const LinearRing* g)
     }
 
     LineIntersector li;
-    delete graph.computeSelfNodes(&li, true, true);
+    graph.computeSelfNodes(&li, true, true);
     checkNoSelfIntersectingRings(&graph);
 }
 
diff --git a/tests/unit/operation/IsSimpleOpTest.cpp b/tests/unit/operation/IsSimpleOpTest.cpp
index a925714..d4a8d59 100644
--- a/tests/unit/operation/IsSimpleOpTest.cpp
+++ b/tests/unit/operation/IsSimpleOpTest.cpp
@@ -132,7 +132,11 @@ void object::test<4>
 
     auto g = reader.readHEX(s);
 
-    g->isSimple();
+    try {
+        g->isSimple();
+    } catch (geos::util::GEOSException & e) {
+        // no memory leaks or invalid reads on exception
+    }
 }
 
 } // namespace tut

commit c487768ede97ea3d4ab78e16f67bb37f9dd3c09d
Author: Daniel Baston <dbaston at gmail.com>
Date:   Sun Aug 25 11:15:27 2019 -0400

    Add failing test for #858

diff --git a/tests/unit/operation/IsSimpleOpTest.cpp b/tests/unit/operation/IsSimpleOpTest.cpp
index 284ccf1..a925714 100644
--- a/tests/unit/operation/IsSimpleOpTest.cpp
+++ b/tests/unit/operation/IsSimpleOpTest.cpp
@@ -11,8 +11,10 @@
 #include <geos/geom/GeometryFactory.h>
 #include <geos/geom/PrecisionModel.h>
 #include <geos/io/WKTReader.h>
+#include <geos/io/WKBReader.h>
 // std
 #include <string>
+#include <sstream>
 #include <memory>
 
 using namespace geos::geom;
@@ -102,4 +104,35 @@ void object::test<3>
     ensure(true == simple);
 }
 
+
+template<>
+template<>
+void object::test<4>
+()
+{
+    // Adapted from https://trac.osgeo.org/geos/ticket/858
+    constexpr char kData[] =
+            "00000000020000000e0000000000000000"
+            "0000000000000000240424242424242424"
+            "24242424280000000000ffffffffffff3b"
+            "ffffffffffffffffffffffff4000010800"
+            "0000030000003b01980000000000000000"
+            "0000000000000000000000000000002900"
+            "000000000100000000490001f34e537437"
+            "6c6f63616c653500000000000000000000"
+            "2800000000000000000000000000000000"
+            "fb0000000000010700000000003a000000"
+            "f100000000000000000000f60000000000"
+            "0000000000000000000000000000000000"
+            "0000000000000000200000000000000000"
+            "0000000000000000000000000000000000";
+
+    geos::io::WKBReader reader;
+    std::istringstream s(kData);
+
+    auto g = reader.readHEX(s);
+
+    g->isSimple();
+}
+
 } // namespace tut

-----------------------------------------------------------------------

Summary of changes:
 include/geos/geomgraph/GeometryGraph.h             | 12 +++----
 .../geomgraph/index/SimpleMCSweepLineIntersector.h |  5 ++-
 include/geos/geomgraph/index/SweepLineEvent.h      |  3 +-
 src/geomgraph/GeometryGraph.cpp                    | 13 ++++----
 .../index/SimpleMCSweepLineIntersector.cpp         | 24 ++++++--------
 src/geomgraph/index/SweepLineEvent.cpp             |  8 +----
 src/operation/overlay/OverlayOp.cpp                |  6 ++--
 src/operation/valid/IsValidOp.cpp                  |  2 +-
 tests/unit/operation/IsSimpleOpTest.cpp            | 37 ++++++++++++++++++++++
 9 files changed, 70 insertions(+), 40 deletions(-)


hooks/post-receive
-- 
GEOS


More information about the geos-commits mailing list