.clang-tidy

 ---
-Checks: '*,-cert-dcl21-cpp,-cert-err60-cpp,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-type-reinterpret-cast,-fuchsia-*,-google-runtime-references,-hicpp-no-array-decay'
+Checks: '*,-cert-dcl21-cpp,-cert-err60-cpp,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-type-reinterpret-cast,-fuchsia-*,-google-runtime-references,-hicpp-no-array-decay,-readability-implicit-bool-conversion'
 #
 #  Disabled checks:
 #
@@ -28,6 +28,9 @@ Checks: '*,-cert-dcl21-cpp,-cert-err60-cpp,-cppcoreguidelines-pro-bounds-pointer
 #  hicpp-no-array-decay
 #    Limited use and many false positives including for all asserts.
 #
+#  readability-implicit-bool-conversion
+#    Not necessarily more readable.
+#
 WarningsAsErrors: '*'
 HeaderFilterRegex: '\/include\/'
 AnalyzeTemporaryDtors: false

.travis.yml

@@ -6,8 +6,6 @@
 
 language: generic
 
-sudo: false
-
 dist: trusty
 
 #-----------------------------------------------------------------------------
@@ -33,7 +31,11 @@ addons_shortcuts:
   addons_clang50: &clang50
     apt:
       sources: [ 'ubuntu-toolchain-r-test', 'llvm-toolchain-trusty-5.0' ]
-      packages: [ 'libprotobuf-dev','protobuf-compiler', 'clang-5.0', 'clang-tidy-5.0' ]
+      packages: [ 'libprotobuf-dev','protobuf-compiler', 'clang-5.0' ]
+  addons_clang60: &clang60
+    apt:
+      sources: [ 'ubuntu-toolchain-r-test', 'llvm-toolchain-trusty-6.0' ]
+      packages: [ 'libprotobuf-dev','protobuf-compiler', 'clang-6.0', 'clang-tidy-6.0' ]
   addons_gcc47: &gcc47
     apt:
       sources: [ 'ubuntu-toolchain-r-test' ]
@@ -54,6 +56,10 @@ addons_shortcuts:
     apt:
       sources: [ 'ubuntu-toolchain-r-test' ]
       packages: [ 'libprotobuf-dev','protobuf-compiler', 'g++-6', 'gcc-6' ]
+  addons_gcc7: &gcc7
+    apt:
+      sources: [ 'ubuntu-toolchain-r-test' ]
+      packages: [ 'libprotobuf-dev','protobuf-compiler', 'g++-7', 'gcc-7' ]
 
 #-----------------------------------------------------------------------------
 
@@ -78,20 +84,22 @@ matrix:
     - os: linux
       compiler: "clang-5.0"
       env: BUILD='Debug' CC=clang-5.0 CXX=clang++-5.0
-           CLANG_TIDY=clang-tidy-5.0
       addons: *clang50
     - os: linux
-      compiler: "clang-5.0"
-      env: BUILD='Release' CC=clang-5.0 CXX=clang++-5.0
-      addons: *clang50
+      compiler: "clang-6.0"
+      env: BUILD='Debug' CC=clang-6.0 CXX=clang++-6.0
+           CLANG_TIDY=clang-tidy-6.0
+      addons: *clang60
     - os: linux
-      compiler: "clang-5.0"
-      env: BUILD='Debug' CC=clang-5.0 CXX=clang++-5.0
+      compiler: "clang-6.0"
+      env: BUILD='Release' CC=clang-6.0 CXX=clang++-6.0
+      addons: *clang60
+    - os: linux
+      compiler: "clang-6.0"
+      env: BUILD='Debug' CC=clang-6.0 CXX=clang++-6.0
            CXXFLAGS="-fsanitize=address,undefined,integer -fno-sanitize-recover=all -fno-omit-frame-pointer"
            LDFLAGS="-fsanitize=address,undefined,integer"
-      # LSAN doesn't work on container-based system
-      sudo: required
-      addons: *clang50
+      addons: *clang60
     - os: linux
       compiler: "gcc-4.7"
       env: BUILD='Debug' CC=gcc-4.7 CXX=g++-4.7
