From b91c2f0299722e64a6945808d64c3397c35811d0 Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Tue, 18 Jul 2017 21:08:32 +0000 Subject: [PATCH] Squashed 'third_party/libosmium/' changes from c1f34c455..9fd2348c6 9fd2348c6 Release v2.11.3 ed708286e Fix namespace. 835df8a7f Fix multipolygon assembler. 0979ab529 Fix areas assembler algorithm. 801f84c62 Bugfix: Invalid use of iterators. f85653820 Read OPL file correctly even if trailing newline in file missing. a31571c0f Release v2.11.2 a3903b368 Use minimum size of 64 bytes for buffers. b86bafefe Release v2.11.1 32ebf736c Updated change log. 632ea5198 Bugfix: Call get_creator_function() in main thread. ddc79eee7 Add test for not correctly handled unsupported_file_format_error. 86197a14f Bugfix: Terminate called on full buffer. 4340be8ad Fix the Filter::count() method. git-subtree-dir: third_party/libosmium git-subtree-split: 9fd2348c6956b6e1b930b50850e99eb31207ed50 --- CHANGELOG.md | 34 +++++++++++++++++-- CMakeLists.txt | 2 +- include/osmium/area/assembler.hpp | 24 +++++++------ include/osmium/builder/builder.hpp | 18 +++++++++- include/osmium/builder/osm_object_builder.hpp | 20 ++++++----- include/osmium/io/detail/opl_input_format.hpp | 5 +++ include/osmium/io/reader.hpp | 8 +++-- include/osmium/memory/buffer.hpp | 15 ++++++-- include/osmium/tags/filter.hpp | 2 +- include/osmium/version.hpp | 4 +-- test/CMakeLists.txt | 3 +- test/t/io/data-nonl.opl | 1 + test/t/io/data.opl | 1 + test/t/io/test_opl_parser.cpp | 22 ++++++++++++ test/t/io/test_reader_fileformat.cpp | 10 ++++++ test/t/memory/test_buffer_basics.cpp | 19 +++++++++-- 16 files changed, 151 insertions(+), 37 deletions(-) create mode 100644 test/t/io/data-nonl.opl create mode 100644 test/t/io/data.opl create mode 100644 test/t/io/test_reader_fileformat.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index d12bb76cf..cc1fdcc8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,12 +6,36 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [unreleased] - -### Added +### Fixed -### Changed +## [2.11.3] - 2017-05-03 ### Fixed +- Two bugs in area assembler affecting very complex multipolygons and + multipolygons with overlapping or nearly overlapping lines. +- Invalid use of iterators leading to undefined behaviour in area assembler + code. +- Read OPL file correctly even if trailing newline in file is missing. + + +## [2.11.2] - 2017-04-10 + +### Fixed + +- Use minimum size of 64 bytes for buffers. This fixes an infinite loop + when buffer size is zero. + + +## [2.11.1] - 2017-03-07 + +### Fixed + +- Terminate called on full non-auto-growing buffer. (Issue #189.) +- When file formats were used that were not compiled into the binary, it + terminated instead of throwing. (Issue #197.) +- The `Filter::count()` method didn't compile at all. + ## [2.11.0] - 2017-01-14 @@ -525,7 +549,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). Doxygen (up to version 1.8.8). This version contains a workaround to fix this. -[unreleased]: https://github.com/osmcode/libosmium/compare/v2.10.3...HEAD +[unreleased]: https://github.com/osmcode/libosmium/compare/v2.11.3...HEAD +[2.11.3]: https://github.com/osmcode/libosmium/compare/v2.11.2...v2.11.3 +[2.11.2]: https://github.com/osmcode/libosmium/compare/v2.11.1...v2.11.2 +[2.11.1]: https://github.com/osmcode/libosmium/compare/v2.11.0...v2.11.1 +[2.11.0]: https://github.com/osmcode/libosmium/compare/v2.10.3...v2.11.0 [2.10.3]: https://github.com/osmcode/libosmium/compare/v2.10.2...v2.10.3 [2.10.2]: https://github.com/osmcode/libosmium/compare/v2.10.1...v2.10.2 [2.10.1]: https://github.com/osmcode/libosmium/compare/v2.10.0...v2.10.1 diff --git a/CMakeLists.txt b/CMakeLists.txt index 21478ec8d..e536c3469 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -25,7 +25,7 @@ project(libosmium) set(LIBOSMIUM_VERSION_MAJOR 2) set(LIBOSMIUM_VERSION_MINOR 11) -set(LIBOSMIUM_VERSION_PATCH 0) +set(LIBOSMIUM_VERSION_PATCH 3) set(LIBOSMIUM_VERSION "${LIBOSMIUM_VERSION_MAJOR}.${LIBOSMIUM_VERSION_MINOR}.${LIBOSMIUM_VERSION_PATCH}") diff --git a/include/osmium/area/assembler.hpp b/include/osmium/area/assembler.hpp index 3509b57bc..78a9d09d7 100644 --- a/include/osmium/area/assembler.hpp +++ b/include/osmium/area/assembler.hpp @@ -474,17 +474,17 @@ namespace osmium { class rings_stack_element { - int32_t m_y; + double m_y; detail::ProtoRing* m_ring_ptr; public: - rings_stack_element(int32_t y, detail::ProtoRing* ring_ptr) : + rings_stack_element(double y, detail::ProtoRing* ring_ptr) : m_y(y), m_ring_ptr(ring_ptr) { } - int32_t y() const noexcept { + double y() const noexcept { return m_y; } @@ -504,7 +504,7 @@ namespace osmium { return m_y < rhs.m_y; } - }; // class ring_stack_element + }; // class rings_stack_element using rings_stack = std::vector; @@ -592,7 +592,7 @@ namespace osmium { if (debug()) { std::cerr << " Segment belongs to outer ring\n"; } - const int32_t y = int32_t(ay + (by - ay) * (lx - ax) / (bx - ax)); + const double y = ay + (by - ay) * (lx - ax) / double(bx - ax); outer_rings.emplace_back(y, segment->ring()); } } @@ -859,8 +859,8 @@ namespace osmium { } void merge_two_rings(open_ring_its_type& open_ring_its, const location_to_ring_map& m1, const location_to_ring_map& m2) { - auto& r1 = *m1.ring_it; - auto& r2 = *m2.ring_it; + std::list::iterator r1 = *m1.ring_it; + std::list::iterator r2 = *m2.ring_it; if (r1->get_node_ref_stop().location() == r2->get_node_ref_start().location()) { r1->join_forward(*r2); @@ -876,11 +876,11 @@ namespace osmium { assert(false); } + open_ring_its.erase(std::find(open_ring_its.begin(), open_ring_its.end(), r2)); m_rings.erase(r2); - open_ring_its.remove(r2); if (r1->closed()) { - open_ring_its.remove(r1); + open_ring_its.erase(std::find(open_ring_its.begin(), open_ring_its.end(), r1)); } } @@ -988,6 +988,7 @@ namespace osmium { } loc_done.insert(c.stop_location); find_candidates(candidates, loc_done, xrings, c); + loc_done.erase(c.stop_location); if (debug()) { std::cerr << " ...back\n"; } @@ -1085,12 +1086,13 @@ namespace osmium { // Join all (open) rings in the candidate to get one closed ring. assert(chosen_cand->rings.size() > 1); const auto& first_ring = chosen_cand->rings.front().first; - for (auto it = chosen_cand->rings.begin() + 1; it != chosen_cand->rings.end(); ++it) { + const detail::ProtoRing& remaining_ring = first_ring.ring(); + for (auto it = std::next(chosen_cand->rings.begin()); it != chosen_cand->rings.end(); ++it) { merge_two_rings(open_ring_its, first_ring, it->first); } if (debug()) { - std::cerr << " Merged to " << first_ring.ring() << "\n"; + std::cerr << " Merged to " << remaining_ring << "\n"; } return true; diff --git a/include/osmium/builder/builder.hpp b/include/osmium/builder/builder.hpp index 107f4a9d0..d4358843b 100644 --- a/include/osmium/builder/builder.hpp +++ b/include/osmium/builder/builder.hpp @@ -167,6 +167,20 @@ namespace osmium { return length; } + /** + * Append data to buffer and append an additional \0. + * + * @param data Pointer to data. + * @param length Length of data in bytes. + * @returns The number of bytes appended (length + 1). + */ + osmium::memory::item_size_type append_with_zero(const char* data, const osmium::memory::item_size_type length) { + unsigned char* target = reserve_space(length + 1); + std::copy_n(reinterpret_cast(data), length, target); + target[length] = '\0'; + return length + 1; + } + /** * Append \0-terminated string to buffer. * @@ -180,9 +194,11 @@ namespace osmium { /** * Append '\0' to the buffer. * + * @deprecated Use append_with_zero() instead. + * * @returns The number of bytes appended (always 1). */ - osmium::memory::item_size_type append_zero() { + OSMIUM_DEPRECATED osmium::memory::item_size_type append_zero() { *reserve_space(1) = '\0'; return 1; } diff --git a/include/osmium/builder/osm_object_builder.hpp b/include/osmium/builder/osm_object_builder.hpp index 1dd5f972f..d4483b192 100644 --- a/include/osmium/builder/osm_object_builder.hpp +++ b/include/osmium/builder/osm_object_builder.hpp @@ -99,7 +99,8 @@ namespace osmium { if (std::strlen(value) > osmium::max_osm_string_length) { throw std::length_error("OSM tag value is too long"); } - add_size(append(key) + append(value)); + add_size(append(key)); + add_size(append(value)); } /** @@ -117,8 +118,8 @@ namespace osmium { if (value_length > osmium::max_osm_string_length) { throw std::length_error("OSM tag value is too long"); } - add_size(append(key, osmium::memory::item_size_type(key_length)) + append_zero() + - append(value, osmium::memory::item_size_type(value_length)) + append_zero()); + add_size(append_with_zero(key, osmium::memory::item_size_type(key_length))); + add_size(append_with_zero(value, osmium::memory::item_size_type(value_length))); } /** @@ -134,8 +135,8 @@ namespace osmium { if (value.size() > osmium::max_osm_string_length) { throw std::length_error("OSM tag value is too long"); } - add_size(append(key.data(), osmium::memory::item_size_type(key.size()) + 1) + - append(value.data(), osmium::memory::item_size_type(value.size()) + 1)); + add_size(append(key.data(), osmium::memory::item_size_type(key.size()) + 1)); + add_size(append(value.data(), osmium::memory::item_size_type(value.size()) + 1)); } /** @@ -144,7 +145,8 @@ namespace osmium { * @param tag Tag. */ void add_tag(const osmium::Tag& tag) { - add_size(append(tag.key()) + append(tag.value())); + add_size(append(tag.key())); + add_size(append(tag.value())); } /** @@ -226,7 +228,7 @@ namespace osmium { throw std::length_error("OSM relation member role is too long"); } member.set_role_size(osmium::string_size_type(length) + 1); - add_size(append(role, osmium::memory::item_size_type(length)) + append_zero()); + add_size(append_with_zero(role, osmium::memory::item_size_type(length))); add_padding(true); } @@ -310,7 +312,7 @@ namespace osmium { throw std::length_error("OSM user name is too long"); } comment.set_user_size(osmium::string_size_type(length) + 1); - add_size(append(user, osmium::memory::item_size_type(length)) + append_zero()); + add_size(append_with_zero(user, osmium::memory::item_size_type(length))); } void add_text(osmium::ChangesetComment& comment, const char* text, const size_t length) { @@ -322,7 +324,7 @@ namespace osmium { throw std::length_error("OSM changeset comment is too long"); } comment.set_text_size(osmium::string_size_type(length) + 1); - add_size(append(text, osmium::memory::item_size_type(length)) + append_zero()); + add_size(append_with_zero(text, osmium::memory::item_size_type(length))); add_padding(true); } diff --git a/include/osmium/io/detail/opl_input_format.hpp b/include/osmium/io/detail/opl_input_format.hpp index 135f1a5e0..62598f706 100644 --- a/include/osmium/io/detail/opl_input_format.hpp +++ b/include/osmium/io/detail/opl_input_format.hpp @@ -123,6 +123,11 @@ namespace osmium { rest = input.substr(ppos); } + if (!rest.empty()) { + m_data = rest.data(); + parse_line(); + } + if (m_buffer.committed() > 0) { send_to_output_queue(std::move(m_buffer)); } diff --git a/include/osmium/io/reader.hpp b/include/osmium/io/reader.hpp index c39eaaa62..825cb41ce 100644 --- a/include/osmium/io/reader.hpp +++ b/include/osmium/io/reader.hpp @@ -92,6 +92,8 @@ namespace osmium { osmium::io::File m_file; + detail::ParserFactory::create_parser_type m_creator; + enum class status { okay = 0, // normal reading error = 1, // some error occurred while reading @@ -128,13 +130,12 @@ namespace osmium { } // This function will run in a separate thread. - static void parser_thread(const osmium::io::File& file, + static void parser_thread(const detail::ParserFactory::create_parser_type& creator, detail::future_string_queue_type& input_queue, detail::future_buffer_queue_type& osmdata_queue, std::promise&& header_promise, osmium::io::detail::reader_options options) { std::promise promise = std::move(header_promise); - const auto creator = detail::ParserFactory::instance().get_creator_function(file); const auto parser = creator(input_queue, osmdata_queue, promise, options); parser->parse(); } @@ -236,6 +237,7 @@ namespace osmium { template explicit Reader(const osmium::io::File& file, TArgs&&... args) : m_file(file.check()), + m_creator(detail::ParserFactory::instance().get_creator_function(m_file)), m_status(status::okay), m_childpid(0), m_input_queue(detail::get_input_queue_size(), "raw_input"), @@ -256,7 +258,7 @@ namespace osmium { std::promise header_promise; m_header_future = header_promise.get_future(); - m_thread = osmium::thread::thread_handler{parser_thread, std::ref(m_file), std::ref(m_input_queue), std::ref(m_osmdata_queue), std::move(header_promise), m_options}; + m_thread = osmium::thread::thread_handler{parser_thread, std::ref(m_creator), std::ref(m_input_queue), std::ref(m_osmdata_queue), std::move(header_promise), m_options}; } template diff --git a/include/osmium/memory/buffer.hpp b/include/osmium/memory/buffer.hpp index 370d01e0b..1b75b4fa5 100644 --- a/include/osmium/memory/buffer.hpp +++ b/include/osmium/memory/buffer.hpp @@ -119,6 +119,15 @@ namespace osmium { auto_grow m_auto_grow{auto_grow::no}; std::function m_full; + static size_t calculate_capacity(size_t capacity) noexcept { + // The majority of all Nodes will fit into this size. + constexpr static const size_t min_capacity = 64; + if (capacity < min_capacity) { + return min_capacity; + } + return capacity; + } + public: /** @@ -198,13 +207,13 @@ namespace osmium { * of the alignment. */ explicit Buffer(size_t capacity, auto_grow auto_grow = auto_grow::yes) : - m_memory(new unsigned char[capacity]), + m_memory(new unsigned char[calculate_capacity(capacity)]), m_data(m_memory.get()), - m_capacity(capacity), + m_capacity(calculate_capacity(capacity)), m_written(0), m_committed(0), m_auto_grow(auto_grow) { - if (capacity % align_bytes != 0) { + if (m_capacity % align_bytes != 0) { throw std::invalid_argument("buffer capacity needs to be multiple of alignment"); } } diff --git a/include/osmium/tags/filter.hpp b/include/osmium/tags/filter.hpp index 22abb1430..797565db8 100644 --- a/include/osmium/tags/filter.hpp +++ b/include/osmium/tags/filter.hpp @@ -140,7 +140,7 @@ namespace osmium { * Return the number of rules in this filter. */ size_t count() const { - return m_rules.count(); + return m_rules.size(); } /** diff --git a/include/osmium/version.hpp b/include/osmium/version.hpp index d32641a55..a36a4d900 100644 --- a/include/osmium/version.hpp +++ b/include/osmium/version.hpp @@ -35,8 +35,8 @@ DEALINGS IN THE SOFTWARE. #define LIBOSMIUM_VERSION_MAJOR 2 #define LIBOSMIUM_VERSION_MINOR 11 -#define LIBOSMIUM_VERSION_PATCH 0 +#define LIBOSMIUM_VERSION_PATCH 3 -#define LIBOSMIUM_VERSION_STRING "2.11.0" +#define LIBOSMIUM_VERSION_STRING "2.11.3" #endif // OSMIUM_VERSION_HPP diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index db0e93485..68f8887c4 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -167,9 +167,10 @@ add_unit_test(io test_compression_factory) add_unit_test(io test_bzip2 ENABLE_IF ${BZIP2_FOUND} LIBS ${BZIP2_LIBRARIES}) add_unit_test(io test_file_formats) add_unit_test(io test_reader LIBS "${OSMIUM_XML_LIBRARIES};${OSMIUM_PBF_LIBRARIES}") +add_unit_test(io test_reader_fileformat ENABLE_IF ${Threads_FOUND} LIBS ${CMAKE_THREAD_LIBS_INIT}) add_unit_test(io test_reader_with_mock_decompression ENABLE_IF ${Threads_FOUND} LIBS ${OSMIUM_XML_LIBRARIES}) add_unit_test(io test_reader_with_mock_parser ENABLE_IF ${Threads_FOUND} LIBS ${CMAKE_THREAD_LIBS_INIT}) -add_unit_test(io test_opl_parser) +add_unit_test(io test_opl_parser ENABLE_IF ${Threads_FOUND} LIBS ${CMAKE_THREAD_LIBS_INIT}) add_unit_test(io test_output_utils) add_unit_test(io test_output_iterator ENABLE_IF ${Threads_FOUND} LIBS ${CMAKE_THREAD_LIBS_INIT}) add_unit_test(io test_string_table) diff --git a/test/t/io/data-nonl.opl b/test/t/io/data-nonl.opl new file mode 100644 index 000000000..db2354439 --- /dev/null +++ b/test/t/io/data-nonl.opl @@ -0,0 +1 @@ +n1 v1 dV c1 t2014-01-01T00:00:00Z i1 utest T x1.02 y1.02 \ No newline at end of file diff --git a/test/t/io/data.opl b/test/t/io/data.opl new file mode 100644 index 000000000..2c7e2a35f --- /dev/null +++ b/test/t/io/data.opl @@ -0,0 +1 @@ +n1 v1 dV c1 t2014-01-01T00:00:00Z i1 utest T x1.02 y1.02 diff --git a/test/t/io/test_opl_parser.cpp b/test/t/io/test_opl_parser.cpp index 9ad6eeb75..dad223865 100644 --- a/test/t/io/test_opl_parser.cpp +++ b/test/t/io/test_opl_parser.cpp @@ -3,8 +3,10 @@ #include #include "catch.hpp" +#include "utils.hpp" #include +#include #include namespace oid = osmium::io::detail; @@ -1073,3 +1075,23 @@ TEST_CASE("Parse line with external interface") { } +TEST_CASE("Parse OPL using Reader") { + osmium::io::File file{with_data_dir("t/io/data.opl")}; + osmium::io::Reader reader{file}; + + const auto buffer = reader.read(); + REQUIRE(buffer); + const auto& node = buffer.get(0); + REQUIRE(node.id() == 1); +} + +TEST_CASE("Parse OPL with missing newline using Reader") { + osmium::io::File file{with_data_dir("t/io/data-nonl.opl")}; + osmium::io::Reader reader{file}; + + const auto buffer = reader.read(); + REQUIRE(buffer); + const auto& node = buffer.get(0); + REQUIRE(node.id() == 1); +} + diff --git a/test/t/io/test_reader_fileformat.cpp b/test/t/io/test_reader_fileformat.cpp new file mode 100644 index 000000000..a64932e70 --- /dev/null +++ b/test/t/io/test_reader_fileformat.cpp @@ -0,0 +1,10 @@ + +#include "catch.hpp" +#include "utils.hpp" + +#include + +TEST_CASE("Reader throws on unsupported file format") { + REQUIRE_THROWS_AS(osmium::io::Reader{with_data_dir("t/io/data.osm")}, osmium::unsupported_file_format_error); +} + diff --git a/test/t/memory/test_buffer_basics.cpp b/test/t/memory/test_buffer_basics.cpp index ffe725168..d9d174b0f 100644 --- a/test/t/memory/test_buffer_basics.cpp +++ b/test/t/memory/test_buffer_basics.cpp @@ -6,8 +6,8 @@ TEST_CASE("Buffer basics") { osmium::memory::Buffer invalid_buffer1; osmium::memory::Buffer invalid_buffer2; - osmium::memory::Buffer empty_buffer1(1024); - osmium::memory::Buffer empty_buffer2(2048); + osmium::memory::Buffer empty_buffer1{1024}; + osmium::memory::Buffer empty_buffer2{2048}; REQUIRE(!invalid_buffer1); REQUIRE(!invalid_buffer2); @@ -32,3 +32,18 @@ TEST_CASE("Buffer basics") { } +TEST_CASE("Buffer with zero size") { + osmium::memory::Buffer buffer{0}; + REQUIRE(buffer.capacity() == 64); +} + +TEST_CASE("Buffer with less than minimum size") { + osmium::memory::Buffer buffer{63}; + REQUIRE(buffer.capacity() == 64); +} + +TEST_CASE("Buffer with minimum size") { + osmium::memory::Buffer buffer{64}; + REQUIRE(buffer.capacity() == 64); +} +