diff --git a/CHANGELOG.md b/CHANGELOG.md index 25327c8e0..ef5c65675 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,29 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +## [2.10.2] - 2016-11-16 + +### Changed + +- Updated embedded protozero to 1.4.4. + +### Fixed + +- Buffer overflow in osmium::Buffer. + + +## [2.10.1] - 2016-11-15 + +### Changed + +- Updated embedded protozero to 1.4.3. + +### Fixed + +- Made IdSet work on 32bit systems. +- Fixed endianness check for WKB tests. + + ## [2.10.0] - 2016-11-11 ### Added @@ -457,7 +480,9 @@ 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.0...HEAD +[unreleased]: https://github.com/osmcode/libosmium/compare/v2.10.2...HEAD +[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 [2.10.0]: https://github.com/osmcode/libosmium/compare/v2.9.0...v2.10.0 [2.9.0]: https://github.com/osmcode/libosmium/compare/v2.8.0...v2.9.0 [2.8.0]: https://github.com/osmcode/libosmium/compare/v2.7.2...v2.8.0 diff --git a/CMakeLists.txt b/CMakeLists.txt index 21cf98e11..6f10b0ceb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -25,7 +25,7 @@ project(libosmium) set(LIBOSMIUM_VERSION_MAJOR 2) set(LIBOSMIUM_VERSION_MINOR 10) -set(LIBOSMIUM_VERSION_PATCH 0) +set(LIBOSMIUM_VERSION_PATCH 2) set(LIBOSMIUM_VERSION "${LIBOSMIUM_VERSION_MAJOR}.${LIBOSMIUM_VERSION_MINOR}.${LIBOSMIUM_VERSION_PATCH}") diff --git a/include/osmium/builder/osm_object_builder.hpp b/include/osmium/builder/osm_object_builder.hpp index b1c7220a7..0a8fa6f96 100644 --- a/include/osmium/builder/osm_object_builder.hpp +++ b/include/osmium/builder/osm_object_builder.hpp @@ -413,11 +413,10 @@ namespace osmium { const auto available_space = min_size_for_user - sizeof(string_size_type) - 1; if (length > available_space) { const auto space_needed = osmium::memory::padded_length(length - available_space); - reserve_space(space_needed); + std::fill_n(reserve_space(space_needed), space_needed, 0); add_size(static_cast(space_needed)); } std::copy_n(user, length, object().data() + size_of_object); - std::fill_n(object().data() + size_of_object + length, osmium::memory::padded_length(length + 1) - length, 0); object().set_user_size(length + 1); return static_cast(*this); @@ -612,11 +611,10 @@ namespace osmium { const auto available_space = min_size_for_user - 1; if (length > available_space) { const auto space_needed = osmium::memory::padded_length(length - available_space); - reserve_space(space_needed); + std::fill_n(reserve_space(space_needed), space_needed, 0); add_size(static_cast(space_needed)); } std::copy_n(user, length, object().data() + sizeof(Changeset)); - std::fill_n(object().data() + sizeof(Changeset) + length, osmium::memory::padded_length(length + 1) - length, 0); object().set_user_size(length + 1); return *this; diff --git a/include/osmium/index/id_set.hpp b/include/osmium/index/id_set.hpp index 4a894a0ba..b9a9f9d7a 100644 --- a/include/osmium/index/id_set.hpp +++ b/include/osmium/index/id_set.hpp @@ -91,13 +91,16 @@ namespace osmium { template class IdSetDenseIterator { + static_assert(std::is_unsigned::value, "Needs unsigned type"); + static_assert(sizeof(T) >= 4, "Needs at least 32bit type"); + const IdSetDense* m_set; T m_value; T m_last; void next() noexcept { while (m_value != m_last && !m_set->get(m_value)) { - const auto cid = IdSetDense::chunk_id(m_value); + const T cid = IdSetDense::chunk_id(m_value); assert(cid < m_set->m_data.size()); if (!m_set->m_data[cid]) { m_value = (cid + 1) << (IdSetDense::chunk_bits + 3); @@ -179,7 +182,7 @@ namespace osmium { constexpr static const size_t chunk_size = 1 << chunk_bits; std::vector> m_data; - size_t m_size = 0; + T m_size = 0; static size_t chunk_id(T id) noexcept { return id >> (chunk_bits + 3); @@ -194,7 +197,7 @@ namespace osmium { } T last() const noexcept { - return m_data.size() * chunk_size * 8; + return static_cast(m_data.size()) * chunk_size * 8; } unsigned char& get_element(T id) { @@ -285,7 +288,7 @@ namespace osmium { /** * The number of Ids stored in the set. */ - size_t size() const noexcept { + T size() const noexcept { return m_size; } diff --git a/include/osmium/osm/location.hpp b/include/osmium/osm/location.hpp index d20871759..bc03a2ef4 100644 --- a/include/osmium/osm/location.hpp +++ b/include/osmium/osm/location.hpp @@ -512,9 +512,9 @@ namespace osmium { template <> inline size_t hash<8>(const osmium::Location& location) noexcept { - size_t h = location.x(); + uint64_t h = location.x(); h <<= 32; - return h ^ location.y(); + return static_cast(h ^ location.y()); } } // namespace detail diff --git a/include/osmium/version.hpp b/include/osmium/version.hpp index 09c576288..bae10dd3e 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 10 -#define LIBOSMIUM_VERSION_PATCH 0 +#define LIBOSMIUM_VERSION_PATCH 2 -#define LIBOSMIUM_VERSION_STRING "2.10.0" +#define LIBOSMIUM_VERSION_STRING "2.10.2" #endif // OSMIUM_VERSION_HPP diff --git a/include/protozero/byteswap.hpp b/include/protozero/byteswap.hpp index 06ba6ded9..3afb34840 100644 --- a/include/protozero/byteswap.hpp +++ b/include/protozero/byteswap.hpp @@ -22,50 +22,63 @@ documentation. #include namespace protozero { +namespace detail { -/** - * Swap N byte value between endianness formats. This template function must - * be specialized to actually work. - */ -template -inline void byteswap(const char* /*data*/, char* /*result*/) noexcept { - static_assert(N == 1, "Can only swap 4 or 8 byte values"); -} - -/** - * Swap 4 byte value (int32_t, uint32_t, float) between endianness formats. - */ -template <> -inline void byteswap<4>(const char* data, char* result) noexcept { +inline uint32_t byteswap_impl(uint32_t value) noexcept { #ifdef PROTOZERO_USE_BUILTIN_BSWAP - *reinterpret_cast(result) = __builtin_bswap32(*reinterpret_cast(data)); + return __builtin_bswap32(value); #else - result[3] = data[0]; - result[2] = data[1]; - result[1] = data[2]; - result[0] = data[3]; + return ((value & 0xff000000) >> 24) | + ((value & 0x00ff0000) >> 8) | + ((value & 0x0000ff00) << 8) | + ((value & 0x000000ff) << 24); #endif } -/** - * Swap 8 byte value (int64_t, uint64_t, double) between endianness formats. - */ -template <> -inline void byteswap<8>(const char* data, char* result) noexcept { +inline uint64_t byteswap_impl(uint64_t value) noexcept { #ifdef PROTOZERO_USE_BUILTIN_BSWAP - *reinterpret_cast(result) = __builtin_bswap64(*reinterpret_cast(data)); + return __builtin_bswap64(value); #else - result[7] = data[0]; - result[6] = data[1]; - result[5] = data[2]; - result[4] = data[3]; - result[3] = data[4]; - result[2] = data[5]; - result[1] = data[6]; - result[0] = data[7]; + return ((value & 0xff00000000000000ULL) >> 56) | + ((value & 0x00ff000000000000ULL) >> 40) | + ((value & 0x0000ff0000000000ULL) >> 24) | + ((value & 0x000000ff00000000ULL) >> 8) | + ((value & 0x00000000ff000000ULL) << 8) | + ((value & 0x0000000000ff0000ULL) << 24) | + ((value & 0x000000000000ff00ULL) << 40) | + ((value & 0x00000000000000ffULL) << 56); #endif } +inline void byteswap_inplace(uint32_t* ptr) noexcept { + *ptr = byteswap_impl(*ptr); +} + +inline void byteswap_inplace(uint64_t* ptr) noexcept { + *ptr = byteswap_impl(*ptr); +} + +inline void byteswap_inplace(int32_t* ptr) noexcept { + auto bptr = reinterpret_cast(ptr); + *bptr = byteswap_impl(*bptr); +} + +inline void byteswap_inplace(int64_t* ptr) noexcept { + auto bptr = reinterpret_cast(ptr); + *bptr = byteswap_impl(*bptr); +} + +inline void byteswap_inplace(float* ptr) noexcept { + auto bptr = reinterpret_cast(ptr); + *bptr = byteswap_impl(*bptr); +} + +inline void byteswap_inplace(double* ptr) noexcept { + auto bptr = reinterpret_cast(ptr); + *bptr = byteswap_impl(*bptr); +} + +} // end namespace detail } // end namespace protozero #endif // PROTOZERO_BYTESWAP_HPP diff --git a/include/protozero/iterators.hpp b/include/protozero/iterators.hpp index 00ba91935..40259a9fc 100644 --- a/include/protozero/iterators.hpp +++ b/include/protozero/iterators.hpp @@ -29,21 +29,6 @@ documentation. namespace protozero { -namespace detail { - - // Copy N bytes from src to dest on little endian machines, on big - // endian swap the bytes in the process. - template - inline void copy_or_byteswap(const char* src, void* dest) noexcept { -#if PROTOZERO_BYTE_ORDER == PROTOZERO_LITTLE_ENDIAN - std::memcpy(dest, src, N); -#else - byteswap(src, reinterpret_cast(dest)); -#endif - } - -} // end namespace detail - /** * A range of iterators based on std::pair. Created from beginning and * end iterators. Used as a return type from some pbf_reader methods @@ -213,7 +198,10 @@ public: value_type operator*() const { value_type result; - detail::copy_or_byteswap(m_data , &result); + std::memcpy(&result, m_data, sizeof(value_type)); +#if PROTOZERO_BYTE_ORDER != PROTOZERO_LITTLE_ENDIAN + detail::byteswap_inplace(&result); +#endif return result; } diff --git a/include/protozero/pbf_message.hpp b/include/protozero/pbf_message.hpp index 055773408..f66604a94 100644 --- a/include/protozero/pbf_message.hpp +++ b/include/protozero/pbf_message.hpp @@ -79,8 +79,8 @@ public: return pbf_reader::next(); } - bool next(T tag) { - return pbf_reader::next(pbf_tag_type(tag)); + bool next(T next_tag) { + return pbf_reader::next(pbf_tag_type(next_tag)); } T tag() const noexcept { diff --git a/include/protozero/pbf_reader.hpp b/include/protozero/pbf_reader.hpp index 2f4054d2d..69f2d7287 100644 --- a/include/protozero/pbf_reader.hpp +++ b/include/protozero/pbf_reader.hpp @@ -75,7 +75,10 @@ class pbf_reader { T get_fixed() { T result; skip_bytes(sizeof(T)); - detail::copy_or_byteswap(m_data - sizeof(T), &result); + std::memcpy(&result, m_data - sizeof(T), sizeof(T)); +#if PROTOZERO_BYTE_ORDER != PROTOZERO_LITTLE_ENDIAN + detail::byteswap_inplace(&result); +#endif return result; } @@ -134,7 +137,7 @@ public: /** * Construct a pbf_reader message from a data_view. The pointer from the * data_view will be stored inside the pbf_reader object, no data is - * copied. So you must* make sure the view stays valid as long as the + * copied. So you must make sure the view stays valid as long as the * pbf_reader object is used. * * The buffer must contain a complete protobuf message. @@ -149,25 +152,27 @@ public: } /** - * Construct a pbf_reader message from a data pointer and a length. The pointer - * will be stored inside the pbf_reader object, no data is copied. So you must - * make sure the buffer stays valid as long as the pbf_reader object is used. + * Construct a pbf_reader message from a data pointer and a length. The + * pointer will be stored inside the pbf_reader object, no data is copied. + * So you must make sure the buffer stays valid as long as the pbf_reader + * object is used. * * The buffer must contain a complete protobuf message. * * @post There is no current field. */ - pbf_reader(const char* data, std::size_t length) noexcept + pbf_reader(const char* data, std::size_t size) noexcept : m_data(data), - m_end(data + length), + m_end(data + size), m_wire_type(pbf_wire_type::unknown), m_tag(0) { } /** - * Construct a pbf_reader message from a data pointer and a length. The pointer - * will be stored inside the pbf_reader object, no data is copied. So you must - * make sure the buffer stays valid as long as the pbf_reader object is used. + * Construct a pbf_reader message from a data pointer and a length. The + * pointer will be stored inside the pbf_reader object, no data is copied. + * So you must make sure the buffer stays valid as long as the pbf_reader + * object is used. * * The buffer must contain a complete protobuf message. * @@ -181,10 +186,10 @@ public: } /** - * Construct a pbf_reader message from a std::string. A pointer to the string - * internals will be stored inside the pbf_reader object, no data is copied. - * So you must make sure the string is unchanged as long as the pbf_reader - * object is used. + * Construct a pbf_reader message from a std::string. A pointer to the + * string internals will be stored inside the pbf_reader object, no data + * is copied. So you must make sure the string is unchanged as long as the + * pbf_reader object is used. * * The string must contain a complete protobuf message. * @@ -231,8 +236,9 @@ public: } /** - * In a boolean context the pbf_reader class evaluates to `true` if there are - * still fields available and to `false` if the last field has been read. + * In a boolean context the pbf_reader class evaluates to `true` if there + * are still fields available and to `false` if the last field has been + * read. */ operator bool() const noexcept { return m_data < m_end; @@ -276,7 +282,8 @@ public: // tags 0 and 19000 to 19999 are not allowed as per // https://developers.google.com/protocol-buffers/docs/proto - protozero_assert(((m_tag > 0 && m_tag < 19000) || (m_tag > 19999 && m_tag <= ((1 << 29) - 1))) && "tag out of range"); + protozero_assert(((m_tag > 0 && m_tag < 19000) || + (m_tag > 19999 && m_tag <= ((1 << 29) - 1))) && "tag out of range"); m_wire_type = pbf_wire_type(value & 0x07); switch (m_wire_type) { @@ -317,9 +324,9 @@ public: * @pre There must be no current field. * @post If it returns `true` there is a current field now with the given tag. */ - bool next(pbf_tag_type tag) { + bool next(pbf_tag_type next_tag) { while (next()) { - if (m_tag == tag) { + if (m_tag == next_tag) { return true; } else { skip(); diff --git a/include/protozero/pbf_writer.hpp b/include/protozero/pbf_writer.hpp index bce1c3ffc..77cc9d025 100644 --- a/include/protozero/pbf_writer.hpp +++ b/include/protozero/pbf_writer.hpp @@ -90,13 +90,10 @@ class pbf_writer { void add_fixed(T value) { protozero_assert(m_pos == 0 && "you can't add fields to a parent pbf_writer if there is an existing pbf_writer for a submessage"); protozero_assert(m_data); -#if PROTOZERO_BYTE_ORDER == PROTOZERO_LITTLE_ENDIAN - m_data->append(reinterpret_cast(&value), sizeof(T)); -#else - const auto size = m_data->size(); - m_data->resize(size + sizeof(T)); - byteswap(reinterpret_cast(&value), const_cast(m_data->data() + size)); +#if PROTOZERO_BYTE_ORDER != PROTOZERO_LITTLE_ENDIAN + detail::byteswap_inplace(&value); #endif + m_data->append(reinterpret_cast(&value), sizeof(T)); } template diff --git a/include/protozero/types.hpp b/include/protozero/types.hpp index 8b046387e..431c2074c 100644 --- a/include/protozero/types.hpp +++ b/include/protozero/types.hpp @@ -78,12 +78,12 @@ public: /** * Create data_view from pointer and size. * - * @param data Pointer to the data. - * @param size Length of the data. + * @param ptr Pointer to the data. + * @param length Length of the data. */ - constexpr data_view(const char* data, std::size_t size) noexcept - : m_data(data), - m_size(size) { + constexpr data_view(const char* ptr, std::size_t length) noexcept + : m_data(ptr), + m_size(length) { } /** @@ -99,11 +99,11 @@ public: /** * Create data_view from zero-terminated string. * - * @param data Pointer to the data. + * @param ptr Pointer to the data. */ - data_view(const char* data) noexcept - : m_data(data), - m_size(std::strlen(data)) { + data_view(const char* ptr) noexcept + : m_data(ptr), + m_size(std::strlen(ptr)) { } /** diff --git a/include/protozero/version.hpp b/include/protozero/version.hpp index 127e64972..c3e91a36b 100644 --- a/include/protozero/version.hpp +++ b/include/protozero/version.hpp @@ -23,13 +23,13 @@ documentation. #define PROTOZERO_VERSION_MINOR 4 /// The patch number -#define PROTOZERO_VERSION_PATCH 2 +#define PROTOZERO_VERSION_PATCH 4 /// The complete version number #define PROTOZERO_VERSION_CODE (PROTOZERO_VERSION_MAJOR * 10000 + PROTOZERO_VERSION_MINOR * 100 + PROTOZERO_VERSION_PATCH) /// Version number as string -#define PROTOZERO_VERSION_STRING "1.4.2" +#define PROTOZERO_VERSION_STRING "1.4.4" #endif // PROTOZERO_VERSION_HPP diff --git a/test/t/geom/test_ogr_wkb.cpp b/test/t/geom/test_ogr_wkb.cpp index 548d14389..47d1cdd77 100644 --- a/test/t/geom/test_ogr_wkb.cpp +++ b/test/t/geom/test_ogr_wkb.cpp @@ -1,6 +1,4 @@ -#if __BYTE_ORDER == __LITTLE_ENDIAN - #include "catch.hpp" #include @@ -9,6 +7,9 @@ #include #include +#include + +#if __BYTE_ORDER == __LITTLE_ENDIAN #include "area_helper.hpp" #include "wnl_helper.hpp" diff --git a/test/t/geom/test_wkb.cpp b/test/t/geom/test_wkb.cpp index 66dd42e51..e46c5deb8 100644 --- a/test/t/geom/test_wkb.cpp +++ b/test/t/geom/test_wkb.cpp @@ -2,6 +2,8 @@ #include #include +#include + #include "wnl_helper.hpp" #if __BYTE_ORDER == __LITTLE_ENDIAN diff --git a/test/t/index/test_id_set.cpp b/test/t/index/test_id_set.cpp index 4c2444717..ec8280a38 100644 --- a/test/t/index/test_id_set.cpp +++ b/test/t/index/test_id_set.cpp @@ -51,9 +51,9 @@ TEST_CASE("Iterating over IdSetDense") { s.set(35); s.set(35); s.set(20); - s.set(1LL << 33); + s.set(1ULL << 33); s.set(21); - s.set((1LL << 27) + 13); + s.set((1ULL << 27) + 13); REQUIRE(s.size() == 6); @@ -71,10 +71,10 @@ TEST_CASE("Iterating over IdSetDense") { REQUIRE(*it == 35); ++it; REQUIRE(it != s.end()); - REQUIRE(*it == (1LL << 27) + 13); + REQUIRE(*it == (1ULL << 27) + 13); ++it; REQUIRE(it != s.end()); - REQUIRE(*it == 1LL << 33); + REQUIRE(*it == 1ULL << 33); ++it; REQUIRE(it == s.end()); } @@ -133,9 +133,9 @@ TEST_CASE("Iterating over IdSetSmall") { s.set(35); s.set(35); s.set(20); - s.set(1LL << 33); + s.set(1ULL << 33); s.set(21); - s.set((1LL << 27) + 13); + s.set((1ULL << 27) + 13); // needs to be called before size() and iterator will work properly s.sort_unique(); @@ -156,10 +156,10 @@ TEST_CASE("Iterating over IdSetSmall") { REQUIRE(*it == 35); ++it; REQUIRE(it != s.end()); - REQUIRE(*it == (1LL << 27) + 13); + REQUIRE(*it == (1ULL << 27) + 13); ++it; REQUIRE(it != s.end()); - REQUIRE(*it == 1LL << 33); + REQUIRE(*it == 1ULL << 33); ++it; REQUIRE(it == s.end()); } diff --git a/test/t/osm/test_location.cpp b/test/t/osm/test_location.cpp index 7abf77962..6aba91dbe 100644 --- a/test/t/osm/test_location.cpp +++ b/test/t/osm/test_location.cpp @@ -156,8 +156,10 @@ TEST_CASE("Location hash") { if (sizeof(size_t) == 8) { REQUIRE(std::hash{}({0, 0}) == 0); REQUIRE(std::hash{}({0, 1}) == 1); - REQUIRE(std::hash{}({1, 0}) == 0x100000000); - REQUIRE(std::hash{}({1, 1}) == 0x100000001); + const int64_t a = std::hash{}({1, 0}); + REQUIRE(a == 0x100000000); + const int64_t b = std::hash{}({1, 1}); + REQUIRE(b == 0x100000001); } else { REQUIRE(std::hash{}({0, 0}) == 0); REQUIRE(std::hash{}({0, 1}) == 1);