.travis.yml

@@ -142,20 +142,22 @@ matrix:
       compiler: clang
       env: BUILD='Debug'
     - os: osx
-      osx_image: xcode9.1
+      osx_image: xcode9.3
       compiler: clang
       env: BUILD='Debug'
     - os: osx
-      osx_image: xcode9.1
+      osx_image: xcode9.3
       compiler: clang
       env: BUILD='Release'
 
 #-----------------------------------------------------------------------------
 
 install:
-  - if [[ $(uname -s) == 'Darwin' ]]; then
-      brew update;
-      brew install protobuf;
+  - |
+    if [[ $(uname -s) == 'Darwin' ]]; then
+      brew update
+      brew install protobuf
+      true
     fi
 
 script:

CHANGELOG.md

@@ -15,6 +15,15 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
 
+## [1.6.3] - 2018-07-17
+
+### Changed
+
+- Moved `byteswap_inplace` functions from detail into protozero namespace.
+  They can be useful outsize protozero.
+- More asserts and unit tests and small cleanups.
+
+
 ## [1.6.2] - 2018-03-09
 
 ### Changed
@@ -301,7 +310,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - Make pbf reader and writer code endianess-aware.
 
 
-[unreleased]: https://github.com/osmcode/libosmium/compare/v1.6.2...HEAD
+[unreleased]: https://github.com/osmcode/libosmium/compare/v1.6.3...HEAD
+[1.6.3]: https://github.com/osmcode/libosmium/compare/v1.6.2...v1.6.3
 [1.6.2]: https://github.com/osmcode/libosmium/compare/v1.6.1...v1.6.2
 [1.6.1]: https://github.com/osmcode/libosmium/compare/v1.6.0...v1.6.1
 [1.6.0]: https://github.com/osmcode/libosmium/compare/v1.5.3...v1.6.0

CMakeLists.txt

@@ -14,7 +14,7 @@ project(protozero)
 
 set(PROTOZERO_VERSION_MAJOR 1)
 set(PROTOZERO_VERSION_MINOR 6)
-set(PROTOZERO_VERSION_PATCH 2)
+set(PROTOZERO_VERSION_PATCH 3)
 
 set(PROTOZERO_VERSION
     "${PROTOZERO_VERSION_MAJOR}.${PROTOZERO_VERSION_MINOR}.${PROTOZERO_VERSION_PATCH}")

debian/changelog

+protozero (1.6.3-1~bpo9+1) stretch-backports; urgency=medium
+
+  * Rebuild for stretch-backports.
+
+ -- Bas Couwenberg <sebastic@debian.org>  Fri, 20 Jul 2018 07:05:57 +0200
+
+protozero (1.6.3-1) unstable; urgency=medium
+
+  * New upstream release.
+  * Update Vcs-* URLs for Salsa.
+  * Bump Standards-Version to 4.1.5, no changes.
+  * Strip trailing whitespace from control & rules files.
+  * Use filter instead of findstring to prevent partial matches.
+
+ -- Bas Couwenberg <sebastic@debian.org>  Tue, 17 Jul 2018 18:17:35 +0200
+
 protozero (1.6.2-1~bpo9+1) stretch-backports; urgency=medium
 
   * Rebuild for stretch-backports.

debian/control

@@ -10,9 +10,9 @@ Build-Depends: debhelper (>= 9),
                libprotobuf-dev,
                protobuf-compiler,
                pkg-config
-Standards-Version: 4.1.3
-Vcs-Browser: https://anonscm.debian.org/cgit/pkg-grass/protozero.git/
-Vcs-Git: https://anonscm.debian.org/git/pkg-grass/protozero.git -b stretch-backports
+Standards-Version: 4.1.5
+Vcs-Browser: https://salsa.debian.org/debian-gis-team/protozero/
+Vcs-Git: https://salsa.debian.org/debian-gis-team/protozero.git -b stretch-backports
 Homepage: https://github.com/mapbox/protozero
 
 Package: libprotozero-dev
@@ -55,4 +55,3 @@ Description: Minimalistic protocol buffer decoder and encoder in C++ - docs
  Protobufs protoc program.
  .
  This package contains the protozero documentation.
-

debian/rules

@@ -18,7 +18,7 @@ override_dh_auto_build:
 
 override_dh_auto_test:
 # Ignore test failures on problematic architectures only
-ifneq (,$(findstring $(DEB_BUILD_ARCH),"armhf"))
+ifneq (,$(filter $(DEB_BUILD_ARCH),armhf))
 	dh_auto_test || echo "Ignoring test failures"
 else
 	dh_auto_test
@@ -29,4 +29,3 @@ override_dh_install:
 
 override_dh_installchangelogs:
 	dh_installchangelogs CHANGELOG.md
