Update Changelog for 5.14.2

CHANGELOG.md

@@ -1,3 +1,10 @@
+# 5.14.2
+ - Changes from 5.14.1:
+   - Bugfixes:
+      - FIXED #4727: Erroring when a old .core file is present.
+      - FIXED #4642: Update checks for EMPTY_NAMEID to check for empty name strings
+      - FIXED #4738: Fix potential segmentation fault
+
 # 5.14.1
   - Changes from 5.14.0
     - Bugfixes:

CMakeLists.txt

@@ -62,7 +62,7 @@ endif()
 project(OSRM C CXX)
 set(OSRM_VERSION_MAJOR 5)
 set(OSRM_VERSION_MINOR 14)
-set(OSRM_VERSION_PATCH 1)
+set(OSRM_VERSION_PATCH 2)
 set(OSRM_VERSION "${OSRM_VERSION_MAJOR}.${OSRM_VERSION_MINOR}.${OSRM_VERSION_PATCH}")
 
 add_definitions(-DOSRM_PROJECT_DIR="${CMAKE_CURRENT_SOURCE_DIR}")

features/guidance/turn.feature

@@ -1372,3 +1372,64 @@ Feature: Simple Turns
             | waypoints | route        | turns                           |
             | a,d       | ab,bcd,bcd   | depart,fork slight right,arrive |
             | a,g       | ab,befg,befg | depart,fork slight left,arrive  |
+
+    # https://www.openstreetmap.org/#map=18/52.25130/10.42545
+    Scenario: Turn for roads with no name, ref changes
+        Given the node map
+            """
+              d
+              .
+              .
+            e c . . f
+              .
+              .
+              b
+              .
+              .
+              a
+            """
+
+        And the ways
+            | nodes | highway     | ref  | name          |
+            | abc   | tertiary    | K 57 |               |
+            | cd    | tertiary    | K 56 |               |
+            | cf    | tertiary    | K 56 |               |
+            | ce    | residential |      | Heinrichshöhe |
+
+       When I route I should get
+            | waypoints | route     | turns                    |
+            | a,f       | ,,        | depart,turn right,arrive |
+
+    # https://www.openstreetmap.org/#map=18/52.24071/10.29066
+    Scenario: Turn for roads with no name, ref changes
+        Given the node map
+            """
+              x
+              .
+              .
+              d
+             . .
+            .   .
+           .     .
+     e. . t . c . p. .f
+           .     .
+            .   .
+             . .
+              b
+              .
+              .
+              a
+            """
+
+        And the ways
+            | nodes | highway     | ref  | name          | oneway |
+            | abp   | tertiary    | K 23 |               | yes    |
+            | pdx   | tertiary    | K 23 |               | yes    |
+            | xdt   | tertiary    | K 23 |               | yes    |
+            | tba   | tertiary    | K 23 |               | yes    |
+            | etcpf | primary     | B 1  |               | no     |
+
+       When I route I should get
+            | waypoints | route     | turns                                    |
+            | e,x       | ,,,       | depart,turn sharp left,turn right,arrive |
+            | f,a       | ,,        | depart,turn left,arrive                  |

include/contractor/contracted_edge_container.hpp

@@ -108,7 +108,7 @@ struct ContractedEdgeContainer
         edges.insert(edges.end(), new_edges.begin(), new_end);
         auto edges_size = edges.size();
         auto new_edges_size = std::distance(new_edges.begin(), new_end);
-        BOOST_ASSERT(edges_size >= new_edges_size);
+        BOOST_ASSERT(static_cast<int>(edges_size) >= new_edges_size);
         flags.resize(edges_size);
         std::fill(flags.begin() + edges_size - new_edges_size, flags.end(), flag);
 

include/storage/shared_datatype.hpp

