fix WKT I/O error handling

Stream_support/include/CGAL/IO/WKT.h

@@ -44,18 +44,42 @@ namespace internal {
 template <typename K>
 void pop_back_if_equal_to_front(CGAL::Polygon_2<K>& poly)
 {
-  typename CGAL::Polygon_2<K>::iterator it = poly.end();
+  auto last_it = std::prev(poly.end());
-  --it;
+  if((*poly.begin()) == *last_it)
-  if((*poly.begin()) == *it)
+    poly.erase(last_it);
-    poly.erase(it);
 }
 
 template <typename K>
 void pop_back_if_equal_to_front(CGAL::Polygon_with_holes_2<K>& pwh)
 {
   pop_back_if_equal_to_front(pwh.outer_boundary());
-  for(auto i = pwh.holes_begin(); i!= pwh.holes_end(); ++i)
+  for(auto& hole : pwh.holes())
-    pop_back_if_equal_to_front(*i);
+    pop_back_if_equal_to_front(hole);
+}
+
+template <typename Geometry>
+bool read_wkt_or_fail_stream(std::istream& in,
+                             const std::string& line,
+                             Geometry& geometry)
+{
+  try {
+    boost::geometry::read_wkt(line, geometry);
+  } catch(std::exception& e) {
+    std::cerr << "error: " << e.what() << std::endl;
+    in.clear(in.rdstate() | std::ios::failbit);
+    return false;
+  }
+  return true;
+}
+
+bool get_a_new_line(std::istream& in, std::string& line)
+{
+  in >> std::ws; // skip whitespaces
+  if(in.good()) {
+    return !std::getline(in, line).fail();
+  } else {
+    return false;
+  }
 }
 
 } // namespace internal
