diff --git a/.clang-tidy b/.clang-tidy index a7e001a..19c74b0 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -12,6 +12,7 @@ Checks: > -google-readability-todo, -google-readability-namespace-comments, -google-runtime-references, + -misc-include-cleaner, -misc-non-private-member-variables-in-classes, -misc-unused-parameters, -misc-no-recursion, @@ -21,6 +22,7 @@ Checks: > -modernize-avoid-c-arrays, -modernize-use-auto, -modernize-use-nodiscard, + -performance-enum-size, -performance-move-const-arg, -readability-braces-around-statements, -readability-identifier-length, diff --git a/.cppcheck-suppress b/.cppcheck-suppress index 870742a..a509977 100644 --- a/.cppcheck-suppress +++ b/.cppcheck-suppress @@ -4,4 +4,5 @@ unusedFunction useStlAlgorithm noExplicitConstructor unusedStructMember +normalCheckLevelMaxBranches *:*_deps/* diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml index d95e5d9..966347f 100644 --- a/.github/workflows/clang-tidy.yml +++ b/.github/workflows/clang-tidy.yml @@ -12,7 +12,7 @@ jobs: clang_tidy: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 0 submodules: true diff --git a/.github/workflows/default.yml b/.github/workflows/default.yml index bbc4c57..80ee53f 100644 --- a/.github/workflows/default.yml +++ b/.github/workflows/default.yml @@ -12,7 +12,7 @@ jobs: build_and_lint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: fetch-depth: 0 submodules: true @@ -23,7 +23,7 @@ jobs: - name: Install deps run: sudo apt install cppcheck ccache - - uses: actions/cache@v3.3.1 + - uses: actions/cache@v4 id: ccache-persistence with: path: ~/.ccache diff --git a/cmake/clang_format.cmake b/cmake/clang_format.cmake index ca0fa8c..80f4fe1 100644 --- a/cmake/clang_format.cmake +++ b/cmake/clang_format.cmake @@ -21,7 +21,7 @@ add_custom_target( clang-tidy COMMAND ${PROJECT_SOURCE_DIR}/tools/run-clang-tidy.py -p . -use-color -header-filter='.*' -quiet - -extra-arg=-std=c++17 + -extra-arg=-std=c++17 -extra-arg=-Wno-unknown-warning-option -ignore ${PROJECT_SOURCE_DIR}/.clang-tidy-ignore WORKING_DIRECTORY ${CMAKE_BINARY_DIR} USES_TERMINAL diff --git a/examples/ulog_writer.cpp b/examples/ulog_writer.cpp index 9992dbe..57a3f89 100644 --- a/examples/ulog_writer.cpp +++ b/examples/ulog_writer.cpp @@ -10,12 +10,14 @@ using namespace std::chrono_literals; -static uint64_t currentTimeUs() +namespace { +uint64_t currentTimeUs() { return std::chrono::duration_cast( std::chrono::steady_clock::now().time_since_epoch()) .count(); } +} // namespace struct MyData { uint64_t timestamp; diff --git a/test/ulog_parsing_test.cpp b/test/ulog_parsing_test.cpp index ac014d7..b3fbe18 100644 --- a/test/ulog_parsing_test.cpp +++ b/test/ulog_parsing_test.cpp @@ -108,8 +108,9 @@ TEST_CASE("ULog parsing - basic test: write then read") } } -static void readFileWriteTest(const std::filesystem::path& path, - const std::function& next_chunk_size) +namespace { +void readFileWriteTest(const std::filesystem::path& path, + const std::function& next_chunk_size) { FILE* file = fopen(path.c_str(), "rb"); CHECK(file); @@ -144,6 +145,7 @@ static void readFileWriteTest(const std::filesystem::path& path, CHECK_EQ(input_data.size(), written_data.size()); CHECK_EQ(input_data, written_data); } +} // namespace TEST_CASE("ULog parsing - read sample files then write") { @@ -343,7 +345,7 @@ TEST_CASE("ULog parsing - simple writer") std::vector written_data_messages; for (int i = 0; i < 100; ++i) { MyData data{}; - data.timestamp = i * 1000; + data.timestamp = static_cast(i) * 1000; data.cpuload = cpuload; data.counter = i; writer.writeData(my_data_msg_id, data); diff --git a/ulog_cpp/data_container.cpp b/ulog_cpp/data_container.cpp index 1fd7bb2..39a10d2 100644 --- a/ulog_cpp/data_container.cpp +++ b/ulog_cpp/data_container.cpp @@ -43,17 +43,17 @@ void DataContainer::headerComplete() // try to resolve all fields for params for (auto& it : _default_parameters) { - auto& parameter = it.second; - parameter.field().resolveDefinition(_message_formats, 0); + auto& parameter_value = it.second; + parameter_value.field().resolveDefinition(_message_formats, 0); } for (auto& it : _initial_parameters) { - auto& parameter = it.second; - parameter.field().resolveDefinition(_message_formats, 0); + auto& parameter_value = it.second; + parameter_value.field().resolveDefinition(_message_formats, 0); } - for (auto& parameter : _changed_parameters) { - parameter.field().resolveDefinition(_message_formats, 0); + for (auto& parameter_value : _changed_parameters) { + parameter_value.field().resolveDefinition(_message_formats, 0); } _header_complete = true; @@ -100,10 +100,10 @@ void DataContainer::parameter(const Parameter& parameter_arg) return; } if (_header_complete) { - auto parameter = parameter_arg; + auto parameter_value = parameter_arg; // if header is complete, we can resolve definition here - parameter.field().resolveDefinition(_message_formats, 0); - _changed_parameters.push_back(parameter); + parameter_value.field().resolveDefinition(_message_formats, 0); + _changed_parameters.push_back(parameter_value); } else { _initial_parameters.insert({parameter_arg.field().name(), parameter_arg}); } diff --git a/ulog_cpp/messages.cpp b/ulog_cpp/messages.cpp index c5c82fd..91aedef 100644 --- a/ulog_cpp/messages.cpp +++ b/ulog_cpp/messages.cpp @@ -196,7 +196,7 @@ Value::NativeTypeVariant Value::asNativeTypeVariant() const if (_field_ref.arrayLength() == -1 || _array_index >= 0) { // decode as a single value. Either it is a single value, // or the user has explicitly selected an array element - int array_offset = _array_index >= 0 ? _array_index : 0; + const int array_offset = _array_index >= 0 ? _array_index : 0; switch (_field_ref.type().type) { case Field::BasicType::INT8: return deserialize(_backing_ref_begin, _backing_ref_end, @@ -298,8 +298,8 @@ Value Value::operator[](const Field& field) const if (!_field_ref.definitionResolved()) { throw AccessException("Cannot access field of unresolved type"); } - int submessage_offset = _field_ref.offsetInMessage() + - ((_array_index >= 0) ? _field_ref.type().size * _array_index : 0); + const int submessage_offset = _field_ref.offsetInMessage() + + ((_array_index >= 0) ? _field_ref.type().size * _array_index : 0); return Value(field, _backing_ref_begin + submessage_offset, _backing_ref_end); } diff --git a/ulog_cpp/messages.hpp b/ulog_cpp/messages.hpp index 2f5769a..63f1a12 100644 --- a/ulog_cpp/messages.hpp +++ b/ulog_cpp/messages.hpp @@ -172,25 +172,25 @@ class Field { * @brief Get the type attributes of the field * @return The type attributes */ - inline const TypeAttributes& type() const { return _type; } + const TypeAttributes& type() const { return _type; } /** * @brief Get the array length of the field * @return The array length, -1 if not an array */ - inline int arrayLength() const { return _array_length; } + int arrayLength() const { return _array_length; } /** * @brief Get the offset of the field in the message. This is only valid if the field is resolved. * @return The offset in bytes, -1 if not resolved */ - inline int offsetInMessage() const { return _offset_in_message_bytes; } + int offsetInMessage() const { return _offset_in_message_bytes; } /** * @brief Get the name of the field * @return The name */ - inline const std::string& name() const { return _name; } + const std::string& name() const { return _name; } /** * @brief Get the size of the field in bytes. This is only valid if the field is resolved. @@ -203,11 +203,11 @@ class Field { * is defined and the type is not nested or the nested message is resolved. * @return True if the field is resolved */ - inline bool definitionResolved() const + bool definitionResolved() const { return _offset_in_message_bytes >= 0 && (_type.type != BasicType::NESTED || _type.nested_message != nullptr); - }; + } /** * @brief Attempt to resolve the definition of the field. @@ -353,8 +353,8 @@ class Value { // this is natively a vector if constexpr (is_vector::value) { // return type is also vector - if constexpr (std::is_same::value) { + if constexpr (std::is_same_v) { // return type is same as native type res = arg; } else { @@ -428,13 +428,22 @@ class Value { int array_offset) { T v; - int total_offset = offset + array_offset * sizeof(T); + const int total_offset = offset + (array_offset * sizeof(T)); if (backing_start > backing_end || - backing_end - backing_start - total_offset < static_cast(sizeof(v))) { + backing_end - backing_start < static_cast(sizeof(v)) + total_offset) { throw AccessException("Unexpected data type size"); } + // GCC 14.2 raises an error here when building with -fsanitize=address +#ifdef __GNUC__ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Warray-bounds" +#pragma GCC diagnostic ignored "-Wstringop-overflow" +#endif std::copy(backing_start + total_offset, backing_start + total_offset + sizeof(v), reinterpret_cast(&v)); +#ifdef __GNUC__ +#pragma GCC diagnostic pop +#endif return v; } @@ -594,6 +603,7 @@ class MessageFormat { std::vector fieldNames() const { std::vector names; + names.reserve(_fields_ordered.size()); for (const auto& field_it : _fields_ordered) { names.push_back(field_it->name()); } diff --git a/ulog_cpp/reader.cpp b/ulog_cpp/reader.cpp index c8ac8fd..0470e0a 100644 --- a/ulog_cpp/reader.cpp +++ b/ulog_cpp/reader.cpp @@ -9,7 +9,7 @@ #include "raw_messages.hpp" -#if 0 +#if 0 // NOLINT (readability-avoid-unconditional-preprocessor-if) #define DBG_PRINTF(...) printf(__VA_ARGS__) #else #define DBG_PRINTF(...) diff --git a/ulog_cpp/simple_writer.hpp b/ulog_cpp/simple_writer.hpp index 3ddfe7c..92d9995 100644 --- a/ulog_cpp/simple_writer.hpp +++ b/ulog_cpp/simple_writer.hpp @@ -6,7 +6,15 @@ #include #include +// GCC 14.2 raises an error in when building with -fsanitize=address +#ifdef __GNUC__ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#endif #include +#ifdef __GNUC__ +#pragma GCC diagnostic pop +#endif #include #include