@@ -48,14 +48,6 @@ const constexpr char *block_id_to_name[] = {"NAME_CHAR_DATA",
                                             "HSGR_CHECKSUM",
                                             "TIMESTAMP",
                                             "FILE_INDEX_PATH",
-                                            "CH_CORE_MARKER_0",
-                                            "CH_CORE_MARKER_1",
-                                            "CH_CORE_MARKER_2",
-                                            "CH_CORE_MARKER_3",
-                                            "CH_CORE_MARKER_4",
-                                            "CH_CORE_MARKER_5",
-                                            "CH_CORE_MARKER_6",
-                                            "CH_CORE_MARKER_7",
                                             "DATASOURCES_NAMES",
                                             "PROPERTIES",
                                             "BEARING_CLASSID",
@@ -132,14 +124,6 @@ struct DataLayout
         HSGR_CHECKSUM,
         TIMESTAMP,
         FILE_INDEX_PATH,
-        CH_CORE_MARKER_0,
-        CH_CORE_MARKER_1,
-        CH_CORE_MARKER_2,
-        CH_CORE_MARKER_3,
-        CH_CORE_MARKER_4,
-        CH_CORE_MARKER_5,
-        CH_CORE_MARKER_6,
-        CH_CORE_MARKER_7,
         DATASOURCES_NAMES,
         PROPERTIES,
         BEARING_CLASSID,
@@ -200,15 +184,7 @@ struct DataLayout
 
     inline uint64_t GetBlockEntries(BlockID bid) const { return num_entries[bid]; }
 
-    inline uint64_t GetBlockSize(BlockID bid) const
+    inline uint64_t GetBlockSize(BlockID bid) const { return num_entries[bid] * entry_size[bid]; }
-    {
-        // special bit encoding
-        if (bid >= CH_CORE_MARKER_0 && bid <= CH_CORE_MARKER_7)
-        {
-            return (num_entries[bid] / 32 + 1) * entry_size[bid];
-        }
-        return num_entries[bid] * entry_size[bid];
-    }
 
     inline uint64_t GetSizeOfLayout() const
     {

include/storage/storage_config.hpp

@@ -64,7 +64,6 @@ struct StorageConfig final : IOConfig
                    {".osrm.hsgr",
                     ".osrm.nbg_nodes",
                     ".osrm.ebg_nodes",
-                    ".osrm.core",
                     ".osrm.cells",
                     ".osrm.cell_metrics",
                     ".osrm.mldgr",

include/util/dynamic_graph.hpp

@@ -115,7 +115,7 @@ template <typename EdgeDataT> class DynamicGraph
 
         number_of_nodes = nodes;
         number_of_edges = static_cast<EdgeIterator>(graph.size());
-        node_array.resize(number_of_nodes + 1);
+        node_array.resize(number_of_nodes);
         EdgeIterator edge = 0;
         EdgeIterator position = 0;
         for (const auto node : irange(0u, number_of_nodes))
@@ -129,7 +129,6 @@ template <typename EdgeDataT> class DynamicGraph
             node_array[node].edges = edge - last_edge;
             position += node_array[node].edges;
         }
-        node_array.back().first_edge = position;
         edge_list.reserve(static_cast<std::size_t>(edge_list.size() * 1.1));
         edge_list.resize(position);
         edge = 0;
@@ -144,6 +143,8 @@ template <typename EdgeDataT> class DynamicGraph
                 ++edge;
             }
         }
+
+        BOOST_ASSERT(node_array.size() == number_of_nodes);
     }
 
     // Copy&move for the same data
