Fix fingerprint for names file on 5.7

CHANGELOG.md

@@ -1,3 +1,8 @@
+# 5.7.3
+  - Changes from 5.7.2:
+   - Bug fixes:
+     - Fixes 4097: .ramIndex files was able to be truncated siliently
+
 # 5.7.2
   - Changes from 5.7.1:
    - Bug fixes:

CMakeLists.txt

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

include/storage/io.hpp

@@ -228,7 +228,7 @@ class FileWriter
 
     template <typename T> void WriteFrom(const T &src) { WriteFrom(&src, 1); }
 
-    template <typename T> void WriteOne(const T tmp) { WriteFrom(tmp); }
+    template <typename T> void WriteOne(const T &tmp) { WriteFrom(tmp); }
 
     void WriteElementCount32(const std::uint32_t count) { WriteOne<std::uint32_t>(count); }
     void WriteElementCount64(const std::uint64_t count) { WriteOne<std::uint64_t>(count); }

include/util/indexed_data.hpp

@@ -1,6 +1,8 @@
 #ifndef OSRM_INDEXED_DATA_HPP
 #define OSRM_INDEXED_DATA_HPP
 
+#include "storage/io.hpp"
+
 #include "util/exception.hpp"
 #include "util/string_view.hpp"
 
@@ -9,7 +11,6 @@
 #include <array>
 #include <iterator>
 #include <limits>
-#include <ostream>
 #include <string>
 #include <type_traits>
 
@@ -85,7 +86,7 @@ template <int N, typename T = std::string> struct VariableGroupBlock
     /// prefix length.
     /// Returns the block prefix length.
     template <typename Offset, typename OffsetIterator>
-    Offset WriteBlockReference(std::ostream &out,
+    Offset WriteBlockReference(storage::io::FileWriter &out,
                                Offset data_offset,
                                OffsetIterator first,
                                OffsetIterator last) const
@@ -105,7 +106,7 @@ template <int N, typename T = std::string> struct VariableGroupBlock
             prefix_length += byte_length;
         }
 
-        out.write((const char *)&refernce, sizeof(refernce));
+        out.WriteOne(refernce);
 
         return prefix_length;
     }
@@ -118,7 +119,8 @@ template <int N, typename T = std::string> struct VariableGroupBlock
     /// [first..last] is an inclusive range of block data.
     /// The length of the last item in the block is not stored.
     template <typename OffsetIterator>
-    void WriteBlockPrefix(std::ostream &out, OffsetIterator first, OffsetIterator last) const
+    void
+    WriteBlockPrefix(storage::io::FileWriter &out, OffsetIterator first, OffsetIterator last) const
     {
         for (OffsetIterator curr = first, next = std::next(first); curr != last; ++curr, ++next)
         {
@@ -127,7 +129,9 @@ template <int N, typename T = std::string> struct VariableGroupBlock
             if (byte_length == 0)
                 continue;
 
-            out.write((const char *)&data_length, byte_length);
+            // Here, we're only writing a few bytes from the 4-byte std::uint32_t,
+            // so we need to cast to (char *)
+            out.WriteFrom((const char *)&data_length, byte_length);
         }
     }
 
@@ -175,20 +179,23 @@ template <int N, typename T = std::string> struct FixedGroupBlock
     /// Write a block reference {offset}, where offset is a global block offset
     /// Returns the fixed block prefix length.
     template <typename Offset, typename OffsetIterator>
-    Offset
-    WriteBlockReference(std::ostream &out, Offset data_offset, OffsetIterator, OffsetIterator) const
+    Offset WriteBlockReference(storage::io::FileWriter &out,
+                               Offset data_offset,
+                               OffsetIterator,
+                               OffsetIterator) const
     {
         BOOST_ASSERT(data_offset <= std::numeric_limits<decltype(BlockReference::offset)>::max());
 
         BlockReference refernce{static_cast<decltype(BlockReference::offset)>(data_offset)};
-        out.write((const char *)&refernce, sizeof(refernce));
+        out.WriteOne(refernce);
 
         return BLOCK_SIZE;
     }
 
     /// Write a fixed length block prefix.
     template <typename OffsetIterator>
