This is an automated email from the git hooks/post-receive script. sebastic pushed a commit to branch master in repository protozero.
commit 99e95a5be58df7a964f5d123bc5c35fd50693f88 Author: Bas Couwenberg <sebas...@xs4all.nl> Date: Fri Mar 9 20:37:11 2018 +0100 New upstream version 1.6.2 --- .clang-tidy | 36 +--- .travis.yml | 2 + CHANGELOG.md | 16 +- CMakeLists.txt | 9 +- README.md | 5 +- appveyor.yml | 6 +- cmake/FindProtozero.cmake | 2 +- include/protozero/byteswap.hpp | 5 +- include/protozero/data_view.hpp | 8 +- include/protozero/exception.hpp | 24 ++- include/protozero/iterators.hpp | 24 +-- include/protozero/pbf_builder.hpp | 4 +- include/protozero/pbf_message.hpp | 6 +- include/protozero/pbf_reader.hpp | 22 +- include/protozero/pbf_writer.hpp | 22 +- include/protozero/types.hpp | 9 +- include/protozero/varint.hpp | 44 ++-- include/protozero/version.hpp | 4 +- test/CMakeLists.txt | 10 +- test/README.md | 10 +- test/catch/catch.hpp | 226 ++++++++++++++------- test/include/packed_access.hpp | 8 +- test/include/scalar_access.hpp | 2 +- test/t/bool/reader_test_cases.cpp | 2 +- test/t/tags/reader_test_cases.cpp | 10 +- test/unit/CMakeLists.txt | 23 +++ test/unit/main.cpp | 4 + .../reader_test_cases.cpp => unit/test_basic.cpp} | 22 +- .../test_data_view.cpp} | 2 +- .../reader_test_cases.cpp => unit/test_endian.cpp} | 0 .../test_exceptions.cpp} | 0 .../reader_test_cases.cpp => unit/test_varint.cpp} | 32 +-- .../reader_test_cases.cpp => unit/test_zigzag.cpp} | 0 tools/pbf-decoder.cpp | 23 ++- 34 files changed, 375 insertions(+), 247 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index c05fe40..1ba914c 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,5 +1,5 @@ --- -Checks: '*,-cert-dcl21-cpp,-cert-err60-cpp,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-type-reinterpret-cast,-google-runtime-references' +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' # # Disabled checks: # @@ -9,42 +9,26 @@ Checks: '*,-cert-dcl21-cpp,-cert-err60-cpp,-cppcoreguidelines-pro-bound # cert-err60-cpp # Reports std::runtime_error as broken which we can't do anything about. # +# cppcoreguidelines-pro-bounds-array-to-pointer-decay +# Limited use and many false positives including for all asserts. +# # cppcoreguidelines-pro-bounds-pointer-arithmetic # This is a low-level library, it needs to do pointer arithmetic. # -# cppcoreguidelines-pro-bounds-array-to-pointer-decay -# Limited use and many false positives including for all asserts -# # cppcoreguidelines-pro-type-reinterpret-cast # This is a low-level library, it needs to do reinterpret-casts. # +# fuchsia-* +# Much too strict. +# # google-runtime-references # This is just a matter of preference, and we can't change the interfaces # now anyways. # +# hicpp-no-array-decay +# Limited use and many false positives including for all asserts. +# WarningsAsErrors: '*' HeaderFilterRegex: '\/include\/' AnalyzeTemporaryDtors: false -CheckOptions: - - key: google-readability-braces-around-statements.ShortStatementLines - value: '1' - - key: google-readability-function-size.StatementThreshold - value: '800' - - key: google-readability-namespace-comments.ShortNamespaceLines - value: '10' - - key: google-readability-namespace-comments.SpacesBeforeComments - value: '2' - - key: modernize-loop-convert.MaxCopySize - value: '16' - - key: modernize-loop-convert.MinConfidence - value: reasonable - - key: modernize-loop-convert.NamingStyle - value: CamelCase - - key: modernize-pass-by-value.IncludeStyle - value: llvm - - key: modernize-replace-auto-ptr.IncludeStyle - value: llvm - - key: modernize-use-nullptr.NullMacros - value: 'NULL' ... - diff --git a/.travis.yml b/.travis.yml index c6be450..891e59b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -89,6 +89,8 @@ matrix: env: BUILD='Debug' CC=clang-5.0 CXX=clang++-5.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 - os: linux compiler: "gcc-4.7" diff --git a/CHANGELOG.md b/CHANGELOG.md index 562bc1a..d43f422 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,19 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +## [1.6.2] - 2018-03-09 + +### Changed + +- Update included catch.hpp to v1.12.0. +- Move basic unit tests into their own directory (`test/unit`). +- Improved clang-tidy config and fixed some code producing warnings. + +### Fixed + +- Buffer overflow in pbf-decoder tool. + + ## [1.6.1] - 2017-11-16 ### Added @@ -288,7 +301,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.1...HEAD +[unreleased]: https://github.com/osmcode/libosmium/compare/v1.6.2...HEAD +[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 [1.5.3]: https://github.com/osmcode/libosmium/compare/v1.5.2...v1.5.3 diff --git a/CMakeLists.txt b/CMakeLists.txt index 9fa22d3..24e293d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,7 +14,7 @@ project(protozero) set(PROTOZERO_VERSION_MAJOR 1) set(PROTOZERO_VERSION_MINOR 6) -set(PROTOZERO_VERSION_PATCH 1) +set(PROTOZERO_VERSION_PATCH 2) set(PROTOZERO_VERSION "${PROTOZERO_VERSION_MAJOR}.${PROTOZERO_VERSION_MINOR}.${PROTOZERO_VERSION_PATCH}") @@ -58,7 +58,7 @@ find_package(Protobuf) # #----------------------------------------------------------------------------- message(STATUS "Looking for clang-tidy") -find_program(CLANG_TIDY NAMES clang-tidy clang-tidy-5.0) +find_program(CLANG_TIDY NAMES clang-tidy clang-tidy-6.0 clang-tidy-5.0) if(CLANG_TIDY) message(STATUS "Looking for clang-tidy - found ${CLANG_TIDY}") @@ -66,8 +66,8 @@ if(CLANG_TIDY) ${CLANG_TIDY} -p ${CMAKE_BINARY_DIR} ${CMAKE_SOURCE_DIR}/test/*.cpp - ${CMAKE_SOURCE_DIR}/test/t/*/reader_test_cases.cpp - ${CMAKE_SOURCE_DIR}/test/t/*/writer_test_cases.cpp + ${CMAKE_SOURCE_DIR}/test/t/*/*.cpp + ${CMAKE_SOURCE_DIR}/test/unit/*.cpp ${CMAKE_SOURCE_DIR}/tools/*.cpp ) add_dependencies(clang-tidy writer_tests) @@ -94,6 +94,7 @@ if(CPPCHECK) ${CMAKE_SOURCE_DIR}/test/*.cpp ${CMAKE_SOURCE_DIR}/test/include/*.hpp ${CMAKE_SOURCE_DIR}/test/t/*/*.cpp + ${CMAKE_SOURCE_DIR}/test/unit/*.cpp ${CMAKE_SOURCE_DIR}/tools/*.cpp ) else() diff --git a/README.md b/README.md index 40a64dd..63972e0 100644 --- a/README.md +++ b/README.md @@ -80,8 +80,8 @@ Extensive tests are included. Build them using CMake: Call `ctest` to run the tests. -The reader tests are always build, the writer tests are only build if the -Google Protobuf library is found when running CMake. +The unit and reader tests are always build, the writer tests are only build if +the Google Protobuf library is found when running CMake. See `test/README.md` for more details about the test. @@ -148,6 +148,7 @@ details. * [Mapnik](https://github.com/mapbox/mapnik-vector-tile) * [OSRM](https://github.com/Project-OSRM/osrm-backend) * [Tippecanoe](https://github.com/mapbox/tippecanoe) +* [Vtzero](https://github.com/mapbox/vtzero) Are you using Protozero? Tell us! Send a pull request with changes to this README. diff --git a/appveyor.yml b/appveyor.yml index 2770011..3c3dc7b 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -31,9 +31,13 @@ init: - git config --global core.autocrlf %autocrlf% - git config --get core.autocrlf +# The option --ask=20 is a workaround for a problem with the MSYS2 update +# process. Without it the following error is printed and the appveyor script +# halts: "msys2-runtime and catgets are in conflict. Remove catgets?" +# See also: https://github.com/Alexpux/MSYS2-packages/issues/1141 install: - if [%config%]==[MSYS2] ( - C:\msys64\usr\bin\pacman --noconfirm --sync --refresh --refresh --sysupgrade --sysupgrade + C:\msys64\usr\bin\pacman --noconfirm --sync --refresh --refresh --sysupgrade --sysupgrade --ask=20 && C:\msys64\usr\bin\pacman -Rc --noconfirm mingw-w64-x86_64-gcc-libs ) diff --git a/cmake/FindProtozero.cmake b/cmake/FindProtozero.cmake index 175dd23..ad16cab 100644 --- a/cmake/FindProtozero.cmake +++ b/cmake/FindProtozero.cmake @@ -39,7 +39,7 @@ # find include path find_path(PROTOZERO_INCLUDE_DIR protozero/version.hpp PATH_SUFFIXES include - PATHS ../protozero + PATHS ${CMAKE_SOURCE_DIR}/../protozero ) # Check version number diff --git a/include/protozero/byteswap.hpp b/include/protozero/byteswap.hpp index bca4844..fd8a83a 100644 --- a/include/protozero/byteswap.hpp +++ b/include/protozero/byteswap.hpp @@ -16,11 +16,10 @@ documentation. * @brief Contains functions to swap bytes in values (for different endianness). */ -#include <cassert> -#include <cstdint> - #include <protozero/config.hpp> +#include <cstdint> + namespace protozero { namespace detail { diff --git a/include/protozero/data_view.hpp b/include/protozero/data_view.hpp index 144025d..952c912 100644 --- a/include/protozero/data_view.hpp +++ b/include/protozero/data_view.hpp @@ -16,14 +16,14 @@ documentation. * @brief Contains the implementation of the data_view class. */ +#include <protozero/config.hpp> + #include <algorithm> #include <cstddef> #include <cstring> #include <string> #include <utility> -#include <protozero/config.hpp> - namespace protozero { #ifdef PROTOZERO_USE_VIEW @@ -64,7 +64,7 @@ public: * * @param str String with the data. */ - data_view(const std::string& str) noexcept // NOLINT clang-tidy: google-explicit-constructor + data_view(const std::string& str) noexcept // NOLINT(google-explicit-constructor, hicpp-explicit-conversions) : m_data(str.data()), m_size(str.size()) { } @@ -74,7 +74,7 @@ public: * * @param ptr Pointer to the data. */ - data_view(const char* ptr) noexcept // NOLINT clang-tidy: google-explicit-constructor + data_view(const char* ptr) noexcept // NOLINT(google-explicit-constructor, hicpp-explicit-conversions) : m_data(ptr), m_size(std::strlen(ptr)) { } diff --git a/include/protozero/exception.hpp b/include/protozero/exception.hpp index 8a20c94..a3cd0f1 100644 --- a/include/protozero/exception.hpp +++ b/include/protozero/exception.hpp @@ -29,7 +29,9 @@ namespace protozero { */ struct exception : std::exception { /// Returns the explanatory string. - const char* what() const noexcept override { return "pbf exception"; } + const char* what() const noexcept override { + return "pbf exception"; + } }; /** @@ -38,7 +40,9 @@ struct exception : std::exception { */ struct varint_too_long_exception : exception { /// Returns the explanatory string. - const char* what() const noexcept override { return "varint too long exception"; } + const char* what() const noexcept override { + return "varint too long exception"; + } }; /** @@ -47,7 +51,9 @@ struct varint_too_long_exception : exception { */ struct unknown_pbf_wire_type_exception : exception { /// Returns the explanatory string. - const char* what() const noexcept override { return "unknown pbf field type exception"; } + const char* what() const noexcept override { + return "unknown pbf field type exception"; + } }; /** @@ -60,7 +66,9 @@ struct unknown_pbf_wire_type_exception : exception { */ struct end_of_buffer_exception : exception { /// Returns the explanatory string. - const char* what() const noexcept override { return "end of buffer exception"; } + const char* what() const noexcept override { + return "end of buffer exception"; + } }; /** @@ -71,7 +79,9 @@ struct end_of_buffer_exception : exception { */ struct invalid_tag_exception : exception { /// Returns the explanatory string. - const char* what() const noexcept override { return "invalid tag exception"; } + const char* what() const noexcept override { + return "invalid tag exception"; + } }; /** @@ -81,7 +91,9 @@ struct invalid_tag_exception : exception { */ struct invalid_length_exception : exception { /// Returns the explanatory string. - const char* what() const noexcept override { return "invalid length exception"; } + const char* what() const noexcept override { + return "invalid length exception"; + } }; } // end namespace protozero diff --git a/include/protozero/iterators.hpp b/include/protozero/iterators.hpp index 2361c99..c1d7d8d 100644 --- a/include/protozero/iterators.hpp +++ b/include/protozero/iterators.hpp @@ -16,11 +16,6 @@ documentation. * @brief Contains the iterators for access to packed repeated fields. */ -#include <algorithm> -#include <cstring> -#include <iterator> -#include <utility> - #include <protozero/config.hpp> #include <protozero/varint.hpp> @@ -28,6 +23,11 @@ documentation. # include <protozero/byteswap.hpp> #endif +#include <algorithm> +#include <cstring> +#include <iterator> +#include <utility> + namespace protozero { /** @@ -306,7 +306,7 @@ public: // significant bit not set. We can use this to quickly figure out // how many varints there are without actually decoding the varints. return std::count_if(begin.m_data, end.m_data, [](char c) noexcept { - return (static_cast<unsigned char>(c) & 0x80) == 0; + return (static_cast<unsigned char>(c) & 0x80u) == 0; }); } @@ -410,42 +410,42 @@ namespace std { 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 clang-tidy: readability-inconsistent-declaration-parameter-name + distance<protozero::const_varint_iterator<int32_t>>(protozero::const_varint_iterator<int32_t> first, // NOLINT(readability-inconsistent-declaration-parameter-name) protozero::const_varint_iterator<int32_t> last) { return protozero::const_varint_iterator<int32_t>::distance(first, last); } template <> inline typename protozero::const_varint_iterator<int64_t>::difference_type - distance<protozero::const_varint_iterator<int64_t>>(protozero::const_varint_iterator<int64_t> first, // NOLINT clang-tidy: readability-inconsistent-declaration-parameter-name + distance<protozero::const_varint_iterator<int64_t>>(protozero::const_varint_iterator<int64_t> first, // NOLINT(readability-inconsistent-declaration-parameter-name) protozero::const_varint_iterator<int64_t> last) { return protozero::const_varint_iterator<int64_t>::distance(first, last); } template <> inline typename protozero::const_varint_iterator<uint32_t>::difference_type - distance<protozero::const_varint_iterator<uint32_t>>(protozero::const_varint_iterator<uint32_t> first, // NOLINT clang-tidy: readability-inconsistent-declaration-parameter-name + distance<protozero::const_varint_iterator<uint32_t>>(protozero::const_varint_iterator<uint32_t> first, // NOLINT(readability-inconsistent-declaration-parameter-name) protozero::const_varint_iterator<uint32_t> last) { return protozero::const_varint_iterator<uint32_t>::distance(first, last); } template <> inline typename protozero::const_varint_iterator<uint64_t>::difference_type - distance<protozero::const_varint_iterator<uint64_t>>(protozero::const_varint_iterator<uint64_t> first, // NOLINT clang-tidy: readability-inconsistent-declaration-parameter-name + distance<protozero::const_varint_iterator<uint64_t>>(protozero::const_varint_iterator<uint64_t> first, // NOLINT(readability-inconsistent-declaration-parameter-name) protozero::const_varint_iterator<uint64_t> last) { return protozero::const_varint_iterator<uint64_t>::distance(first, last); } template <> inline typename protozero::const_svarint_iterator<int32_t>::difference_type - distance<protozero::const_svarint_iterator<int32_t>>(protozero::const_svarint_iterator<int32_t> first, // NOLINT clang-tidy: readability-inconsistent-declaration-parameter-name + distance<protozero::const_svarint_iterator<int32_t>>(protozero::const_svarint_iterator<int32_t> first, // NOLINT(readability-inconsistent-declaration-parameter-name) protozero::const_svarint_iterator<int32_t> last) { return protozero::const_svarint_iterator<int32_t>::distance(first, last); } template <> inline typename protozero::const_svarint_iterator<int64_t>::difference_type - distance<protozero::const_svarint_iterator<int64_t>>(protozero::const_svarint_iterator<int64_t> first, // NOLINT clang-tidy: readability-inconsistent-declaration-parameter-name + distance<protozero::const_svarint_iterator<int64_t>>(protozero::const_svarint_iterator<int64_t> first, // NOLINT(readability-inconsistent-declaration-parameter-name) protozero::const_svarint_iterator<int64_t> last) { return protozero::const_svarint_iterator<int64_t>::distance(first, last); } diff --git a/include/protozero/pbf_builder.hpp b/include/protozero/pbf_builder.hpp index aa4f545..2e74b2f 100644 --- a/include/protozero/pbf_builder.hpp +++ b/include/protozero/pbf_builder.hpp @@ -16,11 +16,11 @@ documentation. * @brief Contains the pbf_builder template class. */ -#include <type_traits> - #include <protozero/pbf_writer.hpp> #include <protozero/types.hpp> +#include <type_traits> + namespace protozero { /** diff --git a/include/protozero/pbf_message.hpp b/include/protozero/pbf_message.hpp index 8adf207..10040d5 100644 --- a/include/protozero/pbf_message.hpp +++ b/include/protozero/pbf_message.hpp @@ -16,11 +16,11 @@ documentation. * @brief Contains the pbf_message template class. */ -#include <type_traits> - #include <protozero/pbf_reader.hpp> #include <protozero/types.hpp> +#include <type_traits> + namespace protozero { /** @@ -77,7 +77,7 @@ public: * parent class. */ template <typename... Args> - pbf_message(Args&&... args) noexcept : // NOLINT clang-tidy: google-explicit-constructor + pbf_message(Args&&... args) noexcept : // NOLINT(google-explicit-constructor, hicpp-explicit-conversions) pbf_reader(std::forward<Args>(args)...) { } diff --git a/include/protozero/pbf_reader.hpp b/include/protozero/pbf_reader.hpp index 6fb90bd..5f8ea0e 100644 --- a/include/protozero/pbf_reader.hpp +++ b/include/protozero/pbf_reader.hpp @@ -16,12 +16,6 @@ documentation. * @brief Contains the pbf_reader class. */ -#include <cstddef> -#include <cstdint> -#include <cstring> -#include <string> -#include <utility> - #include <protozero/config.hpp> #include <protozero/data_view.hpp> #include <protozero/exception.hpp> @@ -33,6 +27,12 @@ documentation. # include <protozero/byteswap.hpp> #endif +#include <cstddef> +#include <cstdint> +#include <cstring> +#include <string> +#include <utility> + namespace protozero { /** @@ -117,9 +117,9 @@ class pbf_reader { } m_data += len; - // In debug builds reset the tag to zero so that we can detect (some) - // wrong code. #ifndef NDEBUG + // In debug builds reset the tag to zero so that we can detect (some) + // wrong code. m_tag = 0; #endif } @@ -241,7 +241,7 @@ public: * are still fields available and to `false` if the last field has been * read. */ - operator bool() const noexcept { // NOLINT clang-tidy: google-explicit-constructor + operator bool() const noexcept { // NOLINT(google-explicit-constructor, hicpp-explicit-conversions) return m_data < m_end; } @@ -279,7 +279,7 @@ public: } const auto value = get_varint<uint32_t>(); - m_tag = pbf_tag_type(value >> 3); + m_tag = pbf_tag_type(value >> 3u); // tags 0 and 19000 to 19999 are not allowed as per // https://developers.google.com/protocol-buffers/docs/proto#assigning-tags @@ -287,7 +287,7 @@ public: throw invalid_tag_exception{}; } - m_wire_type = pbf_wire_type(value & 0x07); + m_wire_type = pbf_wire_type(value & 0x07u); switch (m_wire_type) { case pbf_wire_type::varint: case pbf_wire_type::fixed64: diff --git a/include/protozero/pbf_writer.hpp b/include/protozero/pbf_writer.hpp index edf5522..1abe9a3 100644 --- a/include/protozero/pbf_writer.hpp +++ b/include/protozero/pbf_writer.hpp @@ -16,15 +16,6 @@ documentation. * @brief Contains the pbf_writer class. */ -#include <cstddef> -#include <cstdint> -#include <cstring> -#include <initializer_list> -#include <iterator> -#include <limits> -#include <string> -#include <utility> - #include <protozero/config.hpp> #include <protozero/data_view.hpp> #include <protozero/types.hpp> @@ -34,6 +25,15 @@ documentation. # include <protozero/byteswap.hpp> #endif +#include <cstddef> +#include <cstdint> +#include <cstring> +#include <initializer_list> +#include <iterator> +#include <limits> +#include <string> +#include <utility> + namespace protozero { namespace detail { @@ -78,8 +78,8 @@ class pbf_writer { } void add_field(pbf_tag_type tag, pbf_wire_type type) { - protozero_assert(((tag > 0 && tag < 19000) || (tag > 19999 && tag <= ((1 << 29) - 1))) && "tag out of range"); - const uint32_t b = (tag << 3) | uint32_t(type); + protozero_assert(((tag > 0 && tag < 19000) || (tag > 19999 && tag <= ((1u << 29u) - 1))) && "tag out of range"); + const uint32_t b = (tag << 3u) | uint32_t(type); add_varint(b); } diff --git a/include/protozero/types.hpp b/include/protozero/types.hpp index 0d810e9..b524127 100644 --- a/include/protozero/types.hpp +++ b/include/protozero/types.hpp @@ -16,6 +16,8 @@ documentation. * @brief Contains the declaration of low-level types used in the pbf format. */ +#include <protozero/config.hpp> + #include <algorithm> #include <cstddef> #include <cstdint> @@ -23,8 +25,6 @@ documentation. #include <string> #include <utility> -#include <protozero/config.hpp> - namespace protozero { /** @@ -40,8 +40,7 @@ using pbf_tag_type = uint32_t; enum class pbf_wire_type : uint32_t { varint = 0, // int32/64, uint32/64, sint32/64, bool, enum fixed64 = 1, // fixed64, sfixed64, double - length_delimited = 2, // string, bytes, embedded messages, - // packed repeated fields + length_delimited = 2, // string, bytes, nested messages, packed repeated fields fixed32 = 5, // fixed32, sfixed32, float unknown = 99 // used for default setting in this library }; @@ -54,7 +53,7 @@ enum class pbf_wire_type : uint32_t { */ template <typename T> constexpr inline uint32_t tag_and_type(T tag, pbf_wire_type wire_type) noexcept { - return (static_cast<uint32_t>(static_cast<pbf_tag_type>(tag)) << 3) | static_cast<uint32_t>(wire_type); + return (static_cast<uint32_t>(static_cast<pbf_tag_type>(tag)) << 3u) | static_cast<uint32_t>(wire_type); } /** diff --git a/include/protozero/varint.hpp b/include/protozero/varint.hpp index 21d170e..dd79a6e 100644 --- a/include/protozero/varint.hpp +++ b/include/protozero/varint.hpp @@ -16,10 +16,10 @@ documentation. * @brief Contains low-level varint and zigzag encoding and decoding functions. */ -#include <cstdint> - #include <protozero/exception.hpp> +#include <cstdint> + namespace protozero { /** @@ -39,22 +39,22 @@ namespace detail { if (iend - begin >= max_varint_length) { // fast path do { int64_t b; - b = *p++; val = uint64_t((b & 0x7f) ); if (b >= 0) { break; } - b = *p++; val |= uint64_t((b & 0x7f) << 7); if (b >= 0) { break; } - b = *p++; val |= uint64_t((b & 0x7f) << 14); if (b >= 0) { break; } - b = *p++; val |= uint64_t((b & 0x7f) << 21); if (b >= 0) { break; } - b = *p++; val |= uint64_t((b & 0x7f) << 28); if (b >= 0) { break; } - b = *p++; val |= uint64_t((b & 0x7f) << 35); if (b >= 0) { break; } - b = *p++; val |= uint64_t((b & 0x7f) << 42); if (b >= 0) { break; } - b = *p++; val |= uint64_t((b & 0x7f) << 49); if (b >= 0) { break; } - b = *p++; val |= uint64_t((b & 0x7f) << 56); if (b >= 0) { break; } - b = *p++; val |= uint64_t((b & 0x01) << 63); 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 { - int shift = 0; + unsigned int shift = 0; while (p != iend && *p < 0) { - val |= uint64_t(*p++ & 0x7f) << 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 & 0x80) == 0)) { + if (end != *data && ((**data & 0x80u) == 0)) { const auto val = static_cast<uint64_t>(**data); ++(*data); return val; @@ -145,9 +145,9 @@ template <typename T> inline int write_varint(T data, uint64_t value) { int n = 1; - while (value >= 0x80) { - *data++ = char((value & 0x7f) | 0x80); - value >>= 7; + while (value >= 0x80u) { + *data++ = char((value & 0x7fu) | 0x80u); + value >>= 7u; ++n; } *data++ = char(value); @@ -159,28 +159,28 @@ inline int write_varint(T data, uint64_t value) { * ZigZag encodes a 32 bit integer. */ inline constexpr uint32_t encode_zigzag32(int32_t value) noexcept { - return (static_cast<uint32_t>(value) << 1) ^ (static_cast<uint32_t>(value >> 31)); + return (static_cast<uint32_t>(value) << 1u) ^ (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) << 1) ^ (static_cast<uint64_t>(value >> 63)); + return (static_cast<uint64_t>(value) << 1u) ^ (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 >> 1) ^ -static_cast<int32_t>(value & 1); + return static_cast<int32_t>(value >> 1u) ^ -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 >> 1) ^ -static_cast<int64_t>(value & 1); + return static_cast<int64_t>(value >> 1u) ^ -static_cast<int64_t>(value & 1u); } } // end namespace protozero diff --git a/include/protozero/version.hpp b/include/protozero/version.hpp index edf1bf3..32a494f 100644 --- a/include/protozero/version.hpp +++ b/include/protozero/version.hpp @@ -23,12 +23,12 @@ documentation. #define PROTOZERO_VERSION_MINOR 6 /// The patch number -#define PROTOZERO_VERSION_PATCH 1 +#define PROTOZERO_VERSION_PATCH 2 /// 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.1" +#define PROTOZERO_VERSION_STRING "1.6.2" #endif // PROTOZERO_VERSION_HPP diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 9e03024..c769123 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -9,16 +9,14 @@ include_directories(SYSTEM "${CMAKE_CURRENT_SOURCE_DIR}/catch") include_directories("${CMAKE_CURRENT_SOURCE_DIR}/include") +add_subdirectory(unit) + set(TEST_DIRS alignment - basic bool bytes complex - data_view double - endian enum - exceptions fixed32 fixed64 float @@ -52,10 +50,8 @@ set(TEST_DIRS alignment tags uint32 uint64 - varint vector_tile - wrong_type_access - zigzag) + wrong_type_access) string(REGEX REPLACE "([^;]+)" "t/\\1/reader_test_cases.cpp" _test_sources "${TEST_DIRS}") diff --git a/test/README.md b/test/README.md index 36d5751..843d8d4 100644 --- a/test/README.md +++ b/test/README.md @@ -3,8 +3,16 @@ Tests are using the [Catch Unit Test Framework](https://github.com/philsquared/Catch). +## Organization of the unit tests -## Organization of the test cases +Unit tests test low-level functions of the library. They are in the `unit` +directory. + + +## Organization of the reader/writer test cases + +The hart of the tests are the reader/writer tests checking all aspects of +decoding and encoding protobuf files. Each test case is in its own directory under the `t` directory. Each directory contains (some of) the following files: diff --git a/test/catch/catch.hpp b/test/catch/catch.hpp index 4d1fc33..6b5129d 100644 --- a/test/catch/catch.hpp +++ b/test/catch/catch.hpp @@ -1,6 +1,6 @@ /* - * Catch v1.10.0 - * Generated: 2017-08-26 15:16:46.676990 + * Catch v1.12.0 + * Generated: 2018-01-11 21:56:34.893972 * ---------------------------------------------------------- * This file has been merged from multiple headers. Please don't edit it directly * Copyright (c) 2012 Two Blue Cubes Ltd. All rights reserved. @@ -228,7 +228,12 @@ ( defined __GNUC__ && ( __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3 )) ) || \ ( defined __clang__ && __clang_major__ >= 3 ) -#define CATCH_INTERNAL_CONFIG_COUNTER +// Use of __COUNTER__ is suppressed during code analysis in CLion/AppCode 2017.2.x and former, +// because __COUNTER__ is not properly handled by it. +// This does not affect compilation +#if ( !defined __JETBRAINS_IDE__ || __JETBRAINS_IDE__ >= 20170300L ) + #define CATCH_INTERNAL_CONFIG_COUNTER +#endif #endif @@ -309,10 +314,7 @@ #if defined(CATCH_INTERNAL_CONFIG_CPP11_UNIQUE_PTR) && !defined(CATCH_CONFIG_CPP11_NO_UNIQUE_PTR) && !defined(CATCH_CONFIG_CPP11_UNIQUE_PTR) && !defined(CATCH_CONFIG_NO_CPP11) # define CATCH_CONFIG_CPP11_UNIQUE_PTR #endif -// Use of __COUNTER__ is suppressed if __JETBRAINS_IDE__ is #defined (meaning we're being parsed by a JetBrains IDE for -// analytics) because, at time of writing, __COUNTER__ is not properly handled by it. -// This does not affect compilation -#if defined(CATCH_INTERNAL_CONFIG_COUNTER) && !defined(CATCH_CONFIG_NO_COUNTER) && !defined(CATCH_CONFIG_COUNTER) && !defined(__JETBRAINS_IDE__) +#if defined(CATCH_INTERNAL_CONFIG_COUNTER) && !defined(CATCH_CONFIG_NO_COUNTER) && !defined(CATCH_CONFIG_COUNTER) # define CATCH_CONFIG_COUNTER #endif #if defined(CATCH_INTERNAL_CONFIG_CPP11_SHUFFLE) && !defined(CATCH_CONFIG_CPP11_NO_SHUFFLE) && !defined(CATCH_CONFIG_CPP11_SHUFFLE) && !defined(CATCH_CONFIG_NO_CPP11) @@ -1318,10 +1320,12 @@ namespace Internal { template<> struct OperatorTraits<IsGreaterThanOrEqualTo>{ static const char* getName(){ return ">="; } }; template<typename T> - T& removeConst(T const &t) { return const_cast<T&>(t); } + T& opCast(T const& t) { return const_cast<T&>(t); } + +// nullptr_t support based on pull request #154 from Konstantin Baumann #ifdef CATCH_CONFIG_CPP11_NULLPTR - inline std::nullptr_t removeConst(std::nullptr_t) { return nullptr; } -#endif + inline std::nullptr_t opCast(std::nullptr_t) { return nullptr; } +#endif // CATCH_CONFIG_CPP11_NULLPTR // So the compare overloads can be operator agnostic we convey the operator as a template // enum, which is used to specialise an Evaluator for doing the comparison. @@ -1331,90 +1335,161 @@ namespace Internal { template<typename T1, typename T2> struct Evaluator<T1, T2, IsEqualTo> { static bool evaluate( T1 const& lhs, T2 const& rhs) { - return bool(removeConst(lhs) == removeConst(rhs) ); + return bool( opCast( lhs ) == opCast( rhs ) ); } }; template<typename T1, typename T2> struct Evaluator<T1, T2, IsNotEqualTo> { static bool evaluate( T1 const& lhs, T2 const& rhs ) { - return bool(removeConst(lhs) != removeConst(rhs) ); + return bool( opCast( lhs ) != opCast( rhs ) ); } }; template<typename T1, typename T2> struct Evaluator<T1, T2, IsLessThan> { static bool evaluate( T1 const& lhs, T2 const& rhs ) { - return bool(removeConst(lhs) < removeConst(rhs) ); + return bool( opCast( lhs ) < opCast( rhs ) ); } }; template<typename T1, typename T2> struct Evaluator<T1, T2, IsGreaterThan> { static bool evaluate( T1 const& lhs, T2 const& rhs ) { - return bool(removeConst(lhs) > removeConst(rhs) ); + return bool( opCast( lhs ) > opCast( rhs ) ); } }; template<typename T1, typename T2> struct Evaluator<T1, T2, IsGreaterThanOrEqualTo> { static bool evaluate( T1 const& lhs, T2 const& rhs ) { - return bool(removeConst(lhs) >= removeConst(rhs) ); + return bool( opCast( lhs ) >= opCast( rhs ) ); } }; template<typename T1, typename T2> struct Evaluator<T1, T2, IsLessThanOrEqualTo> { static bool evaluate( T1 const& lhs, T2 const& rhs ) { - return bool(removeConst(lhs) <= removeConst(rhs) ); + return bool( opCast( lhs ) <= opCast( rhs ) ); } }; - // Special case for comparing a pointer to an int (deduced for p==0) - template<typename T> - struct Evaluator<int const&, T* const&, IsEqualTo> { - static bool evaluate( int lhs, T* rhs) { - return reinterpret_cast<void const*>( lhs ) == rhs; - } - }; - template<typename T> - struct Evaluator<T* const&, int const&, IsEqualTo> { - static bool evaluate( T* lhs, int rhs) { - return lhs == reinterpret_cast<void const*>( rhs ); - } - }; - template<typename T> - struct Evaluator<int const&, T* const&, IsNotEqualTo> { - static bool evaluate( int lhs, T* rhs) { - return reinterpret_cast<void const*>( lhs ) != rhs; - } - }; - template<typename T> - struct Evaluator<T* const&, int const&, IsNotEqualTo> { - static bool evaluate( T* lhs, int rhs) { - return lhs != reinterpret_cast<void const*>( rhs ); - } - }; + template<Operator Op, typename T1, typename T2> + bool applyEvaluator( T1 const& lhs, T2 const& rhs ) { + return Evaluator<T1, T2, Op>::evaluate( lhs, rhs ); + } - template<typename T> - struct Evaluator<long const&, T* const&, IsEqualTo> { - static bool evaluate( long lhs, T* rhs) { - return reinterpret_cast<void const*>( lhs ) == rhs; - } - }; - template<typename T> - struct Evaluator<T* const&, long const&, IsEqualTo> { - static bool evaluate( T* lhs, long rhs) { - return lhs == reinterpret_cast<void const*>( rhs ); - } - }; - template<typename T> - struct Evaluator<long const&, T* const&, IsNotEqualTo> { - static bool evaluate( long lhs, T* rhs) { - return reinterpret_cast<void const*>( lhs ) != rhs; - } - }; - template<typename T> - struct Evaluator<T* const&, long const&, IsNotEqualTo> { - static bool evaluate( T* lhs, long rhs) { - return lhs != reinterpret_cast<void const*>( rhs ); - } - }; + // This level of indirection allows us to specialise for integer types + // to avoid signed/ unsigned warnings + + // "base" overload + template<Operator Op, typename T1, typename T2> + bool compare( T1 const& lhs, T2 const& rhs ) { + return Evaluator<T1, T2, Op>::evaluate( lhs, rhs ); + } + + // unsigned X to int + template<Operator Op> bool compare( unsigned int lhs, int rhs ) { + return applyEvaluator<Op>( lhs, static_cast<unsigned int>( rhs ) ); + } + template<Operator Op> bool compare( unsigned long lhs, int rhs ) { + return applyEvaluator<Op>( lhs, static_cast<unsigned int>( rhs ) ); + } + template<Operator Op> bool compare( unsigned char lhs, int rhs ) { + return applyEvaluator<Op>( lhs, static_cast<unsigned int>( rhs ) ); + } + + // unsigned X to long + template<Operator Op> bool compare( unsigned int lhs, long rhs ) { + return applyEvaluator<Op>( lhs, static_cast<unsigned long>( rhs ) ); + } + template<Operator Op> bool compare( unsigned long lhs, long rhs ) { + return applyEvaluator<Op>( lhs, static_cast<unsigned long>( rhs ) ); + } + template<Operator Op> bool compare( unsigned char lhs, long rhs ) { + return applyEvaluator<Op>( lhs, static_cast<unsigned long>( rhs ) ); + } + + // int to unsigned X + template<Operator Op> bool compare( int lhs, unsigned int rhs ) { + return applyEvaluator<Op>( static_cast<unsigned int>( lhs ), rhs ); + } + template<Operator Op> bool compare( int lhs, unsigned long rhs ) { + return applyEvaluator<Op>( static_cast<unsigned int>( lhs ), rhs ); + } + template<Operator Op> bool compare( int lhs, unsigned char rhs ) { + return applyEvaluator<Op>( static_cast<unsigned int>( lhs ), rhs ); + } + + // long to unsigned X + template<Operator Op> bool compare( long lhs, unsigned int rhs ) { + return applyEvaluator<Op>( static_cast<unsigned long>( lhs ), rhs ); + } + template<Operator Op> bool compare( long lhs, unsigned long rhs ) { + return applyEvaluator<Op>( static_cast<unsigned long>( lhs ), rhs ); + } + template<Operator Op> bool compare( long lhs, unsigned char rhs ) { + return applyEvaluator<Op>( static_cast<unsigned long>( lhs ), rhs ); + } + + // pointer to long (when comparing against NULL) + template<Operator Op, typename T> bool compare( long lhs, T* rhs ) { + return Evaluator<T*, T*, Op>::evaluate( reinterpret_cast<T*>( lhs ), rhs ); + } + template<Operator Op, typename T> bool compare( T* lhs, long rhs ) { + return Evaluator<T*, T*, Op>::evaluate( lhs, reinterpret_cast<T*>( rhs ) ); + } + + // pointer to int (when comparing against NULL) + template<Operator Op, typename T> bool compare( int lhs, T* rhs ) { + return Evaluator<T*, T*, Op>::evaluate( reinterpret_cast<T*>( lhs ), rhs ); + } + template<Operator Op, typename T> bool compare( T* lhs, int rhs ) { + return Evaluator<T*, T*, Op>::evaluate( lhs, reinterpret_cast<T*>( rhs ) ); + } + +#ifdef CATCH_CONFIG_CPP11_LONG_LONG + // long long to unsigned X + template<Operator Op> bool compare( long long lhs, unsigned int rhs ) { + return applyEvaluator<Op>( static_cast<unsigned long>( lhs ), rhs ); + } + template<Operator Op> bool compare( long long lhs, unsigned long rhs ) { + return applyEvaluator<Op>( static_cast<unsigned long>( lhs ), rhs ); + } + template<Operator Op> bool compare( long long lhs, unsigned long long rhs ) { + return applyEvaluator<Op>( static_cast<unsigned long>( lhs ), rhs ); + } + template<Operator Op> bool compare( long long lhs, unsigned char rhs ) { + return applyEvaluator<Op>( static_cast<unsigned long>( lhs ), rhs ); + } + + // unsigned long long to X + template<Operator Op> bool compare( unsigned long long lhs, int rhs ) { + return applyEvaluator<Op>( static_cast<long>( lhs ), rhs ); + } + template<Operator Op> bool compare( unsigned long long lhs, long rhs ) { + return applyEvaluator<Op>( static_cast<long>( lhs ), rhs ); + } + template<Operator Op> bool compare( unsigned long long lhs, long long rhs ) { + return applyEvaluator<Op>( static_cast<long>( lhs ), rhs ); + } + template<Operator Op> bool compare( unsigned long long lhs, char rhs ) { + return applyEvaluator<Op>( static_cast<long>( lhs ), rhs ); + } + + // pointer to long long (when comparing against NULL) + template<Operator Op, typename T> bool compare( long long lhs, T* rhs ) { + return Evaluator<T*, T*, Op>::evaluate( reinterpret_cast<T*>( lhs ), rhs ); + } + template<Operator Op, typename T> bool compare( T* lhs, long long rhs ) { + return Evaluator<T*, T*, Op>::evaluate( lhs, reinterpret_cast<T*>( rhs ) ); + } +#endif // CATCH_CONFIG_CPP11_LONG_LONG + +#ifdef CATCH_CONFIG_CPP11_NULLPTR + // pointer to nullptr_t (when comparing against nullptr) + template<Operator Op, typename T> bool compare( std::nullptr_t, T* rhs ) { + return Evaluator<T*, T*, Op>::evaluate( nullptr, rhs ); + } + template<Operator Op, typename T> bool compare( T* lhs, std::nullptr_t ) { + return Evaluator<T*, T*, Op>::evaluate( lhs, nullptr ); + } +#endif // CATCH_CONFIG_CPP11_NULLPTR } // end of namespace Internal } // end of namespace Catch @@ -1835,7 +1910,7 @@ public: void endExpression() const { m_rb - .setResultType( Internal::Evaluator<LhsT, RhsT, Op>::evaluate( m_lhs, m_rhs ) ) + .setResultType( Internal::compare<Op>( m_lhs, m_rhs ) ) .endExpression( *this ); } @@ -2756,7 +2831,8 @@ namespace Detail { if (relativeOK) { return true; } - return std::fabs(lhs_v - rhs.m_value) < rhs.m_margin; + + return std::fabs(lhs_v - rhs.m_value) <= rhs.m_margin; } template <typename T, typename = typename std::enable_if<std::is_constructible<double, T>::value>::type> @@ -2828,7 +2904,7 @@ namespace Detail { if (relativeOK) { return true; } - return std::fabs(lhs - rhs.m_value) < rhs.m_margin; + return std::fabs(lhs - rhs.m_value) <= rhs.m_margin; } friend bool operator == ( Approx const& lhs, double rhs ) { @@ -5279,10 +5355,6 @@ namespace Catch { .describe( "should output be colourised" ) .bind( &setUseColour, "yes|no" ); - cli["--use-colour"] - .describe( "should output be colourised" ) - .bind( &setUseColour, "yes|no" ); - cli["--libidentify"] .describe( "report name and version according to libidentify standard" ) .bind( &ConfigData::libIdentify ); @@ -7215,7 +7287,7 @@ namespace Catch { namespace Catch { struct RandomNumberGenerator { - typedef std::ptrdiff_t result_type; + typedef unsigned int result_type; result_type operator()( result_type n ) const { return std::rand() % n; } @@ -8136,7 +8208,7 @@ namespace Catch { std::string AssertionResult::getExpression() const { if( isFalseTest( m_info.resultDisposition ) ) - return '!' + capturedExpressionWithSecondArgument(m_info.capturedExpression, m_info.secondArg); + return "!(" + capturedExpressionWithSecondArgument(m_info.capturedExpression, m_info.secondArg) + ")"; else return capturedExpressionWithSecondArgument(m_info.capturedExpression, m_info.secondArg); } @@ -8394,7 +8466,7 @@ namespace Catch { } inline Version libraryVersion() { - static Version version( 1, 10, 0, "", 0 ); + static Version version( 1, 12, 0, "", 0 ); return version; } @@ -10208,12 +10280,12 @@ namespace Catch { bool includeResults = m_config->includeSuccessfulResults() || !result.isOk(); - if( includeResults ) { + if( includeResults || result.getResultType() == ResultWas::Warning ) { // Print any info messages in <Info> tags. for( std::vector<MessageInfo>::const_iterator it = assertionStats.infoMessages.begin(), itEnd = assertionStats.infoMessages.end(); it != itEnd; ++it ) { - if( it->type == ResultWas::Info ) { + if( it->type == ResultWas::Info && includeResults ) { m_xml.scopedElement( "Info" ) .writeText( it->message ); } else if ( it->type == ResultWas::Warning ) { diff --git a/test/include/packed_access.hpp b/test/include/packed_access.hpp index 051c1cc..1e9edb2 100644 --- a/test/include/packed_access.hpp +++ b/test/include/packed_access.hpp @@ -1,4 +1,4 @@ -// NOLINT clang-tidy: llvm-header-guard +// NOLINT(llvm-header-guard) #define PBF_TYPE_NAME PROTOZERO_TEST_STRING(PBF_TYPE) #define GET_TYPE PROTOZERO_TEST_CONCAT(get_packed_, PBF_TYPE) @@ -204,7 +204,7 @@ TEST_CASE("move repeated packed field: " PBF_TYPE_NAME) { field = std::move(field2); - REQUIRE_FALSE(field2.valid()); // NOLINT clang-tidy: hicpp-invalid-access-moved + REQUIRE_FALSE(field2.valid()); // NOLINT(hicpp-invalid-access-moved, bugprone-use-after-move) REQUIRE(field.valid()); field.add_element(cpp_type(17)); @@ -216,7 +216,7 @@ TEST_CASE("move repeated packed field: " PBF_TYPE_NAME) { packed_field_type field{std::move(field2)}; REQUIRE(field.valid()); - REQUIRE_FALSE(field2.valid()); // NOLINT clang-tidy: hicpp-invalid-access-moved + REQUIRE_FALSE(field2.valid()); // NOLINT(hicpp-invalid-access-moved, bugprone-use-after-move) field.add_element(cpp_type(17)); } @@ -288,7 +288,7 @@ TEST_CASE("write from different types of iterators: " PBF_TYPE_NAME) { REQUIRE(it_range.front() == 25); it_range.drop_front(); REQUIRE(it_range.empty()); REQUIRE(std::distance(it_range.begin(), it_range.end()) == 0); - REQUIRE(it_range.size() == 0); // NOLINT clang-tidy: readability-container-size-empty + REQUIRE(it_range.size() == 0); // NOLINT(readability-container-size-empty) REQUIRE_THROWS_AS(it_range.front(), const assert_error&); REQUIRE_THROWS_AS(it_range.drop_front(), const assert_error&); diff --git a/test/include/scalar_access.hpp b/test/include/scalar_access.hpp index 6f1e9df..4f49184 100644 --- a/test/include/scalar_access.hpp +++ b/test/include/scalar_access.hpp @@ -1,4 +1,4 @@ -// NOLINT clang-tidy: llvm-header-guard +// NOLINT(llvm-header-guard) #define PBF_TYPE_NAME PROTOZERO_TEST_STRING(PBF_TYPE) #define GET_TYPE PROTOZERO_TEST_CONCAT(get_, PBF_TYPE) diff --git a/test/t/bool/reader_test_cases.cpp b/test/t/bool/reader_test_cases.cpp index 9bf518a..56d6364 100644 --- a/test/t/bool/reader_test_cases.cpp +++ b/test/t/bool/reader_test_cases.cpp @@ -126,7 +126,7 @@ TEST_CASE("write bool field using moved pbf_builder") { protozero::pbf_builder<TestBoolean::Test> pw{std::move(pw2)}; REQUIRE(pw.valid()); - REQUIRE_FALSE(pw2.valid()); // NOLINT clang-tidy: hicpp-invalid-access-moved + REQUIRE_FALSE(pw2.valid()); // NOLINT(hicpp-invalid-access-moved, bugprone-use-after-move) SECTION("false") { pw.add_bool(TestBoolean::Test::required_bool_b, false); diff --git a/test/t/tags/reader_test_cases.cpp b/test/t/tags/reader_test_cases.cpp index f2626ed..3b6a5e7 100644 --- a/test/t/tags/reader_test_cases.cpp +++ b/test/t/tags/reader_test_cases.cpp @@ -11,19 +11,19 @@ inline void check_tag(const std::string& buffer, protozero::pbf_tag_type tag) { } TEST_CASE("read tag: 1") { - check_tag(load_data("tags/data-tag-1"), 1L); + check_tag(load_data("tags/data-tag-1"), 1ul); } TEST_CASE("read tag: 200") { - check_tag(load_data("tags/data-tag-200"), 200L); + check_tag(load_data("tags/data-tag-200"), 200ul); } TEST_CASE("read tag: 200000") { - check_tag(load_data("tags/data-tag-200000"), 200000L); + check_tag(load_data("tags/data-tag-200000"), 200000ul); } TEST_CASE("read tag: max") { - check_tag(load_data("tags/data-tag-max"), (1L << 29) - 1); + check_tag(load_data("tags/data-tag-max"), (1ul << 29u) - 1u); } TEST_CASE("write tags") { @@ -46,7 +46,7 @@ TEST_CASE("write tags") { } SECTION("tag max") { - pw.add_int32((1L << 29) - 1, 333L); + pw.add_int32(static_cast<int32_t>((1ul << 29u) - 1u), 333L); REQUIRE(buffer == load_data("tags/data-tag-max")); } } diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt new file mode 100644 index 0000000..8d9651c --- /dev/null +++ b/test/unit/CMakeLists.txt @@ -0,0 +1,23 @@ +#----------------------------------------------------------------------------- +# +# CMake config +# +# protozero unit tests +# +#----------------------------------------------------------------------------- + +set(UNIT_TESTS data_view + basic + endian + exceptions + varint + zigzag) + +string(REGEX REPLACE "([^;]+)" "test_\\1.cpp" _test_sources "${UNIT_TESTS}") + +add_executable(unit_tests main.cpp ${_test_sources}) + +add_test(NAME unit_tests COMMAND unit_tests) + + +#----------------------------------------------------------------------------- diff --git a/test/unit/main.cpp b/test/unit/main.cpp new file mode 100644 index 0000000..fda3581 --- /dev/null +++ b/test/unit/main.cpp @@ -0,0 +1,4 @@ + +#define CATCH_CONFIG_MAIN +#include <test.hpp> // IWYU pragma: keep + diff --git a/test/t/basic/reader_test_cases.cpp b/test/unit/test_basic.cpp similarity index 82% rename from test/t/basic/reader_test_cases.cpp rename to test/unit/test_basic.cpp index 6d0dd55..0ab4a0f 100644 --- a/test/t/basic/reader_test_cases.cpp +++ b/test/unit/test_basic.cpp @@ -31,7 +31,7 @@ TEST_CASE("check every possible value for single byte in buffer") { } TEST_CASE("next() should throw when illegal wire type is encountered") { - const char buffer = 1 << 3 | 7; + const char buffer = 1u << 3u | 7u; protozero::pbf_reader item{&buffer, 1}; REQUIRE_THROWS_AS(item.next(), const protozero::unknown_pbf_wire_type_exception&); @@ -41,19 +41,19 @@ TEST_CASE("next() should throw when illegal tag is encountered") { std::string data; SECTION("tag 0") { - protozero::write_varint(std::back_inserter(data), 0 << 3 | 1); + protozero::write_varint(std::back_inserter(data), 0u << 3u | 1u); } SECTION("tag 19000") { - protozero::write_varint(std::back_inserter(data), 19000 << 3 | 1); + protozero::write_varint(std::back_inserter(data), 19000u << 3u | 1u); } SECTION("tag 19001") { - protozero::write_varint(std::back_inserter(data), 19001 << 3 | 1); + protozero::write_varint(std::back_inserter(data), 19001u << 3u | 1u); } SECTION("tag 19999") { - protozero::write_varint(std::back_inserter(data), 19999 << 3 | 1); + protozero::write_varint(std::back_inserter(data), 19999u << 3u | 1u); } protozero::pbf_reader item{data}; @@ -64,19 +64,19 @@ TEST_CASE("next() works when a legal tag is encountered") { std::string data; SECTION("tag 1") { - protozero::write_varint(std::back_inserter(data), 1u << 3 | 1); + protozero::write_varint(std::back_inserter(data), 1u << 3u | 1u); } SECTION("tag 18999") { - protozero::write_varint(std::back_inserter(data), 18999u << 3 | 1); + protozero::write_varint(std::back_inserter(data), 18999u << 3u | 1u); } SECTION("tag 20000") { - protozero::write_varint(std::back_inserter(data), 20000u << 3 | 1); + protozero::write_varint(std::back_inserter(data), 20000u << 3u | 1u); } SECTION("tag 1^29 - 1") { - protozero::write_varint(std::back_inserter(data), ((1u << 29) - 1) << 3 | 1); + protozero::write_varint(std::back_inserter(data), ((1u << 29u) - 1u) << 3u | 1u); } protozero::pbf_reader item{data}; @@ -95,7 +95,7 @@ TEST_CASE("pbf_writer asserts on invalid tags") { REQUIRE_THROWS_AS(writer.add_int32(19001, 123), const assert_error&); REQUIRE_THROWS_AS(writer.add_int32(19999, 123), const assert_error&); writer.add_int32(20000, 123); - writer.add_int32((1 << 29) - 1, 123); - REQUIRE_THROWS_AS(writer.add_int32(1 << 29, 123), const assert_error&); + writer.add_int32((1u << 29u) - 1u, 123); + REQUIRE_THROWS_AS(writer.add_int32(1u << 29u, 123), const assert_error&); } diff --git a/test/t/data_view/reader_test_cases.cpp b/test/unit/test_data_view.cpp similarity index 98% rename from test/t/data_view/reader_test_cases.cpp rename to test/unit/test_data_view.cpp index b4db378..da7dfe8 100644 --- a/test/t/data_view/reader_test_cases.cpp +++ b/test/unit/test_data_view.cpp @@ -8,7 +8,7 @@ TEST_CASE("default constructed data_view") { const protozero::data_view view{}; REQUIRE(view.data() == nullptr); - REQUIRE(view.size() == 0); // NOLINT clang-tidy: readability-container-size-empty + REQUIRE(view.size() == 0); // NOLINT(readability-container-size-empty) REQUIRE(view.empty()); } diff --git a/test/t/endian/reader_test_cases.cpp b/test/unit/test_endian.cpp similarity index 100% rename from test/t/endian/reader_test_cases.cpp rename to test/unit/test_endian.cpp diff --git a/test/t/exceptions/reader_test_cases.cpp b/test/unit/test_exceptions.cpp similarity index 100% rename from test/t/exceptions/reader_test_cases.cpp rename to test/unit/test_exceptions.cpp diff --git a/test/t/varint/reader_test_cases.cpp b/test/unit/test_varint.cpp similarity index 86% rename from test/t/varint/reader_test_cases.cpp rename to test/unit/test_varint.cpp index 68111be..4a5b72c 100644 --- a/test/t/varint/reader_test_cases.cpp +++ b/test/unit/test_varint.cpp @@ -26,12 +26,12 @@ TEST_CASE("varint") { } SECTION("encode/decode uint32") { - pw.add_uint32(1, 17U); + pw.add_uint32(1, 17u); protozero::pbf_reader item{buffer}; REQUIRE(item.next()); SECTION("get") { - REQUIRE(17U == item.get_uint32()); + REQUIRE(17u == item.get_uint32()); } SECTION("skip") { @@ -42,12 +42,12 @@ TEST_CASE("varint") { } SECTION("encode/decode uint64") { - pw.add_uint64(1, (1ULL << 40)); + pw.add_uint64(1, (1ull << 40u)); protozero::pbf_reader item{buffer}; REQUIRE(item.next()); SECTION("get") { - REQUIRE((1ULL << 40) == item.get_uint64()); + REQUIRE((1ull << 40u) == item.get_uint64()); } SECTION("skip") { @@ -58,7 +58,7 @@ TEST_CASE("varint") { } SECTION("short buffer while parsing varint") { - pw.add_uint64(1, (1ULL << 40)); + pw.add_uint64(1, (1ull << 40u)); buffer.resize(buffer.size() - 1); // "remove" last byte from buffer protozero::pbf_reader item{buffer}; REQUIRE(item.next()); @@ -73,7 +73,7 @@ TEST_CASE("varint") { } SECTION("data corruption in buffer while parsing varint)") { - pw.add_uint64(1, (1ULL << 20)); + pw.add_uint64(1, (1ull << 20u)); buffer[buffer.size() - 1] += 0x80; // pretend the varint goes on protozero::pbf_reader item{buffer}; REQUIRE(item.next()); @@ -107,9 +107,9 @@ TEST_CASE("10-byte varint") { std::string buffer; protozero::pbf_writer pw{buffer}; pw.add_uint64(1, 1); - buffer.back() = static_cast<char>(0xff); + buffer.back() = static_cast<char>(0xffu); for (int i = 0; i < 9; ++i) { - buffer.push_back(static_cast<char>(0xff)); + buffer.push_back(static_cast<char>(0xffu)); } buffer.push_back(0x02); @@ -151,8 +151,8 @@ TEST_CASE("lots of varints back and forth") { buffer.clear(); } - for (int i = 0; i < 63; ++i) { - int64_t n = 1ll << i; + for (uint32_t i = 0; i < 63; ++i) { + const auto n = static_cast<int64_t>(1ull << i); protozero::pbf_writer pw{buffer}; pw.add_int64(1, n); protozero::pbf_reader item{buffer}; @@ -162,8 +162,8 @@ TEST_CASE("lots of varints back and forth") { buffer.clear(); } - for (int i = 0; i < 63; ++i) { - int64_t n = - (1ll << i); + for (uint32_t i = 0; i < 63; ++i) { + const int64_t n = - static_cast<int64_t>(1ull << i); protozero::pbf_writer pw{buffer}; pw.add_int64(1, n); protozero::pbf_reader item{buffer}; @@ -173,13 +173,13 @@ TEST_CASE("lots of varints back and forth") { buffer.clear(); } - for (int i = 0; i < 64; ++i) { - uint64_t n = 1ull << i; + for (uint32_t i = 0; i < 64; ++i) { + const uint64_t n = 1ull << i; protozero::pbf_writer pw{buffer}; - pw.add_int64(1, n); + pw.add_uint64(1, n); protozero::pbf_reader item{buffer}; REQUIRE(item.next()); - REQUIRE(n == item.get_int64()); + REQUIRE(n == item.get_uint64()); REQUIRE_FALSE(item.next()); buffer.clear(); } diff --git a/test/t/zigzag/reader_test_cases.cpp b/test/unit/test_zigzag.cpp similarity index 100% rename from test/t/zigzag/reader_test_cases.cpp rename to test/unit/test_zigzag.cpp diff --git a/tools/pbf-decoder.cpp b/tools/pbf-decoder.cpp index 3f6700e..5a8d908 100644 --- a/tools/pbf-decoder.cpp +++ b/tools/pbf-decoder.cpp @@ -30,6 +30,7 @@ Call with --help/-h to see more options. #include <iostream> #include <limits> #include <sstream> +#include <stdexcept> #include <string> std::string decode(const char* data, std::size_t len, const std::string& indent); @@ -224,11 +225,11 @@ int main(int argc, char* argv[]) { print_help(); return 0; case 'l': - length = std::atoll(optarg); // NOLINT clang-tidy: cert-err34-c + length = std::atoll(optarg); // NOLINT(cert-err34-c) // good enough for a limited-use tool break; case 'o': - offset = std::atoll(optarg); // NOLINT clang-tidy: cert-err34-c + offset = std::atoll(optarg); // NOLINT(cert-err34-c) // good enough for a limited-use tool break; default: @@ -246,14 +247,22 @@ int main(int argc, char* argv[]) { const std::string filename{argv[optind]}; try { - const std::string buffer{filename == "-" ? read_from_stdin() : - read_from_file(argv[optind])}; + std::string buffer{filename == "-" ? read_from_stdin() : + read_from_file(argv[optind])}; - if (length > buffer.size() - offset) { - length = buffer.size() - offset; + if (offset > buffer.size()) { + throw std::runtime_error{"offset is larger than file size"}; } - std::cout << decode(buffer.data() + offset, length, ""); + if (offset > 0) { + buffer.erase(0, offset); + } + + if (length < buffer.size()) { + buffer.resize(length); + } + + std::cout << decode(buffer.data(), buffer.size(), ""); } catch (const std::exception& ex) { std::cerr << ex.what() << '\n'; return 1; -- Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-grass/protozero.git _______________________________________________ Pkg-grass-devel mailing list Pkg-grass-devel@lists.alioth.debian.org http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-grass-devel