@@ -191,6 +192,8 @@ template <typename EdgeDataT> class DynamicGraph
     // Removes all edges to and from nodes for which filter(node_id) returns false
     template <typename Pred> auto Filter(Pred filter) const &
     {
+        BOOST_ASSERT(node_array.size() == number_of_nodes);
+
         DynamicGraph other;
 
         other.number_of_nodes = number_of_nodes;
@@ -202,6 +205,8 @@ template <typename EdgeDataT> class DynamicGraph
         std::transform(
             node_array.begin(), node_array.end(), other.node_array.begin(), [&](const Node &node) {
                 const EdgeIterator first_edge = other.edge_list.size();
+
+                BOOST_ASSERT(node_id < number_of_nodes);
                 if (filter(node_id++))
                 {
                     std::copy_if(edge_list.begin() + node.first_edge,
@@ -416,7 +421,7 @@ template <typename EdgeDataT> class DynamicGraph
     void Renumber(const std::vector<NodeID> &old_to_new_node)
     {
         // permutate everything but the sentinel
-        util::inplacePermutation(node_array.begin(), std::prev(node_array.end()), old_to_new_node);
+        util::inplacePermutation(node_array.begin(), node_array.end(), old_to_new_node);
 
         // Build up edge permutation
         auto new_edge_index = 0;

package.json

@@ -1,6 +1,6 @@
 {
   "name": "osrm",
-  "version": "5.14.1",
+  "version": "5.14.2",
   "private": false,
   "description": "The Open Source Routing Machine is a high performance routing engine written in C++14 designed to run on OpenStreetMap data.",
   "dependencies": {

src/contractor/graph_contractor.cpp

@@ -384,7 +384,7 @@ void RenumberData(std::vector<RemainingNodeData> &remaining_nodes,
     // we need to make a copy here because we are going to modify it
     auto to_orig = new_to_old_node_id;
 
-    auto new_node_id = 0;
+    auto new_node_id = 0u;
 
     // All remaining nodes get the low IDs
     for (auto &remaining : remaining_nodes)

src/engine/guidance/collapse_scenario_detection.cpp

@@ -41,15 +41,14 @@ bool noIntermediaryIntersections(const RouteStep &step)
 }
 
 // Link roads, as far as we are concerned, are short unnamed segments between to named segments.
-bool isLinkroad(const RouteStep &pre_link_step,
+bool isLinkRoad(const RouteStep &pre_link_step,
                 const RouteStep &link_step,
                 const RouteStep &post_link_step)
 {
     const constexpr double MAX_LINK_ROAD_LENGTH = 2 * MAX_COLLAPSE_DISTANCE;
     const auto is_short = link_step.distance <= MAX_LINK_ROAD_LENGTH;
-    const auto unnamed = link_step.name_id == EMPTY_NAMEID;
+    const auto unnamed = link_step.name.empty();
-    const auto between_named =
+    const auto between_named = !pre_link_step.name.empty() && !post_link_step.name.empty();
-        (pre_link_step.name_id != EMPTY_NAMEID) && (post_link_step.name_id != EMPTY_NAMEID);
 
     return is_short && unnamed && between_named && noIntermediaryIntersections(link_step);
 }
@@ -196,7 +195,7 @@ bool isUTurn(const RouteStepIterator step_prior_to_intersection,
     const auto only_allowed_turn = (numberOfAllowedTurns(*step_leaving_intersection) == 1) &&
                                    noIntermediaryIntersections(*step_entering_intersection);
 
-    return collapsable || isLinkroad(*step_prior_to_intersection,
+    return collapsable || isLinkRoad(*step_prior_to_intersection,
                                      *step_entering_intersection,
                                      *step_leaving_intersection) ||
            only_allowed_turn;

src/extractor/guidance/intersection_handler.cpp

@@ -71,17 +71,19 @@ TurnType::Enum IntersectionHandler::findBasicTurnType(const EdgeID via_edge,
     if (!on_ramp && onto_ramp)
         return TurnType::OnRamp;
 
-    const auto &in_name =
+    const auto &in_name_id =
         node_data_container.GetAnnotation(node_based_graph.GetEdgeData(via_edge).annotation_data)
             .name_id;
-    const auto &out_name =
+    const auto &out_name_id =
         node_data_container.GetAnnotation(node_based_graph.GetEdgeData(road.eid).annotation_data)
             .name_id;
+    const auto &in_name_empty = name_table.GetNameForID(in_name_id).empty();
+    const auto &out_name_empty = name_table.GetNameForID(out_name_id).empty();
 
     const auto same_name = !util::guidance::requiresNameAnnounced(
-        in_name, out_name, name_table, street_name_suffix_table);
+        in_name_id, out_name_id, name_table, street_name_suffix_table);
 
-    if (in_name != EMPTY_NAMEID && out_name != EMPTY_NAMEID && same_name)
+    if (!in_name_empty && !out_name_empty && same_name)
     {
         return TurnType::Continue;
     }
@@ -488,8 +490,8 @@ bool IntersectionHandler::isSameName(const EdgeID source_edge_id, const EdgeID t
     const auto &target_edge_data = node_data_container.GetAnnotation(
         node_based_graph.GetEdgeData(target_edge_id).annotation_data);
 
-    return source_edge_data.name_id != EMPTY_NAMEID && //
+    return !name_table.GetNameForID(source_edge_data.name_id).empty() && //
-           target_edge_data.name_id != EMPTY_NAMEID && //
+           !name_table.GetNameForID(target_edge_data.name_id).empty() && //
            !util::guidance::requiresNameAnnounced(source_edge_data.name_id,
                                                   target_edge_data.name_id,
                                                   name_table,

src/extractor/guidance/mergable_road_detector.cpp

@@ -23,6 +23,8 @@ namespace guidance
 namespace
 {
 // check a connected road for equality of a name
+// returns 'true' if no equality because this is used as a filter elsewhere, i.e. filter if fn
+// returns 'true'
 inline auto makeCheckRoadForName(const NameID name_id,
                                  const util::NodeBasedDynamicGraph &node_based_graph,
                                  const EdgeBasedNodeDataContainer &node_data_container,
@@ -32,15 +34,18 @@ inline auto makeCheckRoadForName(const NameID name_id,
     return [name_id, &node_based_graph, &node_data_container, &name_table, &suffix_table](
         const MergableRoadDetector::MergableRoadData &road) {
         // since we filter here, we don't want any other name than the one we are looking for
-        const auto road_name =
+        const auto road_name_id =
             node_data_container
                 .GetAnnotation(node_based_graph.GetEdgeData(road.eid).annotation_data)
                 .name_id;
-        if (name_id == EMPTY_NAMEID || road_name == EMPTY_NAMEID)
+        const auto road_name_empty = name_table.GetNameForID(road_name_id).empty();
+        const auto in_name_empty = name_table.GetNameForID(name_id).empty();
+        if (in_name_empty || road_name_empty)
             return true;
         const auto requires_announcement =
-            util::guidance::requiresNameAnnounced(name_id, road_name, name_table, suffix_table) ||
+            util::guidance::requiresNameAnnounced(
-            util::guidance::requiresNameAnnounced(road_name, name_id, name_table, suffix_table);
+                name_id, road_name_id, name_table, suffix_table) ||
+            util::guidance::requiresNameAnnounced(road_name_id, name_id, name_table, suffix_table);
 
         return requires_announcement;
     };
@@ -465,16 +470,18 @@ bool MergableRoadDetector::IsTrafficIsland(const NodeID intersection_node,
                 .name_id;
 
         const auto has_required_name = [this, required_name_id](const auto edge_id) {
-            const auto road_name =
+            const auto road_name_id =
                 node_data_container
                     .GetAnnotation(node_based_graph.GetEdgeData(edge_id).annotation_data)
                     .name_id;
-            if (required_name_id == EMPTY_NAMEID || road_name == EMPTY_NAMEID)
+            const auto &road_name_empty = name_table.GetNameForID(road_name_id).empty();
+            const auto &required_name_empty = name_table.GetNameForID(required_name_id).empty();
+            if (required_name_empty && road_name_empty)
                 return false;
             return !util::guidance::requiresNameAnnounced(
-                       required_name_id, road_name, name_table, street_name_suffix_table) ||
+                       required_name_id, road_name_id, name_table, street_name_suffix_table) ||
                    !util::guidance::requiresNameAnnounced(
-                       road_name, required_name_id, name_table, street_name_suffix_table);
+                       road_name_id, required_name_id, name_table, street_name_suffix_table);
         };
 
         /* the beautiful way would be:

src/extractor/guidance/motorway_handler.cpp

@@ -379,11 +379,15 @@ Intersection MotorwayHandler::fromRamp(const EdgeID via_eid, Intersection inters
             //
             //     7         1
             //          0
+            const auto &first_intersection_name_empty =
+                name_table.GetNameForID(first_intersection_data.name_id).empty();
+            const auto &second_intersection_name_empty =
+                name_table.GetNameForID(second_intersection_data.name_id).empty();
             if (intersection[1].entry_allowed)
             {
                 if (isMotorwayClass(intersection[1].eid, node_based_graph) &&
-                    second_intersection_data.name_id != EMPTY_NAMEID &&
+                    !second_intersection_name_empty && !first_intersection_name_empty &&
-                    first_intersection_data.name_id != EMPTY_NAMEID && first_second_same_name)
+                    first_second_same_name)
                 {
                     // circular order indicates a merge to the left (0-3 onto 4
                     if (angularDeviation(intersection[1].angle, STRAIGHT_ANGLE) <
@@ -407,8 +411,8 @@ Intersection MotorwayHandler::fromRamp(const EdgeID via_eid, Intersection inters
             {
                 BOOST_ASSERT(intersection[2].entry_allowed);
                 if (isMotorwayClass(intersection[2].eid, node_based_graph) &&
-                    second_intersection_data.name_id != EMPTY_NAMEID &&
+                    !second_intersection_name_empty && !first_intersection_name_empty &&
-                    first_intersection_data.name_id != EMPTY_NAMEID && first_second_same_name)
+                    first_second_same_name)
                 {
                     // circular order (5-0) onto 4
                     if (angularDeviation(intersection[2].angle, STRAIGHT_ANGLE) <

src/extractor/guidance/roundabout_handler.cpp

@@ -295,7 +295,8 @@ RoundaboutType RoundaboutHandler::getRoundaboutType(const NodeID nid) const
                     return SPECIAL_EDGEID;
                 }
 
-                if (EMPTY_NAMEID != edge_data.name_id)
+                const auto &edge_name_empty = name_table.GetNameForID(edge_data.name_id).empty();
+                if (!edge_name_empty)
                 {
 
                     const auto announce = [&](unsigned id) {
@@ -306,7 +307,6 @@ RoundaboutType RoundaboutHandler::getRoundaboutType(const NodeID nid) const
                     if (std::all_of(begin(roundabout_name_ids), end(roundabout_name_ids), announce))
                         roundabout_name_ids.insert(edge_data.name_id);
                 }
-
                 continue_edge = edge_id;
             }
             else if (!edge.flags.roundabout && !edge.flags.circular)

src/extractor/guidance/sliproad_handler.cpp

@@ -1,5 +1,6 @@
 #include "extractor/guidance/sliproad_handler.hpp"
 #include "extractor/guidance/constants.hpp"
+#include "util/assert.hpp"
 #include "util/bearing.hpp"
 #include "util/coordinate_calculation.hpp"
 #include "util/guidance/name_announcements.hpp"
@@ -474,7 +475,7 @@ operator()(const NodeID /*nid*/, const EdgeID source_edge_id, Intersection inter
 
             // Name mismatch: check roads at `c` and `d` for same name
             const auto name_mismatch = [&](const NameID road_name_id) {
-                const auto unnamed = road_name_id == EMPTY_NAMEID;
+                const auto unnamed = name_table.GetNameForID(road_name_id).empty();
 
                 return unnamed ||
                        util::guidance::requiresNameAnnounced(road_name_id,              //
@@ -499,11 +500,15 @@ operator()(const NodeID /*nid*/, const EdgeID source_edge_id, Intersection inter
                 node_data_container
                     .GetAnnotation(node_based_graph.GetEdgeData(main_road.eid).annotation_data)
                     .name_id;
+            const auto main_road_name_empty = name_table.GetNameForID(main_road_name_id).empty();
             const auto &sliproad_annotation =
                 node_data_container.GetAnnotation(sliproad_edge_data.annotation_data);
+            const auto sliproad_name_empty =
+                name_table.GetNameForID(sliproad_annotation.name_id).empty();
+            const auto candidate_road_name_empty =
+                name_table.GetNameForID(candidate_data.name_id).empty();
             if (!sliproad_edge_data.flags.road_classification.IsLinkClass() &&
-                sliproad_annotation.name_id != EMPTY_NAMEID && main_road_name_id != EMPTY_NAMEID &&
+                !sliproad_name_empty && !main_road_name_empty && !candidate_road_name_empty &&
-                candidate_data.name_id != EMPTY_NAMEID &&
                 util::guidance::requiresNameAnnounced(main_road_name_id,
                                                       sliproad_annotation.name_id,
                                                       name_table,
@@ -575,8 +580,9 @@ operator()(const NodeID /*nid*/, const EdgeID source_edge_id, Intersection inter
             intersection[*obvious].instruction.direction_modifier =
                 getTurnDirection(intersection[*obvious].angle);
         }
-        else if (main_annotation.name_id != EMPTY_NAMEID)
+        else if (!name_table.GetNameForID(main_annotation.name_id).empty())
         {
+            OSRM_ASSERT(false, coordinates[intersection_node_id]);
             intersection[*obvious].instruction.type = TurnType::NewName;
             intersection[*obvious].instruction.direction_modifier =
                 getTurnDirection(intersection[*obvious].angle);

src/extractor/guidance/turn_handler.cpp

@@ -199,7 +199,8 @@ bool TurnHandler::isObviousOfTwo(const EdgeID via_edge,
 
     const bool turn_is_perfectly_straight =
         angularDeviation(road.angle, STRAIGHT_ANGLE) < std::numeric_limits<double>::epsilon();
-    if (via_data.name_id != EMPTY_NAMEID)
+    const auto &via_name_empty = name_table.GetNameForID(via_data.name_id).empty();
+    if (!via_name_empty)
     {
         const auto same_name = !util::guidance::requiresNameAnnounced(
             via_data.name_id, road_data.name_id, name_table, street_name_suffix_table);

src/storage/storage.cpp

@@ -331,43 +331,6 @@ void Storage::PopulateLayout(DataLayout &layout)
         layout.SetBlockSize<char>(DataLayout::TIMESTAMP, timestamp_size);
     }
 
-    // load core marker size
-    if (boost::filesystem::exists(config.GetPath(".osrm.core")))
-    {
-        io::FileReader core_marker_file(config.GetPath(".osrm.core"),
-                                        io::FileReader::VerifyFingerprint);
-        const auto num_metrics = core_marker_file.ReadElementCount64();
-        if (num_metrics > NUM_METRICS)
-        {
-            throw util::exception("Only " + std::to_string(NUM_METRICS) +
-                                  " metrics are supported at the same time.");
-        }
-
-        const auto number_of_core_markers = core_marker_file.ReadElementCount64();
-        for (const auto index : util::irange<std::size_t>(0, num_metrics))
-        {
-            layout.SetBlockSize<unsigned>(
-                static_cast<DataLayout::BlockID>(DataLayout::CH_CORE_MARKER_0 + index),
-                number_of_core_markers);
-        }
-        for (const auto index : util::irange<std::size_t>(num_metrics, NUM_METRICS))
-        {
-            layout.SetBlockSize<unsigned>(
-                static_cast<DataLayout::BlockID>(DataLayout::CH_CORE_MARKER_0 + index), 0);
-        }
-    }
-    else
-    {
-        layout.SetBlockSize<unsigned>(DataLayout::CH_CORE_MARKER_0, 0);
-        layout.SetBlockSize<unsigned>(DataLayout::CH_CORE_MARKER_1, 0);
-        layout.SetBlockSize<unsigned>(DataLayout::CH_CORE_MARKER_2, 0);
-        layout.SetBlockSize<unsigned>(DataLayout::CH_CORE_MARKER_3, 0);
-        layout.SetBlockSize<unsigned>(DataLayout::CH_CORE_MARKER_4, 0);
-        layout.SetBlockSize<unsigned>(DataLayout::CH_CORE_MARKER_5, 0);
-        layout.SetBlockSize<unsigned>(DataLayout::CH_CORE_MARKER_6, 0);
-        layout.SetBlockSize<unsigned>(DataLayout::CH_CORE_MARKER_7, 0);
-    }
-
     // load turn weight penalties
     {
         io::FileReader turn_weight_penalties_file(config.GetPath(".osrm.turn_weight_penalties"),