-    void WriteBlockPrefix(std::ostream &out, OffsetIterator first, OffsetIterator last) const
+    void
+    WriteBlockPrefix(storage::io::FileWriter &out, OffsetIterator first, OffsetIterator last) const
     {
         std::uint32_t index = 0;
         std::array<ValueType, BLOCK_SIZE> block_prefix;
@@ -200,7 +207,7 @@ template <int N, typename T = std::string> struct FixedGroupBlock
 
             block_prefix[index++] = static_cast<ValueType>(data_length);
         }
-        out.write((const char *)block_prefix.data(), block_prefix.size());
+        out.WriteFrom(block_prefix.data(), block_prefix.size());
     }
 
     /// Advances the range to an item stored in the referenced block.
@@ -244,8 +251,10 @@ template <typename GroupBlock> struct IndexedData
     bool empty() const { return blocks_number == 0; }
 
     template <typename OffsetIterator, typename DataIterator>
-    void
-    write(std::ostream &out, OffsetIterator first, OffsetIterator last, DataIterator data) const
+    void write(storage::io::FileWriter &out,
+               OffsetIterator first,
+               OffsetIterator last,
+               DataIterator data) const
     {
         static_assert(sizeof(typename DataIterator::value_type) == 1, "data basic type must char");
 
@@ -259,7 +268,7 @@ template <typename GroupBlock> struct IndexedData
         const BlocksNumberType number_of_blocks =
             number_of_elements == 0 ? 0
                                     : 1 + (std::distance(first, sentinel) - 1) / (BLOCK_SIZE + 1);
-        out.write((const char *)&number_of_blocks, sizeof(number_of_blocks));
+        out.WriteOne(number_of_blocks);
 
         // Write block references and compute the total data size that includes prefix and data
         const GroupBlock block;
@@ -273,7 +282,7 @@ template <typename GroupBlock> struct IndexedData
         }
 
         // Write the total data size
-        out.write((const char *)&data_size, sizeof(data_size));
+        out.WriteOne(data_size);
 
         // Write data blocks that are (prefix, data)
         for (OffsetIterator curr = first, next = first; next != sentinel; curr = next)
@@ -281,7 +290,8 @@ template <typename GroupBlock> struct IndexedData
             std::advance(next, std::min<diff_type>(BLOCK_SIZE, std::distance(next, sentinel)));
             block.WriteBlockPrefix(out, curr, next);
             std::advance(next, std::min<diff_type>(1, std::distance(next, sentinel)));
-            std::copy(data + *curr, data + *next, std::ostream_iterator<unsigned char>(out));
+            std::for_each(
+                data + *curr, data + *next, [&out](const auto &element) { out.WriteOne(element); });
         }
     }
 

include/util/static_rtree.hpp

@@ -19,7 +19,6 @@
 
 #include <boost/assert.hpp>
 #include <boost/filesystem.hpp>
-#include <boost/filesystem/fstream.hpp>
 #include <boost/format.hpp>
 #include <boost/iostreams/device/mapped_file.hpp>
 
@@ -204,73 +203,78 @@ class StaticRTree
                 }
             });
 
-        // open leaf file
-        boost::filesystem::ofstream leaf_node_file(leaf_node_filename, std::ios::binary);
-
+        std::vector<TreeNode> tree_nodes_in_level;
         // sort the hilbert-value representatives
         tbb::parallel_sort(input_wrapper_vector.begin(), input_wrapper_vector.end());
-        std::vector<TreeNode> tree_nodes_in_level;
 
