From 765744e8b2ea77a00b2ba99b42f5b7e6527601c7 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Thu, 6 Feb 2025 01:22:02 +0100 Subject: [PATCH] fix WKT I/O error handling --- Stream_support/include/CGAL/IO/WKT.h | 215 +++++++----------- .../internal/Geometry_container.h | 10 +- 2 files changed, 79 insertions(+), 146 deletions(-) diff --git a/Stream_support/include/CGAL/IO/WKT.h b/Stream_support/include/CGAL/IO/WKT.h index 3a3eea129d6..4aa12443fc1 100644 --- a/Stream_support/include/CGAL/IO/WKT.h +++ b/Stream_support/include/CGAL/IO/WKT.h @@ -44,18 +44,42 @@ namespace internal { template void pop_back_if_equal_to_front(CGAL::Polygon_2& poly) { - typename CGAL::Polygon_2::iterator it = poly.end(); - --it; - if((*poly.begin()) == *it) - poly.erase(it); + auto last_it = std::prev(poly.end()); + if((*poly.begin()) == *last_it) + poly.erase(last_it); } template void pop_back_if_equal_to_front(CGAL::Polygon_with_holes_2& pwh) { pop_back_if_equal_to_front(pwh.outer_boundary()); - for(auto i = pwh.holes_begin(); i!= pwh.holes_end(); ++i) - pop_back_if_equal_to_front(*i); + for(auto& hole : pwh.holes()) + pop_back_if_equal_to_front(hole); +} + +template +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 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); - std::string type; - iss >> type; - - if(type.substr(0, 5).compare("POINT") == 0) + if(line.substr(0, 5).compare("POINT") == 0) { - try - { - boost::geometry::read_wkt(line, point); - } - catch(...) - { - std::cerr << "error." << std::endl; - return false; - } - + internal::read_wkt_or_fail_stream(in, line, point); break; } } @@ -124,25 +132,13 @@ template bool read_multi_point_WKT(std::istream& in, MultiPoint& mp) { - if(!in.good()) - return false; - - CGAL::internal::Geometry_container gc(mp); std::string line; - while(std::getline(in, line)) + while(internal::get_a_new_line(in, line)) { - std::istringstream iss(line); - std::string type; - iss >> type; - - if(type.substr(0, 10).compare("MULTIPOINT") == 0) + if(line.substr(0, 10).compare("MULTIPOINT") == 0) { - try{ - boost::geometry::read_wkt(line, gc); - } catch(...){ - std::cerr << "error." << std::endl; - return false; - } + CGAL::internal::Geometry_container gc(mp); + internal::read_wkt_or_fail_stream(in, line, gc); break; } } @@ -170,25 +166,13 @@ template bool read_linestring_WKT(std::istream& in, LineString& polyline) { - if(!in.good()) - return false; - - CGAL::internal::Geometry_container gc(polyline); std::string line; - while(std::getline(in, line)) + while(internal::get_a_new_line(in, line)) { - std::istringstream iss(line); - std::string type; - iss >> type; - - if(type.substr(0, 10).compare("LINESTRING") == 0) + if(line.substr(0, 10).compare("LINESTRING") == 0) { - try{ - boost::geometry::read_wkt(line, gc); - } catch(...){ - std::cerr << "error." << std::endl; - return false; - } + CGAL::internal::Geometry_container gc(polyline); + internal::read_wkt_or_fail_stream(in, line, gc); break; } } @@ -214,40 +198,26 @@ template 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 LineString; - - std::vector pr_range; - CGAL::internal::Geometry_container, 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); - std::string type; - iss >> type; - - if(type.substr(0, 15).compare("MULTILINESTRING") == 0) + if(line.substr(0, 15).compare("MULTILINESTRING") == 0) { - try - { - boost::geometry::read_wkt(line, gc); - } - catch(...) - { - std::cerr << "error." << std::endl; - return false; + using PointRange = typename MultiLineString::value_type; + using LineString = CGAL::internal::Geometry_container; + + std::vector pr_range; + CGAL::internal::Geometry_container, boost::geometry::multi_linestring_tag> gc(pr_range); + + internal::read_wkt_or_fail_stream(in, line, gc); + 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 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); - std::string type; - iss >> type; - - if(type.substr(0, 7).compare("POLYGON") == 0) + if(line.substr(0, 7).compare("POLYGON") == 0) { - try - { - boost::geometry::read_wkt(line, polygon); - } - catch( ...) - { - in.setstate(std::ios::failbit); - return false; - }; - + internal::read_wkt_or_fail_stream(in, line, polygon); internal::pop_back_if_equal_to_front(polygon); break; } @@ -313,31 +267,16 @@ template bool read_multi_polygon_WKT(std::istream& in, MultiPolygon& polygons) { - if(!in.good()) - return false; - - CGAL::internal::Geometry_container gc(polygons); std::string line; - while(std::getline(in, line)) + while(internal::get_a_new_line(in, line)) { - std::istringstream iss(line); - std::string type; - iss >> type; - - if(type.substr(0, 12).compare("MULTIPOLYGON") == 0) + if(line.substr(0, 12).compare("MULTIPOLYGON") == 0) { - try - { - boost::geometry::read_wkt(line, gc); - } - catch( ...) - { - in.setstate(std::ios::failbit); - return false; - }; + CGAL::internal::Geometry_container gc(polygons); + internal::read_wkt_or_fail_stream(in, line, gc); - for(typename CGAL::internal::Geometry_container::iterator it = gc.begin(); it != gc.end(); ++it) - internal::pop_back_if_equal_to_front(*it); + for(auto& p : gc) + 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()) - return false; + auto fail = [&is]() { is.clear(is.rdstate() | std::ios::failbit); 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); - polylines.push_back(l); + if(!IO::read_linestring_WKT(iss, l)) return fail(); + 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); - for(const LineString& ls : mls) - polylines.push_back(ls); + if(!IO::read_multi_linestring_WKT(iss, mls)) return fail(); + for(LineString& ls : mls) + polylines.push_back(std::move(ls)); } else if(type == "MULTIPOLYGON") { MultiPolygon mp; - CGAL::IO::read_multi_polygon_WKT(iss, mp); - for(const Polygon& poly : mp) - polygons.push_back(poly); + if(!IO::read_multi_polygon_WKT(iss, mp)) return fail(); + for(Polygon& poly : mp) + polygons.push_back(std::move(poly)); } } diff --git a/Stream_support/include/CGAL/Stream_support/internal/Geometry_container.h b/Stream_support/include/CGAL/Stream_support/internal/Geometry_container.h index e3e26fe8b61..28a1aff6245 100644 --- a/Stream_support/include/CGAL/Stream_support/internal/Geometry_container.h +++ b/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; - 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()) {} /* - Copy constructor. + Store a pointer to the given range. Memory NOT de-allocated after usage. */ - Geometry_container(Range& range) - :range(&range, Dummy_deleter()), must_delete(false){} + Geometry_container(Range& range) : range(&range, Dummy_deleter()) {} iterator begin() { return range->begin(); }