@@ -129,10 +137,10 @@ matrix:
       compiler: "gcc-6"
       env: BUILD='Release' CC=gcc-6 CXX=g++-6
       addons: *gcc6
-    - os: osx
-      osx_image: xcode6.4
-      compiler: clang
-      env: BUILD='Debug'
+    - os: linux
+      compiler: "gcc-7"
+      env: BUILD='Debug' CC=gcc-7 CXX=g++-7
+      addons: *gcc7
     - os: osx
       osx_image: xcode7.3
       compiler: clang
@@ -142,11 +150,15 @@ matrix:
       compiler: clang
       env: BUILD='Debug'
     - os: osx
-      osx_image: xcode9.3
+      osx_image: xcode9.4
       compiler: clang
       env: BUILD='Debug'
     - os: osx
-      osx_image: xcode9.3
+      osx_image: xcode9.4
+      compiler: clang
+      env: BUILD='Release'
+    - os: osx
+      osx_image: xcode10
       compiler: clang
       env: BUILD='Release'
 

CHANGELOG.md

@@ -15,6 +15,27 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
 
+## [1.6.4] - 2018-11-08
+
+### Added
+
+- Add function `data()` to get the not yet read data from a `pbf_reader`.
+- New `add_packed_fixed()` template function for `pbf_writer`.
+- New `length_of_varint()` helper function calculates how long a varint
+  would be for a specified value.
+
+### Changed
+
+- More consistent implementation of operators as free friend functions.
+
+### Fixed
+
+- Fixed some zigzag encoding tests on MSVC.
+- Add extra cast so we do an xor with unsigned ints.
+- No more bitwise operations on signed integers in varint decoder.
+- No more bitwise operations on signed integers in zigzag encoder/decoder.
+
+
 ## [1.6.3] - 2018-07-17
 
 ### Changed
@@ -310,7 +331,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.3...HEAD
+[unreleased]: https://github.com/osmcode/libosmium/compare/v1.6.4...HEAD
+[1.6.4]: https://github.com/osmcode/libosmium/compare/v1.6.3...v1.6.4
 [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

CMakeLists.txt

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

appveyor.yml

@@ -48,10 +48,5 @@ build_script:
         build-appveyor.bat
     )
 
-# remove garbage VS messages
-# http://help.appveyor.com/discussions/problems/4569-the-target-_convertpdbfiles-listed-in-a-beforetargets-attribute-at-c-does-not-exist-in-the-project-and-will-be-ignored
-before_build:
-  - del "C:\Program Files (x86)\MSBuild\14.0\Microsoft.Common.targets\ImportAfter\Xamarin.Common.targets"
-
 
 #-----------------------------------------------------------------------------

build-appveyor.bat

@@ -9,16 +9,9 @@ SET
 ECHO cmake on AppVeyor
 cmake -version
 
-ECHO activating VS cmd prompt && CALL "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat" amd64
+ECHO activating VS cmd prompt && CALL "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build\vcvars64.bat"
 IF %ERRORLEVEL% NEQ 0 GOTO ERROR
 