-        // pack M elements into leaf node, write to leaf file and add child index to the parent node
-        uint64_t wrapped_element_index = 0;
-        for (std::uint32_t node_index = 0; wrapped_element_index < element_count; ++node_index)
         {
-            TreeNode current_node;
-            for (std::uint32_t leaf_index = 0;
-                 leaf_index < BRANCHING_FACTOR && wrapped_element_index < element_count;
-                 ++leaf_index)
+            storage::io::FileWriter leaf_node_file(leaf_node_filename,
+                                                   storage::io::FileWriter::HasNoFingerprint);
+
+            // pack M elements into leaf node, write to leaf file and add child index to the parent
+            // node
+            uint64_t wrapped_element_index = 0;
+            for (std::uint32_t node_index = 0; wrapped_element_index < element_count; ++node_index)
             {
-                LeafNode current_leaf;
-                Rectangle &rectangle = current_leaf.minimum_bounding_rectangle;
-                for (std::uint32_t object_index = 0;
-                     object_index < LEAF_NODE_SIZE && wrapped_element_index < element_count;
-                     ++object_index, ++wrapped_element_index)
+                TreeNode current_node;
+                for (std::uint32_t leaf_index = 0;
+                     leaf_index < BRANCHING_FACTOR && wrapped_element_index < element_count;
+                     ++leaf_index)
                 {
-                    const std::uint32_t input_object_index =
-                        input_wrapper_vector[wrapped_element_index].m_array_index;
-                    const EdgeDataT &object = input_data_vector[input_object_index];
+                    LeafNode current_leaf;
+                    Rectangle &rectangle = current_leaf.minimum_bounding_rectangle;
+                    for (std::uint32_t object_index = 0;
+                         object_index < LEAF_NODE_SIZE && wrapped_element_index < element_count;
+                         ++object_index, ++wrapped_element_index)
+                    {
+                        const std::uint32_t input_object_index =
+                            input_wrapper_vector[wrapped_element_index].m_array_index;
+                        const EdgeDataT &object = input_data_vector[input_object_index];
 
-                    current_leaf.object_count += 1;
-                    current_leaf.objects[object_index] = object;
+                        current_leaf.object_count += 1;
+                        current_leaf.objects[object_index] = object;
 
-                    Coordinate projected_u{
-                        web_mercator::fromWGS84(Coordinate{m_coordinate_list[object.u]})};
-                    Coordinate projected_v{
-                        web_mercator::fromWGS84(Coordinate{m_coordinate_list[object.v]})};
+                        Coordinate projected_u{
+                            web_mercator::fromWGS84(Coordinate{m_coordinate_list[object.u]})};
+                        Coordinate projected_v{
+                            web_mercator::fromWGS84(Coordinate{m_coordinate_list[object.v]})};
 
-                    BOOST_ASSERT(std::abs(toFloating(projected_u.lon).operator double()) <= 180.);
-                    BOOST_ASSERT(std::abs(toFloating(projected_u.lat).operator double()) <= 180.);
-                    BOOST_ASSERT(std::abs(toFloating(projected_v.lon).operator double()) <= 180.);
-                    BOOST_ASSERT(std::abs(toFloating(projected_v.lat).operator double()) <= 180.);
+                        BOOST_ASSERT(std::abs(toFloating(projected_u.lon).operator double()) <=
+                                     180.);
+                        BOOST_ASSERT(std::abs(toFloating(projected_u.lat).operator double()) <=
+                                     180.);
+                        BOOST_ASSERT(std::abs(toFloating(projected_v.lon).operator double()) <=
+                                     180.);
+                        BOOST_ASSERT(std::abs(toFloating(projected_v.lat).operator double()) <=
+                                     180.);
 
-                    rectangle.min_lon =
-                        std::min(rectangle.min_lon, std::min(projected_u.lon, projected_v.lon));
-                    rectangle.max_lon =
-                        std::max(rectangle.max_lon, std::max(projected_u.lon, projected_v.lon));
+                        rectangle.min_lon =
+                            std::min(rectangle.min_lon, std::min(projected_u.lon, projected_v.lon));
+                        rectangle.max_lon =
+                            std::max(rectangle.max_lon, std::max(projected_u.lon, projected_v.lon));
 
-                    rectangle.min_lat =
-                        std::min(rectangle.min_lat, std::min(projected_u.lat, projected_v.lat));
-                    rectangle.max_lat =
-                        std::max(rectangle.max_lat, std::max(projected_u.lat, projected_v.lat));
+                        rectangle.min_lat =
+                            std::min(rectangle.min_lat, std::min(projected_u.lat, projected_v.lat));
+                        rectangle.max_lat =
+                            std::max(rectangle.max_lat, std::max(projected_u.lat, projected_v.lat));
 
-                    BOOST_ASSERT(rectangle.IsValid());
+                        BOOST_ASSERT(rectangle.IsValid());
+                    }
+
+                    // append the leaf node to the current tree node
+                    current_node.child_count += 1;
+                    current_node.children[leaf_index] =
+                        TreeIndex{node_index * BRANCHING_FACTOR + leaf_index, true};
+                    current_node.minimum_bounding_rectangle.MergeBoundingBoxes(
+                        current_leaf.minimum_bounding_rectangle);
+
+                    // write leaf_node to leaf node file
+                    leaf_node_file.WriteOne(current_leaf);
                 }
 
-                // append the leaf node to the current tree node
-                current_node.child_count += 1;
-                current_node.children[leaf_index] =
-                    TreeIndex{node_index * BRANCHING_FACTOR + leaf_index, true};
-                current_node.minimum_bounding_rectangle.MergeBoundingBoxes(
-                    current_leaf.minimum_bounding_rectangle);
-
-                // write leaf_node to leaf node file
-                leaf_node_file.write((char *)&current_leaf, sizeof(current_leaf));
+                tree_nodes_in_level.emplace_back(current_node);
             }
-
-            tree_nodes_in_level.emplace_back(current_node);
         }