@@ -76,28 +100,12 @@ template<typename Point>
 bool read_point_WKT(std::istream& in,
                     Point& point)
 {
-  if(!in.good())
-    return false;
-
   std::string line;
-  while(std::getline(in, line))
+  while(internal::get_a_new_line(in, line))
   {
-    std::istringstream iss(line);
+    if(line.substr(0, 5).compare("POINT") == 0)
-    std::string type;
-    iss >> type;
-
-    if(type.substr(0, 5).compare("POINT") == 0)
     {
-      try
+      internal::read_wkt_or_fail_stream(in, line, point);
-      {
-        boost::geometry::read_wkt(line, point);
-      }
-      catch(...)
-      {
-        std::cerr << "error." << std::endl;
-        return false;
-      }
-
       break;
     }
   }
@@ -124,25 +132,13 @@ template<typename MultiPoint>
 bool read_multi_point_WKT(std::istream& in,
                           MultiPoint& mp)
 {
-  if(!in.good())
-    return false;
-
-  CGAL::internal::Geometry_container<MultiPoint, boost::geometry::multi_point_tag> gc(mp);
   std::string line;
-  while(std::getline(in, line))
+  while(internal::get_a_new_line(in, line))
   {
-    std::istringstream iss(line);
+    if(line.substr(0, 10).compare("MULTIPOINT") == 0)
-    std::string type;
-    iss >> type;
-
-    if(type.substr(0, 10).compare("MULTIPOINT") == 0)
     {
-      try{
+      CGAL::internal::Geometry_container<MultiPoint, boost::geometry::multi_point_tag> gc(mp);
-        boost::geometry::read_wkt(line, gc);
+      internal::read_wkt_or_fail_stream(in, line, gc);
-      } catch(...){
-        std::cerr << "error." << std::endl;
-        return false;
-      }
       break;
     }
   }
@@ -170,25 +166,13 @@ template<typename LineString>
 bool read_linestring_WKT(std::istream& in,
                          LineString& polyline)
 {
-  if(!in.good())
-    return false;
-
-  CGAL::internal::Geometry_container<LineString, boost::geometry::linestring_tag> gc(polyline);
   std::string line;
-  while(std::getline(in, line))
+  while(internal::get_a_new_line(in, line))
   {
-    std::istringstream iss(line);
+    if(line.substr(0, 10).compare("LINESTRING") == 0)
-    std::string type;
-    iss >> type;
-
-    if(type.substr(0, 10).compare("LINESTRING") == 0)
     {
-      try{
+      CGAL::internal::Geometry_container<LineString, boost::geometry::linestring_tag> gc(polyline);
-        boost::geometry::read_wkt(line, gc);
+      internal::read_wkt_or_fail_stream(in, line, gc);
-      } catch(...){
-        std::cerr << "error." << std::endl;
-        return false;
-      }
       break;
     }
   }
@@ -214,40 +198,26 @@ template<typename MultiLineString>
 bool read_multi_linestring_WKT(std::istream& in,
                                MultiLineString& mls)
 {
-  if(!in.good())
-    return false;
-
-  typedef typename MultiLineString::value_type                                      PointRange;
-  typedef CGAL::internal::Geometry_container<PointRange, boost::geometry::linestring_tag> LineString;
-
-  std::vector<LineString> pr_range;
-  CGAL::internal::Geometry_container<std::vector<LineString>, boost::geometry::multi_linestring_tag> gc(pr_range);
   std::string line;
-  while(std::getline(in, line))
+  while(internal::get_a_new_line(in, line))
   {
-    std::istringstream iss(line);
+    if(line.substr(0, 15).compare("MULTILINESTRING") == 0)
-    std::string type;
-    iss >> type;
-
-    if(type.substr(0, 15).compare("MULTILINESTRING") == 0)
     {
-      try
+      using PointRange = typename MultiLineString::value_type;
-      {
+      using LineString = CGAL::internal::Geometry_container<PointRange, boost::geometry::linestring_tag>;
-        boost::geometry::read_wkt(line, gc);
+
-      }
+      std::vector<LineString> pr_range;
-      catch(...)
+      CGAL::internal::Geometry_container<std::vector<LineString>, boost::geometry::multi_linestring_tag> gc(pr_range);
-      {
+
-        std::cerr << "error." << std::endl;
+      internal::read_wkt_or_fail_stream(in, line, gc);
-        return false;
+      for(LineString& ls : gc) {
+        mls.push_back(*ls.range);
       }
 
       break;
     }
   }
 
-  for(LineString& ls : gc)
-    mls.push_back(*ls.range);
-
   return !in.fail();
 }
 
@@ -266,28 +236,12 @@ template<typename Polygon>
 bool read_polygon_WKT(std::istream& in,
                       Polygon& polygon)
 {
-  if(!in.good())
-    return false;
-
   std::string line;
-  while(std::getline(in, line))
+  while(internal::get_a_new_line(in, line))
   {
-    std::istringstream iss(line);
+    if(line.substr(0, 7).compare("POLYGON") == 0)
-    std::string type;
-    iss >> type;
-
-    if(type.substr(0, 7).compare("POLYGON") == 0)
     {
-      try
+      internal::read_wkt_or_fail_stream(in, line, polygon);
-      {
-        boost::geometry::read_wkt(line, polygon);
-      }
-      catch( ...)
-      {
-        in.setstate(std::ios::failbit);
-        return false;
-      };
-
       internal::pop_back_if_equal_to_front(polygon);
       break;
     }
@@ -313,31 +267,16 @@ template<typename MultiPolygon>
 bool read_multi_polygon_WKT(std::istream& in,
                             MultiPolygon& polygons)
 {
-  if(!in.good())
-    return false;
-
-  CGAL::internal::Geometry_container<MultiPolygon, boost::geometry::multi_polygon_tag> gc(polygons);
   std::string line;
-  while(std::getline(in, line))
+  while(internal::get_a_new_line(in, line))
   {
-    std::istringstream iss(line);
+    if(line.substr(0, 12).compare("MULTIPOLYGON") == 0)
-    std::string type;
-    iss >> type;
-
-    if(type.substr(0, 12).compare("MULTIPOLYGON") == 0)
     {
-      try
+      CGAL::internal::Geometry_container<MultiPolygon, boost::geometry::multi_polygon_tag> gc(polygons);
-      {
+      internal::read_wkt_or_fail_stream(in, line, gc);
-        boost::geometry::read_wkt(line, gc);
-      }
-      catch( ...)
-      {
-        in.setstate(std::ios::failbit);
-        return false;
-      };
 
-      for(typename CGAL::internal::Geometry_container<MultiPolygon, boost::geometry::multi_polygon_tag>::iterator it = gc.begin(); it != gc.end(); ++it)
+      for(auto& p : gc)
-        internal::pop_back_if_equal_to_front(*it);
+        internal::pop_back_if_equal_to_front(p);
 
       break;
     }
@@ -517,17 +456,15 @@ bool read_WKT(std::istream& is,
               MultiLineString& polylines,
               MultiPolygon& polygons)
 {
-  if(!is.good())
+  auto fail = [&is]() { is.clear(is.rdstate() | std::ios::failbit); return false; };
-    return false;
 
-  while(is.good() && !is.eof())
+  std::string line;
+  while(is >> std::ws && is.good() && std::getline(is, line))
   {
     typedef typename MultiPoint::value_type Point;
     typedef typename MultiLineString::value_type LineString;
     typedef typename MultiPolygon::value_type Polygon;
 
-    std::string line;
-    std::getline(is, line);
     std::string::size_type header_end = line.find("("); // }
     if(header_end == std::string::npos){
       continue;
@@ -549,42 +486,42 @@ bool read_WKT(std::istream& is,
     if(type == "POINT")
     {
       Point p;
-      CGAL::IO::read_point_WKT(iss, p);
+      if(!IO::read_point_WKT(iss, p) ) return fail();
       points.push_back(p);
     }
     else if(type == "LINESTRING")
     {
       LineString l;
-      CGAL::IO::read_linestring_WKT(iss, l);
+      if(!IO::read_linestring_WKT(iss, l)) return fail();
-      polylines.push_back(l);
+      polylines.push_back(std::move(l));
     }
     else if(type == "POLYGON")
     {
       Polygon p;
-      CGAL::IO::read_polygon_WKT(iss, p);
+      if(!IO::read_polygon_WKT(iss, p)) return fail();
       if(!p.outer_boundary().is_empty())
-        polygons.push_back(p);
+        polygons.push_back(std::move(p));
     }
     else if(type == "MULTIPOINT")
     {
       MultiPoint mp;
-      CGAL::IO::read_multi_point_WKT(iss, mp);
+      if(!IO::read_multi_point_WKT(iss, mp)) return fail();
       for(const Point& point : mp)
         points.push_back(point);
     }
     else if(type == "MULTILINESTRING")
     {
       MultiLineString mls;
-      CGAL::IO::read_multi_linestring_WKT(iss, mls);
+      if(!IO::read_multi_linestring_WKT(iss, mls)) return fail();
-      for(const LineString& ls : mls)
+      for(LineString& ls : mls)
-        polylines.push_back(ls);
+        polylines.push_back(std::move(ls));
     }
     else if(type == "MULTIPOLYGON")
     {
       MultiPolygon mp;
-      CGAL::IO::read_multi_polygon_WKT(iss, mp);
+      if(!IO::read_multi_polygon_WKT(iss, mp)) return fail();
-      for(const Polygon& poly : mp)
+      for(Polygon& poly : mp)
-        polygons.push_back(poly);
+        polygons.push_back(std::move(poly));
     }
   }
 

Stream_support/include/CGAL/Stream_support/internal/Geometry_container.h

@@ -46,20 +46,16 @@ struct Geometry_container{
   typedef typename Range::size_type size_type;
   typedef typename Range::value_type value_type;
   std::shared_ptr<Range>  range;
-  bool must_delete;
   //
   // Default constructor.
   // Creates a new internal Range.
   // De-allocate memory after usage.
-  Geometry_container():range(new Range()), must_delete(true)
+  Geometry_container() : range(std::make_shared<Range>()) {}
-  {
-  }
   /*
-   Copy constructor.
+   Store a pointer to the given range.
    Memory NOT de-allocated after usage.
   */
-  Geometry_container(Range& range)
+  Geometry_container(Range& range) : range(&range, Dummy_deleter()) {}
-    :range(&range, Dummy_deleter()), must_delete(false){}
 
   iterator begin()
   { return range->begin(); }