[geos-commits] [SCM] GEOS branch master updated. d62686316e59f59e9f64814d3a769c736b27d885

git at osgeo.org git at osgeo.org
Fri Jan 17 19:10:47 PST 2020


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  d62686316e59f59e9f64814d3a769c736b27d885 (commit)
       via  24f22e720ebcd3ed12d1651e539f44f774f3c5ef (commit)
      from  07254bfc260d0c106019d66b095d76410a7d4202 (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 d62686316e59f59e9f64814d3a769c736b27d885
Merge: 07254bf 24f22e7
Author: Daniel Baston <dbaston at gmail.com>
Date:   Fri Jan 17 22:10:19 2020 -0500

    Merge branch 'trac-1010'


commit 24f22e720ebcd3ed12d1651e539f44f774f3c5ef
Author: Daniel Baston <dbaston at gmail.com>
Date:   Wed Jan 15 16:24:46 2020 -0500

    Fix memory leak in GEOSLineMerge
    
    Fixes #1010

diff --git a/capi/geos_ts_c.cpp b/capi/geos_ts_c.cpp
index e60e1bd..e9bdffa 100644
--- a/capi/geos_ts_c.cpp
+++ b/capi/geos_ts_c.cpp
@@ -1893,9 +1893,9 @@ extern "C" {
             LineMerger lmrgr;
             lmrgr.add(g);
 
-            std::vector<LineString*>* lines = lmrgr.getMergedLineStrings();
+            auto lines = lmrgr.getMergedLineStrings();
 
-            auto out = gf->buildGeometry(lines->begin(), lines->end());
+            auto out = gf->buildGeometry(std::move(lines));
             out->setSRID(g->getSRID());
 
             return out.release();
diff --git a/doc/example.cpp b/doc/example.cpp
index 678e821..31aa562 100644
--- a/doc/example.cpp
+++ b/doc/example.cpp
@@ -1073,12 +1073,11 @@ do_all()
     /////////////////////////////////////////////
     LineMerger lm;
     lm.add(geoms);
-    vector<LineString*>* mls = lm.getMergedLineStrings();
+    auto mls = lm.getMergedLineStrings();
     newgeoms = new vector<const Geometry*>;
-    for(unsigned int i = 0; i < mls->size(); i++) {
-        newgeoms->push_back((*mls)[i]);
+    for(unsigned int i = 0; i < mls.size(); i++) {
+        newgeoms->push_back(mls[i].release());
     }
-    delete mls;
 
     cout << endl << "----- HERE IS THE LINEMERGE OUTPUT ------" << endl;
     wkt_print_geoms(newgeoms);
diff --git a/include/geos/geom/GeometryFactory.h b/include/geos/geom/GeometryFactory.h
index 10c90da..8bb99a3 100644
--- a/include/geos/geom/GeometryFactory.h
+++ b/include/geos/geom/GeometryFactory.h
@@ -334,6 +334,12 @@ public:
 
     std::unique_ptr<Geometry> buildGeometry(std::vector<std::unique_ptr<Geometry>> && geoms) const;
 
+    std::unique_ptr<Geometry> buildGeometry(std::vector<std::unique_ptr<Point>> && geoms) const;
+
+    std::unique_ptr<Geometry> buildGeometry(std::vector<std::unique_ptr<LineString>> && geoms) const;
+
+    std::unique_ptr<Geometry> buildGeometry(std::vector<std::unique_ptr<Polygon>> && geoms) const;
+
     /// See buildGeometry(std::vector<Geometry *>&) for semantics
     //
     /// Will clone the geometries accessible trough the iterator.
diff --git a/include/geos/operation/linemerge/LineMerger.h b/include/geos/operation/linemerge/LineMerger.h
index d21465b..9ea6125 100644
--- a/include/geos/operation/linemerge/LineMerger.h
+++ b/include/geos/operation/linemerge/LineMerger.h
@@ -20,8 +20,10 @@
 #define GEOS_OP_LINEMERGE_LINEMERGER_H
 
 #include <geos/export.h>
+#include <geos/geom/LineString.h>
 #include <geos/operation/linemerge/LineMergeGraph.h> // for composition
 
+#include <memory>
 #include <vector>
 
 #ifdef _MSC_VER
@@ -32,7 +34,6 @@
 // Forward declarations
 namespace geos {
 namespace geom {
-class LineString;
 class GeometryFactory;
 class Geometry;
 }
@@ -78,7 +79,7 @@ private:
 
     LineMergeGraph graph;
 
-    std::vector<geom::LineString*>* mergedLineStrings;
+    std::vector<std::unique_ptr<geom::LineString>> mergedLineStrings;
 
     std::vector<EdgeString*> edgeStrings;
 
@@ -128,7 +129,7 @@ public:
      *
      * Ownership of vector _and_ its elements to caller.
      */
-    std::vector<geom::LineString*>* getMergedLineStrings();
+    std::vector<std::unique_ptr<geom::LineString>> getMergedLineStrings();
 
     void add(const geom::LineString* lineString);
 
diff --git a/src/geom/GeometryFactory.cpp b/src/geom/GeometryFactory.cpp
index 6cfa396..2a0b968 100644
--- a/src/geom/GeometryFactory.cpp
+++ b/src/geom/GeometryFactory.cpp
@@ -255,7 +255,7 @@ GeometryFactory::toGeometry(const Envelope* envelope) const
     Coordinate coord;
 
     if(envelope->isNull()) {
-        return std::unique_ptr<Geometry>(createPoint());
+        return createPoint();
     }
     if(envelope->getMinX() == envelope->getMaxX() && envelope->getMinY() == envelope->getMaxY()) {
         coord.x = envelope->getMinX();
@@ -714,6 +714,48 @@ GeometryFactory::buildGeometry(std::vector<std::unique_ptr<Geometry>> && geoms)
     }
 }
 
+std::unique_ptr<Geometry>
+GeometryFactory::buildGeometry(std::vector<std::unique_ptr<Point>> && geoms) const
+{
+    if (geoms.empty()) {
+        return createGeometryCollection();
+    }
+
+    if (geoms.size() == 1) {
+        return std::move(geoms[0]);
+    }
+
+    return createMultiPoint(std::move(geoms));
+}
+
+std::unique_ptr<Geometry>
+GeometryFactory::buildGeometry(std::vector<std::unique_ptr<LineString>> && geoms) const
+{
+    if (geoms.empty()) {
+        return createGeometryCollection();
+    }
+
+    if (geoms.size() == 1) {
+        return std::move(geoms[0]);
+    }
+
+    return createMultiLineString(std::move(geoms));
+}
+
+std::unique_ptr<Geometry>
+GeometryFactory::buildGeometry(std::vector<std::unique_ptr<Polygon>> && geoms) const
+{
+    if (geoms.empty()) {
+        return createGeometryCollection();
+    }
+
+    if (geoms.size() == 1) {
+        return std::move(geoms[0]);
+    }
+
+    return createMultiPolygon(std::move(geoms));
+}
+
 /*public*/
 Geometry*
 GeometryFactory::buildGeometry(const vector<const Geometry*>& fromGeoms) const
diff --git a/src/operation/buffer/BufferBuilder.cpp b/src/operation/buffer/BufferBuilder.cpp
index 2f84c51..8515918 100644
--- a/src/operation/buffer/BufferBuilder.cpp
+++ b/src/operation/buffer/BufferBuilder.cpp
@@ -252,17 +252,16 @@ BufferBuilder::bufferLineSingleSided(const Geometry* g, double distance,
     // Merge result lines together.
     LineMerger lineMerge;
     lineMerge.add(intersectedLines.get());
-    std::unique_ptr< std::vector< LineString* > > mergedLines(
-        lineMerge.getMergedLineStrings());
+    auto mergedLines = lineMerge.getMergedLineStrings();
 
     // Convert the result into a std::vector< Geometry* >.
     std::vector< Geometry* >* mergedLinesGeom = new std::vector< Geometry* >();
     const Coordinate& startPoint = l->getCoordinatesRO()->front();
     const Coordinate& endPoint = l->getCoordinatesRO()->back();
-    while(!mergedLines->empty()) {
+    while(!mergedLines.empty()) {
         // Remove end points if they are a part of the original line to be
         // buffered.
-        CoordinateSequence::Ptr coords(mergedLines->back()->getCoordinates());
+        CoordinateSequence::Ptr coords(mergedLines.back()->getCoordinates());
         if(nullptr != coords.get()) {
             // Use 98% of the buffer width as the point-distance requirement - this
             // is to ensure that the point that is "distance" +/- epsilon is not
@@ -349,8 +348,7 @@ BufferBuilder::bufferLineSingleSided(const Geometry* g, double distance,
             }
         }
 
-        geomFact->destroyGeometry(mergedLines->back());
-        mergedLines->pop_back();
+        mergedLines.pop_back();
     }
 
     // Clean up.
diff --git a/src/operation/linemerge/LineMerger.cpp b/src/operation/linemerge/LineMerger.cpp
index a45d2f5..ec680f3 100644
--- a/src/operation/linemerge/LineMerger.cpp
+++ b/src/operation/linemerge/LineMerger.cpp
@@ -26,6 +26,7 @@
 //#include <geos/planargraph/GraphComponent.h>
 #include <geos/geom/GeometryComponentFilter.h>
 #include <geos/geom/LineString.h>
+#include <geos/util.h>
 
 #include <cassert>
 #include <functional>
@@ -52,7 +53,6 @@ LineMerger::add(vector<const Geometry*>* geometries)
 }
 
 LineMerger::LineMerger():
-    mergedLineStrings(nullptr),
     factory(nullptr)
 {
 }
@@ -105,7 +105,7 @@ LineMerger::add(const LineString* lineString)
 void
 LineMerger::merge()
 {
-    if(mergedLineStrings != nullptr) {
+    if(!mergedLineStrings.empty()) {
         return;
     }
 
@@ -124,10 +124,10 @@ LineMerger::merge()
     buildEdgeStringsForIsolatedLoops();
 
     auto numEdgeStrings = edgeStrings.size();
-    mergedLineStrings = new vector<LineString*>(numEdgeStrings);
+    mergedLineStrings.reserve(numEdgeStrings);
     for(size_t i = 0; i < numEdgeStrings; ++i) {
         EdgeString* edgeString = edgeStrings[i];
-        (*mergedLineStrings)[i] = edgeString->toLineString();
+        mergedLineStrings.emplace_back(edgeString->toLineString());
     }
 }
 
@@ -227,14 +227,14 @@ LineMerger::buildEdgeStringStartingWith(LineMergeDirectedEdge* start)
 /**
  * Returns the LineStrings built by the merging process.
  */
-vector<LineString*>*
+std::vector<std::unique_ptr<LineString>>
 LineMerger::getMergedLineStrings()
 {
     merge();
 
     // Explicitly give ownership to the caller.
-    vector<LineString*>* ret = mergedLineStrings;
-    mergedLineStrings = nullptr;
+    auto ret = std::move(mergedLineStrings);
+    mergedLineStrings.clear();
     return ret;
 }
 
diff --git a/tests/unit/capi/GEOSLineMergeTest.cpp b/tests/unit/capi/GEOSLineMergeTest.cpp
new file mode 100644
index 0000000..59ae269
--- /dev/null
+++ b/tests/unit/capi/GEOSLineMergeTest.cpp
@@ -0,0 +1,70 @@
+//
+// Test Suite for C-API GEOSLineMerge
+
+#include <tut/tut.hpp>
+// geos
+#include <geos_c.h>
+// std
+#include <cstdarg>
+#include <cstdio>
+
+namespace tut {
+//
+// Test Group
+//
+
+// Common data used in test cases.
+struct test_capigeoslinemerge_data {
+
+    static void
+    notice(const char* fmt, ...)
+    {
+        std::fprintf(stdout, "NOTICE: ");
+
+        va_list ap;
+        va_start(ap, fmt);
+        std::vfprintf(stdout, fmt, ap);
+        va_end(ap);
+
+        std::fprintf(stdout, "\n");
+    }
+
+    test_capigeoslinemerge_data()
+    {
+        initGEOS(notice, notice);
+    }
+
+    ~test_capigeoslinemerge_data()
+    {
+        finishGEOS();
+    }
+
+};
+
+typedef test_group<test_capigeoslinemerge_data> group;
+typedef group::object object;
+
+group test_capigeoslinemerge_group("capi::GEOSLineMerge");
+
+//
+// Test Cases
+//
+
+template<>
+template<>
+void object::test<1>
+()
+{
+    auto input = GEOSGeomFromWKT("MULTILINESTRING((0 0, 0 100),(0 -5, 0 0))");
+    auto result = GEOSLineMerge(input);
+    auto expected = GEOSGeomFromWKT("LINESTRING(0 -5,0 0,0 100)");
+
+    ensure(GEOSEqualsExact(result, expected, 0));
+
+    GEOSGeom_destroy(input);
+    GEOSGeom_destroy(result);
+    GEOSGeom_destroy(expected);
+}
+
+} // namespace tut
+
diff --git a/tests/unit/operation/linemerge/LineMergerTest.cpp b/tests/unit/operation/linemerge/LineMergerTest.cpp
index 6ede138..19906c2 100644
--- a/tests/unit/operation/linemerge/LineMergerTest.cpp
+++ b/tests/unit/operation/linemerge/LineMergerTest.cpp
@@ -36,10 +36,9 @@ struct test_linemerger_data {
 
     std::vector<const geos::geom::Geometry*> inpGeoms;
     std::vector<const geos::geom::Geometry*> expGeoms;
-    LineVect* mrgGeoms;
 
     test_linemerger_data()
-        : wktreader(), wktwriter(), mrgGeoms(nullptr)
+        : wktreader(), wktwriter()
     {
         wktwriter.setTrim(true);
     }
@@ -48,10 +47,6 @@ struct test_linemerger_data {
     {
         delAll(inpGeoms);
         delAll(expGeoms);
-        if(mrgGeoms) {
-            delAll(*mrgGeoms);
-            delete mrgGeoms;
-        }
     }
 
     GeomPtr
@@ -79,8 +74,8 @@ struct test_linemerger_data {
         readWKT(expectedWKT, expGeoms);
 
         lineMerger.add(&inpGeoms);
-        mrgGeoms = lineMerger.getMergedLineStrings();
-        compare(expGeoms, *mrgGeoms, compareDirections);
+        auto mrgGeoms = lineMerger.getMergedLineStrings();
+        compare(expGeoms, mrgGeoms, compareDirections);
 
     }
 
@@ -110,10 +105,8 @@ struct test_linemerger_data {
     contains(TargetContainer& actualGeometries,
              const Geom* g, bool exact)
     {
-        for(typename TargetContainer::const_iterator
-                i = actualGeometries.begin(),
-                e = actualGeometries.end(); i != e; ++i) {
-            Geom* element = dynamic_cast<Geom*>(*i);
+        for(const auto& e : actualGeometries) {
+            Geom* element = dynamic_cast<Geom*>(e.get());
             if(exact && element->equalsExact(g)) {
                 return true;
             }
@@ -288,10 +281,10 @@ void object::test<8>
     // Merge MultiLineString into LineString
     LineMerger lineMerger;
     lineMerger.add(lines1234.get());
-    mrgGeoms = lineMerger.getMergedLineStrings();
+    auto mrgGeoms = lineMerger.getMergedLineStrings();
 
     GeomPtr expected(readWKT("LINESTRING(0 0, 0 5, 5 5, 5 0, 0 0)"));
-    ensure(contains(*mrgGeoms, expected.get(), true));
+    ensure(contains(mrgGeoms, expected.get(), true));
 }
 
 } // namespace tut
diff --git a/tests/xmltester/XMLTester.cpp b/tests/xmltester/XMLTester.cpp
index c133809..b44cf31 100644
--- a/tests/xmltester/XMLTester.cpp
+++ b/tests/xmltester/XMLTester.cpp
@@ -1544,11 +1544,9 @@ XMLTester::parseTest(const tinyxml2::XMLNode* node)
 
             LineMerger merger;
             merger.add(p_gT);
-            std::unique_ptr< std::vector<geom::LineString*> > lines(merger.getMergedLineStrings());
-            std::vector<geom::Geometry*>* newgeoms = new std::vector<geom::Geometry*>(lines->begin(),
-                    lines->end());
+            auto lines = merger.getMergedLineStrings();
 
-            GeomPtr gRealRes(factory->createGeometryCollection(newgeoms));
+            GeomPtr gRealRes(factory->createGeometryCollection(std::move(lines)));
             gRealRes->normalize();
 
             if(gRes->compareTo(gRealRes.get()) == 0) {

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

Summary of changes:
 capi/geos_ts_c.cpp                                 |  4 +-
 doc/example.cpp                                    |  7 ++--
 include/geos/geom/GeometryFactory.h                |  6 +++
 include/geos/operation/linemerge/LineMerger.h      |  7 ++--
 src/geom/GeometryFactory.cpp                       | 44 +++++++++++++++++++++-
 src/operation/buffer/BufferBuilder.cpp             | 10 ++---
 src/operation/linemerge/LineMerger.cpp             | 14 +++----
 ...OSInterpolateTest.cpp => GEOSLineMergeTest.cpp} | 34 +++++++++--------
 tests/unit/operation/linemerge/LineMergerTest.cpp  | 21 ++++-------
 tests/xmltester/XMLTester.cpp                      |  6 +--
 10 files changed, 96 insertions(+), 57 deletions(-)
 copy tests/unit/capi/{GEOSInterpolateTest.cpp => GEOSLineMergeTest.cpp} (51%)


hooks/post-receive
-- 
GEOS


More information about the geos-commits mailing list