-        leaf_node_file.flush();
-        leaf_node_file.close();
 
         std::uint32_t processing_level = 0;
         while (1 < tree_nodes_in_level.size())
@@ -332,15 +336,17 @@ class StaticRTree
                 }
             });
 
-        // open tree file
-        storage::io::FileWriter tree_node_file(tree_node_filename,
-                                               storage::io::FileWriter::HasNoFingerprint);
+        {
+            // open tree file
+            storage::io::FileWriter tree_node_file(tree_node_filename,
+                                                   storage::io::FileWriter::HasNoFingerprint);
 
-        std::uint64_t size_of_tree = m_search_tree.size();
-        BOOST_ASSERT_MSG(0 < size_of_tree, "tree empty");
+            std::uint64_t size_of_tree = m_search_tree.size();
+            BOOST_ASSERT_MSG(0 < size_of_tree, "tree empty");
 
-        tree_node_file.WriteOne(size_of_tree);
-        tree_node_file.WriteFrom(&m_search_tree[0], size_of_tree);
+            tree_node_file.WriteOne(size_of_tree);
+            tree_node_file.WriteFrom(&m_search_tree[0], size_of_tree);
+        }
 
         MapLeafNodesFile(leaf_node_filename);
     }
@@ -379,6 +385,24 @@ class StaticRTree
             std::size_t num_leaves = m_leaves_region.size() / sizeof(LeafNode);
             auto data_ptr = m_leaves_region.data();
             BOOST_ASSERT(reinterpret_cast<uintptr_t>(data_ptr) % alignof(LeafNode) == 0);
+#ifndef NDEBUG
+            // Find the maximum leaf node ID and make sure we have that many from our file
+            BOOST_ASSERT(m_search_tree.size() > 0);
+            std::uint32_t max_leaf_node_id = 0;
+            // Search in reverse, the bottom of the tree is at the end of the array
+            for (auto iter = m_search_tree.rbegin(); iter != m_search_tree.rend(); iter++)
+            {
+                // If we find a non-leaf node, we can quit, we've seen the
+                // bottom level of the tree
+                if (iter->child_count > 0 && !iter->children[0].is_leaf)
+                    break;
+                for (std::uint32_t i = 0; i < iter->child_count; ++i)
+                {
+                    max_leaf_node_id = std::max(max_leaf_node_id, iter->children[i].index);
+                }
+            }
+            BOOST_ASSERT(max_leaf_node_id == num_leaves - 1);
+#endif
             m_leaves.reset(reinterpret_cast<const LeafNode *>(data_ptr), num_leaves);
         }
         catch (const std::exception &exc)

package.json