-SET protobuf_sdk=protozero-dep-protobuf-2.6.1.7z
-IF EXIST %protobuf_sdk% (ECHO protobuf already downloaded) ELSE (ECHO downloading protobuf ... && powershell Invoke-WebRequest https://mapbox.s3.amazonaws.com/windows-builds/windows-build-deps/$env:protobuf_sdk -OutFile $pwd\$env:protobuf_sdk)
-IF %ERRORLEVEL% NEQ 0 GOTO ERROR
-IF EXIST deps\protobuf (ECHO protobuf already extracted) ELSE (CALL 7z x -y %protobuf_sdk% | %windir%\system32\FIND "ing archive")
-IF %ERRORLEVEL% NEQ 0 GOTO ERROR
-SET PATH=%~dp0deps\protobuf;%PATH%
-
 IF EXIST build ECHO deleting build dir... && RD /Q /S build
 IF %ERRORLEVEL% NEQ 0 GOTO ERROR
 
@@ -28,11 +21,8 @@ IF %ERRORLEVEL% NEQ 0 GOTO ERROR
 CD build
 ECHO config^: %config%
 
-::This will produce lots of LNK4099 warnings which can be ignored.
-::Unfortunately they can't be disabled, see
-::http://stackoverflow.com/questions/661606/visual-c-how-to-disable-specific-linker-warnings
 SET CMAKE_CMD=cmake .. ^
--LA -G "Visual Studio 14 Win64"
+-LA -G "Visual Studio 15 2017 Win64"
 
 ECHO calling^: %CMAKE_CMD%
 %CMAKE_CMD%
@@ -43,9 +33,7 @@ IF /I "%APPVEYOR%"=="True" SET avlogger=/logger:"C:\Program Files\AppVeyor\Build
 
 msbuild protozero.sln ^
 /p:Configuration=%config% ^
-/toolsversion:14.0 ^
-/p:Platform=x64 ^
-/p:PlatformToolset=v140 %avlogger%
+%avlogger%
 IF %ERRORLEVEL% NEQ 0 GOTO ERROR
 
 ctest --output-on-failure ^

debian/changelog

+protozero (1.6.4-1~bpo9+1) stretch-backports; urgency=medium
+
+  * Rebuild for stretch-backports.
+
+ -- Bas Couwenberg <sebastic@debian.org>  Wed, 14 Nov 2018 06:53:17 +0100
+
+protozero (1.6.4-1) unstable; urgency=medium
+
+  * New upstream release.
+  * Bump Standards-Version to 4.2.1, no changes.
+  * Update watch file to limit matches to archive path.
+
+ -- Bas Couwenberg <sebastic@debian.org>  Thu, 08 Nov 2018 17:45:31 +0100
+
+protozero (1.6.3-2) unstable; urgency=medium
+
+  * Drop autopkgtest to test installability.
+  * Add lintian override for testsuite-autopkgtest-missing.
+
+ -- Bas Couwenberg <sebastic@debian.org>  Wed, 01 Aug 2018 16:38:06 +0200
+
 protozero (1.6.3-1~bpo9+1) stretch-backports; urgency=medium
 
   * Rebuild for stretch-backports.

debian/control

@@ -10,7 +10,7 @@ Build-Depends: debhelper (>= 9),
                libprotobuf-dev,
                protobuf-compiler,
                pkg-config
-Standards-Version: 4.1.5
+Standards-Version: 4.2.1
 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

debian/source/lintian-overrides

+# Not worth the effort
+testsuite-autopkgtest-missing
+

debian/tests/control

-# Test installability
-Depends: @
-Test-Command: /bin/true

debian/watch

@@ -4,4 +4,4 @@ dversionmangle=s/\+(debian|dfsg|ds|deb)\d*$//,\
 uversionmangle=s/(\d)[_\.\-\+]?((RC|rc|pre|dev|beta|alpha)\d*)$/$1~$2/,\
 filenamemangle=s/(?:.*\/)?(?:rel|v|protozero)[\-\_]?(\d[\d\-\.]*)\.(tgz|tbz|txz|(?:tar\.(?:gz|bz2|xz)))/protozero-$1.$2/ \
 https://github.com/mapbox/protozero/releases \
-(?:.*/)?(?:rel|v|protozero)[\-\_]?(\d[\d\-\.]+)\.(?:tgz|tbz|txz|(?:tar\.(?:gz|bz2|xz)))
+(?:.*?/archive/)?(?:rel|v|protozero)[\-\_]?(\d[\d\-\.]+)\.(?:tgz|tbz|txz|(?:tar\.(?:gz|bz2|xz)))

include/protozero/byteswap.hpp

@@ -51,29 +51,35 @@ inline uint64_t byteswap_impl(uint64_t value) noexcept {
 
 } // end namespace detail
 
+/// byteswap the data pointed to by ptr in-place.
 inline void byteswap_inplace(uint32_t* ptr) noexcept {
     *ptr = detail::byteswap_impl(*ptr);
 }
 
+/// byteswap the data pointed to by ptr in-place.
 inline void byteswap_inplace(uint64_t* ptr) noexcept {
     *ptr = detail::byteswap_impl(*ptr);
 }
 
+/// byteswap the data pointed to by ptr in-place.
 inline void byteswap_inplace(int32_t* ptr) noexcept {
     auto bptr = reinterpret_cast<uint32_t*>(ptr);
     *bptr = detail::byteswap_impl(*bptr);
 }
 
+/// byteswap the data pointed to by ptr in-place.
 inline void byteswap_inplace(int64_t* ptr) noexcept {
     auto bptr = reinterpret_cast<uint64_t*>(ptr);
     *bptr = detail::byteswap_impl(*bptr);
 }
 
+/// byteswap the data pointed to by ptr in-place.
 inline void byteswap_inplace(float* ptr) noexcept {
     auto bptr = reinterpret_cast<uint32_t*>(ptr);
     *bptr = detail::byteswap_impl(*bptr);
 }
 
+/// byteswap the data pointed to by ptr in-place.
 inline void byteswap_inplace(double* ptr) noexcept {
     auto bptr = reinterpret_cast<uint64_t*>(ptr);
     *bptr = detail::byteswap_impl(*bptr);

include/protozero/iterators.hpp

@@ -164,6 +164,8 @@ class const_fixed_iterator {
 
 public:
 
+    /// @cond usual iterator functions not documented
+
     using iterator_category = std::random_access_iterator_tag;
     using value_type        = T;
     using difference_type   = std::ptrdiff_t;
@@ -204,14 +206,6 @@ public:
         return tmp;
     }
 
-    bool operator==(const_fixed_iterator rhs) const noexcept {
-        return m_data == rhs.m_data;
-    }
-
-    bool operator!=(const_fixed_iterator rhs) const noexcept {
-        return !(*this == rhs);
-    }
-
     const_fixed_iterator& operator--() noexcept {
         m_data -= sizeof(value_type);
         return *this;
@@ -223,6 +217,14 @@ public:
         return tmp;
     }
 
+    friend bool operator==(const_fixed_iterator lhs, const_fixed_iterator rhs) noexcept {
+        return lhs.m_data == rhs.m_data;
+    }
+
+    friend bool operator!=(const_fixed_iterator lhs, const_fixed_iterator rhs) noexcept {
+        return !(lhs == rhs);
+    }
+
     friend bool operator<(const_fixed_iterator lhs, const_fixed_iterator rhs) noexcept {
         return lhs.m_data < rhs.m_data;
     }
@@ -237,7 +239,6 @@ public:
 
     friend bool operator>=(const_fixed_iterator lhs, const_fixed_iterator rhs) noexcept {
         return !(lhs < rhs);
-
     }
 
     const_fixed_iterator& operator+=(difference_type val) noexcept {
@@ -276,6 +277,8 @@ public:
         return *(*this + n);
     }
 
+    /// @endcond
+
 }; // class const_fixed_iterator
 
 /**
@@ -295,6 +298,8 @@ protected:
 
 public:
 
+    /// @cond usual iterator functions not documented
+
     using iterator_category = std::forward_iterator_tag;
     using value_type        = T;
     using difference_type   = std::ptrdiff_t;
@@ -357,6 +362,8 @@ public:
         return !(*this == rhs);
     }
 
+    /// @endcond
+
 }; // class const_varint_iterator
 
 /**
@@ -368,6 +375,8 @@ class const_svarint_iterator : public const_varint_iterator<T> {
 
 public:
 
+    /// @cond usual iterator functions not documented
+
     using iterator_category = std::forward_iterator_tag;
     using value_type        = T;
     using difference_type   = std::ptrdiff_t;
@@ -409,6 +418,8 @@ public:
         return tmp;
     }
 
+    /// @endcond
+
 }; // class const_svarint_iterator
 
 } // end namespace protozero
@@ -419,6 +430,8 @@ namespace std {
     // functions can't be partially specialized, we have to do this for
     // every value_type we are using.
 
+    /// @cond individual overloads do not need to be documented
+
     template <>
     inline typename protozero::const_varint_iterator<int32_t>::difference_type
     distance<protozero::const_varint_iterator<int32_t>>(protozero::const_varint_iterator<int32_t> first, // NOLINT(readability-inconsistent-declaration-parameter-name)
@@ -461,6 +474,8 @@ namespace std {
         return protozero::const_svarint_iterator<int64_t>::distance(first, last);
     }
 
+    /// @endcond
+
 } // end namespace std
 
 #endif // PROTOZERO_ITERATORS_HPP

include/protozero/pbf_reader.hpp

@@ -247,6 +247,13 @@ public:
         return m_data < m_end;
     }
 
+    /**
+     * Get a view of the not yet read data.
+     */
+    data_view data() const noexcept {
+        return {m_data, static_cast<std::size_t>(m_end - m_data)};
+    }
+
     /**
      * Return the length in bytes of the current message. If you have
      * already called next() and/or any of the get_*() functions, this will

include/protozero/pbf_writer.hpp

@@ -785,6 +785,35 @@ public:
         add_packed_varint(tag, first, last);
     }
 
+    /**
+     * Add a "repeated packed" fixed-size field to data. The following
+     * fixed-size fields are available:
+     *
+     * uint32_t -> repeated packed fixed32
+     * int32_t  -> repeated packed sfixed32
+     * uint64_t -> repeated packed fixed64
+     * int64_t  -> repeated packed sfixed64
+     * double   -> repeated packed double
+     * float    -> repeated packed float
+     *
+     * @tparam ValueType One of the following types: (u)int32/64_t, double, float.
+     * @tparam InputIterator A type satisfying the InputIterator concept.
+     * @param tag Tag (field number) of the field
+     * @param first Iterator pointing to the beginning of the data
+     * @param last Iterator pointing one past the end of data
+     */
+    template <typename ValueType, typename InputIterator>
+    void add_packed_fixed(pbf_tag_type tag, InputIterator first, InputIterator last) {
+        static_assert(std::is_same<ValueType, uint32_t>::value ||
+                      std::is_same<ValueType, int32_t>::value ||
+                      std::is_same<ValueType, int64_t>::value ||
+                      std::is_same<ValueType, uint64_t>::value ||
+                      std::is_same<ValueType, double>::value ||
+                      std::is_same<ValueType, float>::value, "Only some types are allowed");
+        add_packed_fixed<ValueType, InputIterator>(tag, first, last,
+            typename std::iterator_traits<InputIterator>::iterator_category{});
+    }
+
     /**
      * Add "repeated packed fixed32" field to data.
      *
@@ -797,7 +826,7 @@ public:
     template <typename InputIterator>
     void add_packed_fixed32(pbf_tag_type tag, InputIterator first, InputIterator last) {
         add_packed_fixed<uint32_t, InputIterator>(tag, first, last,
-            typename std::iterator_traits<InputIterator>::iterator_category());
+            typename std::iterator_traits<InputIterator>::iterator_category{});
     }
 
     /**
@@ -812,7 +841,7 @@ public:
     template <typename InputIterator>
     void add_packed_sfixed32(pbf_tag_type tag, InputIterator first, InputIterator last) {
         add_packed_fixed<int32_t, InputIterator>(tag, first, last,
-            typename std::iterator_traits<InputIterator>::iterator_category());
+            typename std::iterator_traits<InputIterator>::iterator_category{});
     }
 
     /**
@@ -827,7 +856,7 @@ public:
     template <typename InputIterator>
     void add_packed_fixed64(pbf_tag_type tag, InputIterator first, InputIterator last) {
         add_packed_fixed<uint64_t, InputIterator>(tag, first, last,
-            typename std::iterator_traits<InputIterator>::iterator_category());
+            typename std::iterator_traits<InputIterator>::iterator_category{});
     }
 
     /**
@@ -842,7 +871,7 @@ public:
     template <typename InputIterator>
     void add_packed_sfixed64(pbf_tag_type tag, InputIterator first, InputIterator last) {
         add_packed_fixed<int64_t, InputIterator>(tag, first, last,
-            typename std::iterator_traits<InputIterator>::iterator_category());
+            typename std::iterator_traits<InputIterator>::iterator_category{});
     }
 
     /**
@@ -857,7 +886,7 @@ public:
     template <typename InputIterator>
     void add_packed_float(pbf_tag_type tag, InputIterator first, InputIterator last) {
         add_packed_fixed<float, InputIterator>(tag, first, last,
-            typename std::iterator_traits<InputIterator>::iterator_category());
+            typename std::iterator_traits<InputIterator>::iterator_category{});
     }
 
     /**
@@ -872,7 +901,7 @@ public:
     template <typename InputIterator>
     void add_packed_double(pbf_tag_type tag, InputIterator first, InputIterator last) {
         add_packed_fixed<double, InputIterator>(tag, first, last,
-            typename std::iterator_traits<InputIterator>::iterator_category());
+            typename std::iterator_traits<InputIterator>::iterator_category{});
     }
 
     ///@}

include/protozero/varint.hpp

@@ -39,22 +39,22 @@ namespace detail {
         if (iend - begin >= max_varint_length) {  // fast path
             do {
                 int64_t b;
-                b = *p++; val  = uint64_t((b & 0x7fu)       ); if (b >= 0) { break; }
-                b = *p++; val |= uint64_t((b & 0x7fu) <<  7u); if (b >= 0) { break; }
-                b = *p++; val |= uint64_t((b & 0x7fu) << 14u); if (b >= 0) { break; }
-                b = *p++; val |= uint64_t((b & 0x7fu) << 21u); if (b >= 0) { break; }
-                b = *p++; val |= uint64_t((b & 0x7fu) << 28u); if (b >= 0) { break; }
-                b = *p++; val |= uint64_t((b & 0x7fu) << 35u); if (b >= 0) { break; }
-                b = *p++; val |= uint64_t((b & 0x7fu) << 42u); if (b >= 0) { break; }
-                b = *p++; val |= uint64_t((b & 0x7fu) << 49u); if (b >= 0) { break; }
-                b = *p++; val |= uint64_t((b & 0x7fu) << 56u); if (b >= 0) { break; }
-                b = *p++; val |= uint64_t((b & 0x01u) << 63u); if (b >= 0) { break; }
+                b = *p++; val  = ((uint64_t(b) & 0x7fu)       ); if (b >= 0) { break; }
+                b = *p++; val |= ((uint64_t(b) & 0x7fu) <<  7u); if (b >= 0) { break; }
+                b = *p++; val |= ((uint64_t(b) & 0x7fu) << 14u); if (b >= 0) { break; }
+                b = *p++; val |= ((uint64_t(b) & 0x7fu) << 21u); if (b >= 0) { break; }
+                b = *p++; val |= ((uint64_t(b) & 0x7fu) << 28u); if (b >= 0) { break; }
+                b = *p++; val |= ((uint64_t(b) & 0x7fu) << 35u); if (b >= 0) { break; }
+                b = *p++; val |= ((uint64_t(b) & 0x7fu) << 42u); if (b >= 0) { break; }
+                b = *p++; val |= ((uint64_t(b) & 0x7fu) << 49u); if (b >= 0) { break; }
+                b = *p++; val |= ((uint64_t(b) & 0x7fu) << 56u); if (b >= 0) { break; }
+                b = *p++; val |= ((uint64_t(b) & 0x01u) << 63u); if (b >= 0) { break; }
                 throw varint_too_long_exception{};
             } while (false);
         } else {
             unsigned int shift = 0;
             while (p != iend && *p < 0) {
-                val |= uint64_t(*p++ & 0x7fu) << shift;
+                val |= (uint64_t(*p++) & 0x7fu) << shift;
                 shift += 7;
             }
             if (p == iend) {
@@ -88,7 +88,7 @@ namespace detail {
  */
 inline uint64_t decode_varint(const char** data, const char* end) {
     // If this is a one-byte varint, decode it here.
-    if (end != *data && ((**data & 0x80u) == 0)) {
+    if (end != *data && ((static_cast<uint64_t>(**data) & 0x80u) == 0)) {
         const auto val = static_cast<uint64_t>(**data);
         ++(*data);
         return val;
@@ -155,32 +155,49 @@ inline int write_varint(T data, uint64_t value) {
     return n;
 }
 
+/**
+ * Get the length of the varint the specified value would produce.
+ *
+ * @param value The integer to be encoded.
+ * @returns the number of bytes the varint would have if we created it.
+ */
+inline int length_of_varint(uint64_t value) noexcept {
+    int n = 1;
+
+    while (value >= 0x80u) {
+        value >>= 7u;
+        ++n;
+    }
+
+    return n;
+}
+
 /**
  * ZigZag encodes a 32 bit integer.
  */
 inline constexpr uint32_t encode_zigzag32(int32_t value) noexcept {
-    return (static_cast<uint32_t>(value) << 1u) ^ (static_cast<uint32_t>(value >> 31u));
+    return (static_cast<uint32_t>(value) << 1u) ^ static_cast<uint32_t>(-static_cast<int32_t>(static_cast<uint32_t>(value) >> 31u));
 }
 
 /**
  * ZigZag encodes a 64 bit integer.
  */
 inline constexpr uint64_t encode_zigzag64(int64_t value) noexcept {
-    return (static_cast<uint64_t>(value) << 1u) ^ (static_cast<uint64_t>(value >> 63u));
+    return (static_cast<uint64_t>(value) << 1u) ^ static_cast<uint64_t>(-static_cast<int64_t>(static_cast<uint64_t>(value) >> 63u));
 }
 
 /**
  * Decodes a 32 bit ZigZag-encoded integer.
  */
 inline constexpr int32_t decode_zigzag32(uint32_t value) noexcept {
-    return static_cast<int32_t>(value >> 1u) ^ -static_cast<int32_t>(value & 1u);
+    return static_cast<int32_t>((value >> 1u) ^ static_cast<uint32_t>(-static_cast<int32_t>(value & 1u)));
 }
 
 /**
  * Decodes a 64 bit ZigZag-encoded integer.
  */
 inline constexpr int64_t decode_zigzag64(uint64_t value) noexcept {
-    return static_cast<int64_t>(value >> 1u) ^ -static_cast<int64_t>(value & 1u);
+    return static_cast<int64_t>((value >> 1u) ^ static_cast<uint64_t>(-static_cast<int64_t>(value & 1u)));
 }
 
 } // end namespace protozero

include/protozero/version.hpp

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

test/create_pbf_test_data.sh

@@ -8,9 +8,16 @@
 #  If called without a test case it will iterate over all test cases generating
 #  all data.
 #
+#  This program should be called with the "test" directory as current directory.
+#
 
 set -e
 
+if [ -z "$CXX" ]; then
+    echo "Please set CXX before running this script"
+    exit 1
+fi
+
 if [ -z "$1" ]; then
     for dir in t/*; do
         $0 $dir

test/t/complex/reader_test_cases.cpp

@@ -460,10 +460,6 @@ TEST_CASE("write complex data using pbf_writer: all") {
                 REQUIRE(sum == 5);
                 break;
             }
-            case 8: {
-                REQUIRE(item.get_string() == "optionalstring");
-                break;
-            }
             default: {
                 REQUIRE(false); // should not be here
                 break;
@@ -621,10 +617,6 @@ TEST_CASE("write complex data using pbf_builder: all") {
                 REQUIRE(sum == 5);
                 break;
             }
-            case 8: {
-                REQUIRE(item.get_string() == "optionalstring");
-                break;
-            }
             default: {
                 REQUIRE(false); // should not be here
                 break;

test/t/message/reader_test_cases.cpp

@@ -6,10 +6,16 @@ TEST_CASE("read message field: string") {
 
     protozero::pbf_reader item{buffer};
 
+    REQUIRE(item.data().data() == buffer.data());
+    REQUIRE(item.data().size() == buffer.size());
+
     REQUIRE(item.next());
     protozero::pbf_reader subitem{item.get_message()};
     REQUIRE_FALSE(item.next());
 
+    REQUIRE(item.data().data() == buffer.data() + buffer.size());
+    REQUIRE(item.data().empty());
+
     REQUIRE(subitem.next());
     REQUIRE(subitem.get_string() == "foobar");
     REQUIRE_FALSE(subitem.next());