ensure we're returning the same object from multiple return paths

from_wkb now returns geometry::geometry_empty instead of throwing
update cpp_tests

plugins/input/shape/shape_io.cpp

@@ -90,12 +90,12 @@ void shape_io::read_bbox(shape_file::record_type & record, mapnik::box2d<double>
 
 mapnik::geometry::geometry shape_io::read_polyline(shape_file::record_type & record)
 {
+    mapnik::geometry::geometry geom; // default empty
     int num_parts = record.read_ndr_integer();
     int num_points = record.read_ndr_integer();
 
     if (num_parts == 1)
     {
-
         mapnik::geometry::line_string line;
         line.reserve(num_points);
         record.skip(4);
@@ -105,7 +105,7 @@ mapnik::geometry::geometry shape_io::read_polyline(shape_file::record_type & rec
             double y = record.read_double();
             line.add_coord(x, y);
         }
-        return mapnik::geometry::geometry(std::move(line));
+        geom = std::move(line);
     }
     else
     {
@@ -139,13 +139,15 @@ mapnik::geometry::geometry shape_io::read_polyline(shape_file::record_type & rec
             }
             multi_line.push_back(std::move(line));
         }
-        return mapnik::geometry::geometry(std::move(multi_line));
+        geom = std::move(multi_line);
     }
+    return geom;
 }
 
 
 mapnik::geometry::geometry shape_io::read_polygon(shape_file::record_type & record)
 {
+    mapnik::geometry::geometry geom; // default empty
     int num_parts = record.read_ndr_integer();
     int num_points = record.read_ndr_integer();
 
@@ -190,7 +192,9 @@ mapnik::geometry::geometry shape_io::read_polygon(shape_file::record_type & reco
     if (multi_poly.size() > 0) // multi
     {
         multi_poly.emplace_back(std::move(poly));
-        return mapnik::geometry::geometry(std::move(multi_poly));
+        geom = std::move(multi_poly);
+        return geom;
     }
-    return mapnik::geometry::geometry(std::move(poly));
+    geom = std::move(poly);
+    return geom;
 }

src/wkb.cpp

@@ -120,61 +120,81 @@ public:
 
     mapnik::geometry::geometry read()
     {
+        mapnik::geometry::geometry geom = mapnik::geometry::geometry_empty();
         int type = read_integer();
         switch (type)
         {
         case wkbPoint:
-            return std::move(mapnik::geometry::geometry(read_point()));
+            geom = std::move(read_point());
+            break;
         case wkbLineString:
-            return std::move(mapnik::geometry::geometry(read_linestring()));
+            geom = std::move(read_linestring());
+            break;
         case wkbPolygon:
-            return std::move(mapnik::geometry::geometry(read_polygon()));
+            geom = std::move(read_polygon());
+            break;
         case wkbMultiPoint:
-            return std::move(mapnik::geometry::geometry(read_multipoint()));
+            geom = std::move(read_multipoint());
+            break;
         case wkbMultiLineString:
-            return std::move(mapnik::geometry::geometry(read_multilinestring()));
+            geom = std::move(read_multilinestring());
+            break;
         case wkbMultiPolygon:
-            return std::move(mapnik::geometry::geometry(read_multipolygon()));
+            geom = std::move(read_multipolygon());
+            break;
         case wkbGeometryCollection:
-            return std::move(mapnik::geometry::geometry(read_collection()));
+            geom = std::move(read_collection());
+            break;
         case wkbPointZ:
         case wkbPointM:
-            return std::move(mapnik::geometry::geometry(read_point<true>()));
+            geom = std::move(read_point<true>());
+            break;
         case wkbPointZM:
-            return std::move(mapnik::geometry::geometry(read_point<true,true>()));
+            geom = std::move(read_point<true,true>());
+            break;
         case wkbLineStringZ:
         case wkbLineStringM:
-            return std::move(mapnik::geometry::geometry(read_linestring<true>()));
+            geom = std::move(read_linestring<true>());
+            break;
         case wkbLineStringZM:
-            return std::move(mapnik::geometry::geometry(read_linestring<true,true>()));
+            geom = std::move(read_linestring<true,true>());
+            break;
         case wkbPolygonZ:
         case wkbPolygonM:
-            return std::move(mapnik::geometry::geometry(read_polygon<true>()));
+            geom = std::move(read_polygon<true>());
+            break;
         case wkbPolygonZM:
-            return std::move(mapnik::geometry::geometry(read_polygon<true,true>()));
+            geom = std::move(read_polygon<true,true>());
         case wkbMultiPointZ:
         case wkbMultiPointM:
-            return std::move(mapnik::geometry::geometry(read_multipoint<true>()));
+            geom = std::move(read_multipoint<true>());
+            break;
         case wkbMultiPointZM:
-            return std::move(mapnik::geometry::geometry(read_multipoint<true,true>()));
+            geom = std::move(read_multipoint<true,true>());
+            break;
         case wkbMultiLineStringZ:
         case wkbMultiLineStringM:
-            return std::move(mapnik::geometry::geometry(read_multilinestring<true>()));
+            geom = std::move(read_multilinestring<true>());
+            break;
         case wkbMultiLineStringZM:
-            return std::move(mapnik::geometry::geometry(read_multilinestring<true,true>()));
+            geom = std::move(read_multilinestring<true,true>());
+            break;
         case wkbMultiPolygonZ:
         case wkbMultiPolygonM:
-            return std::move(mapnik::geometry::geometry(read_multipolygon<true>()));
+            geom = std::move(read_multipolygon<true>());
+            break;
         case wkbMultiPolygonZM:
-            return std::move(mapnik::geometry::geometry(read_multipolygon<true,true>()));
+            geom = std::move(read_multipolygon<true,true>());
+            break;
         case wkbGeometryCollectionZ:
         case wkbGeometryCollectionM:
         case wkbGeometryCollectionZM:
-            return std::move(mapnik::geometry::geometry(read_collection()));
+            geom = std::move(read_collection());
+            break;
         default:
             break;
         }
-        throw std::runtime_error("Uknown WKB geometry type");
+        return geom;
     }
 
 private:

tests/cpp_tests/wkb_formats_test.cpp

@@ -80,9 +80,11 @@ int main(int argc, char** argv)
 
 
 
-        BOOST_TEST_THROWS(mapnik::geometry_utils::from_wkb((const char*)sp_invalid_blob,
+        geom = mapnik::geometry_utils::from_wkb((const char*)sp_invalid_blob,
                                                 sizeof(sp_invalid_blob) / sizeof(sp_invalid_blob[0]),
-                                                           mapnik::wkbAuto), std::exception);
+                                                mapnik::wkbAuto);
+        BOOST_TEST(geom.is<mapnik::geometry::geometry_empty>()); // returns geometry_empty
+
         // sqlite generic wkb blob
 
         geom = mapnik::geometry_utils::from_wkb((const char*)sq_valid_blob,
@@ -96,9 +98,10 @@ int main(int argc, char** argv)
 
         BOOST_TEST(mapnik::geometry::is_valid(geom) && mapnik::geometry::is_simple(geom));
 
-        BOOST_TEST_THROWS(mapnik::geometry_utils::from_wkb((const char*)sq_invalid_blob,
+        geom = mapnik::geometry_utils::from_wkb((const char*)sq_invalid_blob,
                                                 sizeof(sq_invalid_blob) / sizeof(sq_invalid_blob[0]),
-                                                           mapnik::wkbGeneric), std::exception);
+                                                mapnik::wkbGeneric);
+        BOOST_TEST(geom.is<mapnik::geometry::geometry_empty>()); // returns geometry_empty
 
     } catch (std::exception const& ex) {
         BOOST_TEST(false);