@@ -1,6 +1,6 @@
 {
   "name": "osrm",
-  "version": "5.7.2",
+  "version": "5.7.3",
   "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/extractor/extraction_containers.cpp

@@ -173,7 +173,7 @@ void ExtractionContainers::WriteCharData(const std::string &file_name)
     util::UnbufferedLog log;
     log << "writing street name index ... ";
     TIMER_START(write_index);
-    boost::filesystem::ofstream file(file_name, std::ios::binary);
+    storage::io::FileWriter file(file_name, storage::io::FileWriter::HasNoFingerprint);
 
     const util::NameTable::IndexedData indexed_data;
     indexed_data.write(file, name_offsets.begin(), name_offsets.end(), name_char_data.begin());

unit_tests/util/indexed_data.cpp

@@ -1,6 +1,8 @@
 #include "util/indexed_data.hpp"
 #include "util/exception.hpp"
 
+#include <boost/filesystem.hpp>
+
 #include <boost/test/test_case_template.hpp>
 #include <boost/test/unit_test.hpp>
 
@@ -34,11 +36,18 @@ BOOST_AUTO_TEST_CASE(check_variable_group_block_bitops)
 template <typename IndexedData, typename Offsets, typename Data>
 void test_rw(const Offsets &offsets, const Data &data)
 {
-    std::stringstream sstr;
     IndexedData indexed_data;
-    indexed_data.write(sstr, offsets.begin(), offsets.end(), data.begin());
+    auto path = boost::filesystem::unique_path();
 
-    const std::string str = sstr.str();
+    {
+        storage::io::FileWriter writer(path, storage::io::FileWriter::HasNoFingerprint);
+        indexed_data.write(writer, offsets.begin(), offsets.end(), data.begin());
+    }
+
+    storage::io::FileReader reader(path, storage::io::FileReader::HasNoFingerprint);
+    auto length = reader.GetSize();
+    std::string str(length, '\0');
+    reader.ReadInto(const_cast<char *>(str.data()), length);
 
 #if 0
     std::cout << "\n" << typeid(IndexedData).name() << "\nsaved size = " << str.size() << "\n";
@@ -175,14 +184,21 @@ BOOST_AUTO_TEST_CASE(check_corrupted_memory)
 
 BOOST_AUTO_TEST_CASE(check_string_view)
 {
-    std::stringstream sstr;
+    auto path = boost::filesystem::unique_path();
     std::string name_data = "hellostringview";
     std::vector<std::uint32_t> name_offsets = {0, 5, 11, 15};
 
     IndexedData<VariableGroupBlock<16, StringView>> indexed_data;
-    indexed_data.write(sstr, name_offsets.begin(), name_offsets.end(), name_data.begin());
+    {
+        storage::io::FileWriter writer(path, storage::io::FileWriter::HasNoFingerprint);
+        indexed_data.write(writer, name_offsets.begin(), name_offsets.end(), name_data.begin());
+    }
+
+    storage::io::FileReader reader(path, storage::io::FileReader::HasNoFingerprint);
+    auto length = reader.GetSize();
+    std::string str(length, '\0');
+    reader.ReadInto(const_cast<char *>(str.data()), length);
 
-    const std::string str = sstr.str();
     indexed_data.reset(str.c_str(), str.c_str() + str.size());
 
     BOOST_CHECK_EQUAL(indexed_data.at(0), "hello");

unit_tests/util/name_table.cpp

@@ -1,6 +1,7 @@
 #include "util/name_table.hpp"
 #include "util/exception.hpp"
 
+#include <boost/filesystem.hpp>
 #include <boost/test/test_case_template.hpp>
 #include <boost/test/unit_test.hpp>
 
@@ -19,7 +20,6 @@ using namespace osrm::util;
 
 std::string PrapareNameTableData(std::vector<std::string> &data, bool fill_all)
 {
-    std::stringstream sstr;
     NameTable::IndexedData indexed_data;
     std::vector<unsigned char> name_char_data;
     std::vector<std::uint32_t> name_offsets;
@@ -54,9 +54,19 @@ std::string PrapareNameTableData(std::vector<std::string> &data, bool fill_all)
     }
     name_offsets.push_back(name_char_data.size());
 
-    indexed_data.write(sstr, name_offsets.begin(), name_offsets.end(), name_char_data.begin());
+    auto path = boost::filesystem::unique_path();
+    {
+        storage::io::FileWriter writer(path, storage::io::FileWriter::HasNoFingerprint);
+        indexed_data.write(
+            writer, name_offsets.begin(), name_offsets.end(), name_char_data.begin());
+    }
 
-    return sstr.str();
+    storage::io::FileReader reader(path, storage::io::FileReader::HasNoFingerprint);
+    auto length = reader.GetSize();
+    std::string str(length, '\0');
+    reader.ReadInto(const_cast<char *>(str.data()), length);
+
+    return str;
 }
 
 BOOST_AUTO_TEST_CASE(check_name_table_fill)