-

include/protozero/byteswap.hpp

@@ -49,35 +49,45 @@ inline uint64_t byteswap_impl(uint64_t value) noexcept {
 #endif
 }
 
+} // end namespace detail
+
 inline void byteswap_inplace(uint32_t* ptr) noexcept {
-    *ptr = byteswap_impl(*ptr);
+    *ptr = detail::byteswap_impl(*ptr);
 }
 
 inline void byteswap_inplace(uint64_t* ptr) noexcept {
-    *ptr = byteswap_impl(*ptr);
+    *ptr = detail::byteswap_impl(*ptr);
 }
 
 inline void byteswap_inplace(int32_t* ptr) noexcept {
     auto bptr = reinterpret_cast<uint32_t*>(ptr);
-    *bptr = byteswap_impl(*bptr);
+    *bptr = detail::byteswap_impl(*bptr);
 }
 
 inline void byteswap_inplace(int64_t* ptr) noexcept {
     auto bptr = reinterpret_cast<uint64_t*>(ptr);
-    *bptr = byteswap_impl(*bptr);
+    *bptr = detail::byteswap_impl(*bptr);
 }
 
 inline void byteswap_inplace(float* ptr) noexcept {
     auto bptr = reinterpret_cast<uint32_t*>(ptr);
-    *bptr = byteswap_impl(*bptr);
+    *bptr = detail::byteswap_impl(*bptr);
 }
 
 inline void byteswap_inplace(double* ptr) noexcept {
     auto bptr = reinterpret_cast<uint64_t*>(ptr);
-    *bptr = byteswap_impl(*bptr);
+    *bptr = detail::byteswap_impl(*bptr);
 }
 
+namespace detail {
+
+    // Added for backwards compatibility with any code that might use this
+    // function (even if it shouldn't have). Will be removed in a later
+    // version of protozero.
+    using ::protozero::byteswap_inplace;
+
 } // end namespace detail
+
 } // end namespace protozero
 
 #endif // PROTOZERO_BYTESWAP_HPP

include/protozero/iterators.hpp

@@ -184,11 +184,11 @@ public:
 
     ~const_fixed_iterator() noexcept = default;
 
-    value_type operator*() const {
+    value_type operator*() const noexcept {
         value_type result;
         std::memcpy(&result, m_data, sizeof(value_type));
 #if PROTOZERO_BYTE_ORDER != PROTOZERO_LITTLE_ENDIAN
-        detail::byteswap_inplace(&result);
+        byteswap_inplace(&result);
 #endif
         return result;
     }
@@ -302,6 +302,11 @@ public:
     using reference         = value_type&;
 
     static difference_type distance(const_varint_iterator begin, const_varint_iterator end) noexcept {
+        // The "distance" between default initialized const_varint_iterator's
+        // is always 0.
+        if (!begin.m_data) {
+            return 0;
+        }
         // We know that each varint contains exactly one byte with the most
         // significant bit not set. We can use this to quickly figure out
         // how many varints there are without actually decoding the varints.
@@ -326,16 +331,19 @@ public:
     ~const_varint_iterator() noexcept = default;
 
     value_type operator*() const {
+        protozero_assert(m_data);
         const char* d = m_data; // will be thrown away
         return static_cast<value_type>(decode_varint(&d, m_end));
     }
 
     const_varint_iterator& operator++() {
+        protozero_assert(m_data);
         skip_varint(&m_data, m_end);
         return *this;
     }
 
     const_varint_iterator operator++(int) {
+        protozero_assert(m_data);
         const const_varint_iterator tmp{*this};
         ++(*this);
         return tmp;
@@ -383,16 +391,19 @@ public:
     ~const_svarint_iterator() = default;
 
     value_type operator*() const {
+        protozero_assert(this->m_data);
         const char* d = this->m_data; // will be thrown away
         return static_cast<value_type>(decode_zigzag64(decode_varint(&d, this->m_end)));
     }
 
     const_svarint_iterator& operator++() {
+        protozero_assert(this->m_data);
         skip_varint(&this->m_data, this->m_end);
         return *this;
     }
 
     const_svarint_iterator operator++(int) {
+        protozero_assert(this->m_data);
         const const_svarint_iterator tmp{*this};
         ++(*this);
         return tmp;

include/protozero/pbf_reader.hpp

@@ -80,7 +80,7 @@ class pbf_reader {
         skip_bytes(sizeof(T));
         std::memcpy(&result, data, sizeof(T));
 #if PROTOZERO_BYTE_ORDER != PROTOZERO_LITTLE_ENDIAN
-        detail::byteswap_inplace(&result);
+        byteswap_inplace(&result);
 #endif
         return result;
     }
@@ -98,7 +98,9 @@ class pbf_reader {
 
     template <typename T>
     T get_varint() {
-        return static_cast<T>(decode_varint(&m_data, m_end));
+        const auto val = static_cast<T>(decode_varint(&m_data, m_end));
+        assert(m_data <= m_end);
+        return val;
     }
 
     template <typename T>
@@ -469,6 +471,7 @@ public:
             default:
                 break;
         }
+        assert(m_data <= m_end);
     }
 
     ///@{

include/protozero/pbf_writer.hpp

@@ -93,7 +93,7 @@ class pbf_writer {
         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
-        detail::byteswap_inplace(&value);
+        byteswap_inplace(&value);
 #endif
         m_data->append(reinterpret_cast<const char*>(&value), sizeof(T));
     }

include/protozero/version.hpp

@@ -23,12 +23,12 @@ documentation.
 #define PROTOZERO_VERSION_MINOR 6
 
 /// The patch number
-#define PROTOZERO_VERSION_PATCH 2
+#define PROTOZERO_VERSION_PATCH 3
 
 /// 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.6.2"
+#define PROTOZERO_VERSION_STRING "1.6.3"
 
 #endif // PROTOZERO_VERSION_HPP

test/unit/CMakeLists.txt

@@ -10,6 +10,7 @@ set(UNIT_TESTS data_view
                basic
                endian
                exceptions
+               iterators
                varint
                zigzag)
 

test/unit/test_basic.cpp

@@ -37,48 +37,58 @@ TEST_CASE("next() should throw when illegal wire type is encountered") {
     REQUIRE_THROWS_AS(item.next(), const protozero::unknown_pbf_wire_type_exception&);
 }
 
-TEST_CASE("next() should throw when illegal tag is encountered") {
+TEST_CASE("next() should throw when illegal tag 0 is encountered") {
     std::string data;
-
-    SECTION("tag 0") {
     protozero::write_varint(std::back_inserter(data), 0u << 3u | 1u);
+    protozero::pbf_reader item{data};
+    REQUIRE_THROWS_AS(item.next(), const protozero::invalid_tag_exception&);
 }
 
-    SECTION("tag 19000") {
+TEST_CASE("next() should throw when illegal tag 19000 is encountered") {
+    std::string data;
     protozero::write_varint(std::back_inserter(data), 19000u << 3u | 1u);
+    protozero::pbf_reader item{data};
+    REQUIRE_THROWS_AS(item.next(), const protozero::invalid_tag_exception&);
 }
 
-    SECTION("tag 19001") {
+TEST_CASE("next() should throw when illegal tag 19001 is encountered") {
+    std::string data;
     protozero::write_varint(std::back_inserter(data), 19001u << 3u | 1u);
+    protozero::pbf_reader item{data};
+    REQUIRE_THROWS_AS(item.next(), const protozero::invalid_tag_exception&);
 }
 
-    SECTION("tag 19999") {
+TEST_CASE("next() should throw when illegal tag 19999 is encountered") {
+    std::string data;
     protozero::write_varint(std::back_inserter(data), 19999u << 3u | 1u);
-    }
-
     protozero::pbf_reader item{data};
     REQUIRE_THROWS_AS(item.next(), const protozero::invalid_tag_exception&);
 }
 
-TEST_CASE("next() works when a legal tag is encountered") {
+TEST_CASE("next() works when legal tag 1 is encountered") {
     std::string data;
-
-    SECTION("tag 1") {
     protozero::write_varint(std::back_inserter(data), 1u << 3u | 1u);
+    protozero::pbf_reader item{data};
+    REQUIRE(item.next());
 }
 
-    SECTION("tag 18999") {
+TEST_CASE("next() works when legal tag 18999 is encountered") {
+    std::string data;
     protozero::write_varint(std::back_inserter(data), 18999u << 3u | 1u);
+    protozero::pbf_reader item{data};
+    REQUIRE(item.next());
 }
 
-    SECTION("tag 20000") {
+TEST_CASE("next() works when legal tag 20000 is encountered") {
+    std::string data;
     protozero::write_varint(std::back_inserter(data), 20000u << 3u | 1u);
+    protozero::pbf_reader item{data};
+    REQUIRE(item.next());
 }
 
-    SECTION("tag 1^29 - 1") {
-        protozero::write_varint(std::back_inserter(data), ((1u << 29u) - 1u) << 3u | 1u);
-    }
-
+TEST_CASE("next() works when legal tag 1^29 - 1 is encountered") {
+    std::string data;
+    protozero::write_varint(std::back_inserter(data), ((1ul << 29u) - 1u) << 3u | 1u);
     protozero::pbf_reader item{data};
     REQUIRE(item.next());
 }

test/unit/test_endian.cpp

@@ -7,14 +7,14 @@
 #include <protozero/byteswap.hpp>
 
 static int32_t check_swap_4(int32_t data) noexcept {
-    protozero::detail::byteswap_inplace(&data);
-    protozero::detail::byteswap_inplace(&data);
+    protozero::byteswap_inplace(&data);
+    protozero::byteswap_inplace(&data);
     return data;
 }
 
 static int64_t check_swap_8(int64_t data) noexcept {
-    protozero::detail::byteswap_inplace(&data);
-    protozero::detail::byteswap_inplace(&data);
+    protozero::byteswap_inplace(&data);
+    protozero::byteswap_inplace(&data);
     return data;
 }
 
@@ -40,32 +40,32 @@ TEST_CASE("byte swapping") {
 
 TEST_CASE("byte swap uint32_t") {
     uint32_t a = 17;
-    protozero::detail::byteswap_inplace(&a);
-    protozero::detail::byteswap_inplace(&a);
+    protozero::byteswap_inplace(&a);
+    protozero::byteswap_inplace(&a);
 
     REQUIRE(17 == a);
 }
 
 TEST_CASE("byte swap uint64_t") {
     uint64_t a = 347529808;
-    protozero::detail::byteswap_inplace(&a);
-    protozero::detail::byteswap_inplace(&a);
+    protozero::byteswap_inplace(&a);
+    protozero::byteswap_inplace(&a);
 
     REQUIRE(347529808 == a);
 }
 
 TEST_CASE("byte swap double") {
     double a = 1.1;
-    protozero::detail::byteswap_inplace(&a);
-    protozero::detail::byteswap_inplace(&a);
+    protozero::byteswap_inplace(&a);
+    protozero::byteswap_inplace(&a);
 
     REQUIRE(a == Approx(1.1));
 }
 
 TEST_CASE("byte swap float") {
     float a = 1.1f;
-    protozero::detail::byteswap_inplace(&a);
-    protozero::detail::byteswap_inplace(&a);
+    protozero::byteswap_inplace(&a);
+    protozero::byteswap_inplace(&a);
 
     REQUIRE(a == Approx(1.1f));
 }

test/unit/test_iterators.cpp

+
+#include <test.hpp>
+
+TEST_CASE("default constructed varint_iterators are equal") {
+    protozero::const_varint_iterator<uint32_t> a{};
+    protozero::const_varint_iterator<uint32_t> b{};
+
+    protozero::iterator_range<protozero::const_varint_iterator<uint32_t>> r{};
+
+    REQUIRE(a == a);
+    REQUIRE(a == b);
+    REQUIRE(a == r.begin());
+    REQUIRE(a == r.end());
+    REQUIRE(r.empty());
+    REQUIRE(r.size() == 0); // NOLINT(readability-container-size-empty)
+    REQUIRE(r.begin() == r.end());
+}
+

test/unit/test_varint.cpp

@@ -185,3 +185,43 @@ TEST_CASE("lots of varints back and forth") {
     }
 }
 
+TEST_CASE("skip_varint with empty buffer throws") {
+    const char* buffer = "";
+    REQUIRE_THROWS_AS(protozero::skip_varint(&buffer, buffer), const protozero::end_of_buffer_exception&);
+}
+
+TEST_CASE("call skip_varint with every possible value for single byte in buffer") {
+    char buffer;
+    for (int i = 0; i <= 127; ++i) {
+        buffer = static_cast<char>(i);
+        const char* b = &buffer;
+        protozero::skip_varint(&b, &buffer + 1);
+        REQUIRE(b == &buffer + 1);
+    }
+    for (int i = 128; i <= 255; ++i) {
+        buffer = static_cast<char>(i);
+        const char* b = &buffer;
+        REQUIRE_THROWS_AS(protozero::skip_varint(&b, &buffer + 1), const protozero::end_of_buffer_exception&);
+    }
+}
+
+TEST_CASE("decode_varint with empty buffer throws") {
+    const char* buffer = "";
+    REQUIRE_THROWS_AS(protozero::decode_varint(&buffer, buffer), const protozero::end_of_buffer_exception&);
+}
+
+TEST_CASE("call decode_varint with every possible value for single byte in buffer") {
+    char buffer;
+    for (int i = 0; i <= 127; ++i) {
+        buffer = static_cast<char>(i);
+        const char* b = &buffer;
+        REQUIRE(protozero::decode_varint(&b, &buffer + 1) == i);
+        REQUIRE(b == &buffer + 1);
+    }
+    for (int i = 128; i <= 255; ++i) {
+        buffer = static_cast<char>(i);
+        const char* b = &buffer;
+        REQUIRE_THROWS_AS(protozero::decode_varint(&b, &buffer + 1), const protozero::end_of_buffer_exception&);
+    }
+}
+