From 05af6806bfca41ac545edc55332b08a0835b3093 Mon Sep 17 00:00:00 2001 From: DownerCase <119755054+DownerCase@users.noreply.github.com> Date: Tue, 3 Dec 2024 10:42:15 +0000 Subject: [PATCH] feat: Enforce compiler warnings (#16) --- .github/workflows/ci.yml | 2 +- ecal/core.cpp | 6 +-- ecal/monitoring/monitoring.cpp | 8 ++-- ecal/monitoring/monitoring.h | 6 +-- ecal/publisher/publisher.cpp | 2 +- ecal/registration/registration.cpp | 24 +--------- ecal/registration/registration.go | 6 +-- ecal/subscriber/subscriber.cpp | 12 ++--- ecal/types/types.cpp | 17 ++++--- ecal/types/types.h | 5 +-- internal/handle_map.hpp | 2 +- project/gcc.cmake | 72 ++++++++++++++++++++++++++++++ 12 files changed, 102 insertions(+), 60 deletions(-) create mode 100644 project/gcc.cmake diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b04e08f..c02eddc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,7 +38,7 @@ jobs: - uses: actions/checkout@v4 - name: Configure - run: cmake -S . -B build -G Ninja -Werror=dev -DCMAKE_COMPILE_WARNING_AS_ERROR=ON + run: cmake -S . -B build -G Ninja -Werror=dev -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DCMAKE_TOOLCHAIN_FILE=project/gcc.cmake - name: Build run: cmake --build ./build diff --git a/ecal/core.cpp b/ecal/core.cpp index 1b143d9..5a8882b 100644 --- a/ecal/core.cpp +++ b/ecal/core.cpp @@ -18,11 +18,7 @@ version GetVersion() { return version_; } -int Initialize( - struct config *config, - const char *unit_name, - unsigned int components -) { +int Initialize(config *config, const char *unit_name, unsigned int components) { auto cfg = convertConfig(*config); // TODO: Initialize should take by const ref return eCAL::Initialize(cfg, unit_name, components); diff --git a/ecal/monitoring/monitoring.cpp b/ecal/monitoring/monitoring.cpp index a6d2dde..979e34a 100644 --- a/ecal/monitoring/monitoring.cpp +++ b/ecal/monitoring/monitoring.cpp @@ -24,18 +24,18 @@ namespace { // Makes uses of copy elision CTopicMon toCType(const eCAL::Monitoring::STopicMon &topic) { return { - topic.rclock, topic.uname.c_str(), topic.tid.c_str(), topic.tname.c_str(), topic.direction.c_str(), toCDataType(topic.tdatatype), + topic.dclock, + topic.dfreq, + topic.rclock, topic.tsize, topic.connections_loc, topic.connections_ext, - topic.message_drops, - topic.dclock, - topic.dfreq + topic.message_drops }; } diff --git a/ecal/monitoring/monitoring.h b/ecal/monitoring/monitoring.h index ac9db58..5259800 100644 --- a/ecal/monitoring/monitoring.h +++ b/ecal/monitoring/monitoring.h @@ -24,18 +24,18 @@ enum eCAL_Monitoring_eEntity { }; struct CTopicMon { - int32_t registration_clock; const char *unit_name; const char *topic_id; const char *topic_name; const char *direction; struct CDatatype datatype; + int64_t data_clock; + int64_t data_freq; + int32_t registration_clock; int32_t topic_size; int32_t connections_local; int32_t connections_external; int32_t message_drops; - int64_t data_clock; - int64_t data_freq; }; struct CProcessMon { diff --git a/ecal/publisher/publisher.cpp b/ecal/publisher/publisher.cpp index a179282..7a00c68 100644 --- a/ecal/publisher/publisher.cpp +++ b/ecal/publisher/publisher.cpp @@ -5,7 +5,7 @@ #include "internal/handle_map.hpp" namespace { -handle_map publishers; +handle_map publishers{}; } // namespace bool NewPublisher(uintptr_t handle) { diff --git a/ecal/registration/registration.cpp b/ecal/registration/registration.cpp index 5d9bae9..50154c1 100644 --- a/ecal/registration/registration.cpp +++ b/ecal/registration/registration.cpp @@ -6,31 +6,9 @@ #include extern "C" { -extern void -goTopicEventCallback(uintptr_t handle, struct CTopicId id, uint8_t event); +extern void goTopicEventCallback(uintptr_t handle, CTopicId id, uint8_t event); } -namespace { -int safe_len(size_t str_len) { - if (str_len > INT_MAX) { - return INT_MAX; - } - return str_len; -} - -struct CQualityInfo -toCQualityInfo(const eCAL::Registration::SQualityTopicInfo &quality) { - return { - {quality.info.name.c_str(), - quality.info.encoding.c_str(), - quality.info.descriptor.c_str(), - safe_len(quality.info.descriptor.size())}, - static_cast(quality.quality), - }; -} - -} // namespace - size_t AddPublisherEventCallback(uintptr_t handle) { const auto callback_adapter = [handle]( const eCAL::Registration::STopicId &id, diff --git a/ecal/registration/registration.go b/ecal/registration/registration.go index cfaec93..6317ee7 100644 --- a/ecal/registration/registration.go +++ b/ecal/registration/registration.go @@ -76,11 +76,11 @@ func toQualityTopicInfo(quality *C.struct_CQualityInfo) QualityTopicInfo { func toTopicId(id *C.struct_CTopicId) TopicId { return TopicId{ Topic_id: EntityId{ - Entity_id: C.GoStringN(id.topic_id.entity_id, id.topic_id.entity_id_len), + Entity_id: C.GoString(id.topic_id.entity_id), Process_id: int32(id.topic_id.process_id), - Host_name: C.GoStringN(id.topic_id.host_name, id.topic_id.host_name_len), + Host_name: C.GoString(id.topic_id.host_name), }, - Topic_name: C.GoStringN(id.topic_name, id.topic_name_len), + Topic_name: C.GoString(id.topic_name), } } diff --git a/ecal/subscriber/subscriber.cpp b/ecal/subscriber/subscriber.cpp index 55179e2..c70b8b4 100644 --- a/ecal/subscriber/subscriber.cpp +++ b/ecal/subscriber/subscriber.cpp @@ -13,8 +13,8 @@ handle_map subscribers; void receive_callback( const uintptr_t handle, - const eCAL::Registration::STopicId &topic, - const eCAL::SDataTypeInformation &datatype, + const eCAL::Registration::STopicId & /*topic*/, + const eCAL::SDataTypeInformation & /*datatype*/, const eCAL::SReceiveCallbackData &data ) { goReceiveCallback(handle, data.buf, data.size); @@ -51,11 +51,11 @@ bool SubscriberCreate( std::string(datatype_descriptor, datatype_descriptor_len)} ); const auto bound_callback = [handle]( - const eCAL::Registration::STopicId &topic, - const eCAL::SDataTypeInformation &datatype, - const eCAL::SReceiveCallbackData &data + const eCAL::Registration::STopicId &_topic, + const eCAL::SDataTypeInformation &_datatype, + const eCAL::SReceiveCallbackData &_data ) { - receive_callback(handle, topic, datatype, data); + receive_callback(handle, _topic, _datatype, _data); }; subscriber->AddReceiveCallback(bound_callback); return created; diff --git a/ecal/types/types.cpp b/ecal/types/types.cpp index 9aaf8a8..98c8d30 100644 --- a/ecal/types/types.cpp +++ b/ecal/types/types.cpp @@ -2,10 +2,10 @@ namespace { int safe_len(size_t str_len) { - if (str_len > INT_MAX) { + if (str_len > size_t{INT_MAX}) { return INT_MAX; } - return str_len; + return static_cast(str_len); } } // namespace @@ -20,12 +20,11 @@ CDatatype toCDataType(const eCAL::SDataTypeInformation &datatype) { CTopicId toCTopicId(const eCAL::Registration::STopicId &id) { return { - {id.topic_id.entity_id.data(), - safe_len(id.topic_id.entity_id.size()), - id.topic_id.process_id, - id.topic_id.host_name.data(), - safe_len(id.topic_id.host_name.size())}, - id.topic_name.data(), - safe_len(id.topic_name.size()) + { + id.topic_id.entity_id.data(), + id.topic_id.host_name.data(), + id.topic_id.process_id, + }, + id.topic_name.data() }; } diff --git a/ecal/types/types.h b/ecal/types/types.h index 63e7a2b..c128aab 100644 --- a/ecal/types/types.h +++ b/ecal/types/types.h @@ -9,16 +9,13 @@ extern "C" { struct CEntityId { const char *entity_id; - int entity_id_len; - int32_t process_id; const char *host_name; - int host_name_len; + int32_t process_id; }; struct CTopicId { struct CEntityId topic_id; const char *topic_name; - int topic_name_len; }; struct CDatatype { diff --git a/internal/handle_map.hpp b/internal/handle_map.hpp index 5a34c11..dc1822d 100644 --- a/internal/handle_map.hpp +++ b/internal/handle_map.hpp @@ -6,7 +6,7 @@ template class handle_map { private: - std::unordered_map handles; + std::unordered_map handles{}; public: using iterator = typename decltype(handles)::iterator; diff --git a/project/gcc.cmake b/project/gcc.cmake new file mode 100644 index 0000000..1bfc89f --- /dev/null +++ b/project/gcc.cmake @@ -0,0 +1,72 @@ +set(CMAKE_C_COMPILER gcc) +set(CMAKE_CXX_COMPILER g++) + +set(gcc_warnings +-Wpedantic # Forbid extensions +-pedantic-errors # Warnings from Wpedantic (and default warnings) are errors +-Wall # Enable warnings +-Wextra # Enable more warnings +-Wdouble-promotion # Accidental float -> double promotion +-Wformat=2 # Check arguments to printf etc. +-Wformat-overflow=2 # Check for potential sprintf etc. destination overflows +-Wformat-signedness # Checking format string signedness +-Wformat-truncation=2 # Check for output truncation +-Wnull-dereference # Dereference of null +-Wnrvo # Warn when NRVO fails +-Wimplicit-fallthrough=5 # Fallthrough can only be allowed by [[fallthrough]] +-Wmissing-include-dirs # Include directories that don't exist +-Wswitch-default # Require default for switch +-Wswitch-enum # Require all named cases for switch on an enum +-Wunused +-Wuse-after-free=3 +-Wuseless-cast # Casting to your own type +-Wmaybe-uninitialized # Potential unitialized uses +-Wstrict-overflow=5 +-Wstringop-overflow=4 +-Warith-conversion # WARN: Noisy warning +-Warray-bounds=2 +-Wbidi-chars=any # Misleading bidirectional UTF-8 control characters +-Wduplicated-branches # Duplicated conditional branches +-Wduplicated-cond # Duplicated conditions +-Wtrampolines # No trampolines +-Wfloat-equal # Comparing floats is hard +-Wshadow # Don't shadow varibles +-Wunsafe-loop-optimizations +-Wundef +-Wcast-qual +-Wcast-align=strict +-Wconversion # Catch implicit conversions +-Wdate-time # Date/time prevents reproducible builds +-Wsign-conversion # Implicit conversions that change the sign +-Wlogical-op # Suspicious use of logical operators +-Wmissing-declarations +-Wpadded +-Wno-error=padded # WARN: Wpadded is extremely noisy +-Wredundant-decls +-Winline +-Wdisabled-optimization +-Wstack-protector + +-Wctad-maybe-unsupported +-Wctor-dtor-privacy +-Wnoexcept +-Wnon-virtual-dtor # Virtual classes need virtual destructors +-Wredundant-tags +-Weffc++ +-Wstrict-null-sentinel +-Wold-style-cast +-Woverloaded-virtual +-Wsign-promo +-Wcatch-value=3 +-Wextra-semi + +-Wsuggest-final-types +-Wsuggest-final-methods +-Wsuggest-override +) + +string(REPLACE ";" " " gcc_warnings "${gcc_warnings}") + +set(CMAKE_CXX_FLAGS_INIT "${gcc_warnings}") + +set(CMAKE_COMPILE_WARNING_AS_ERROR ON)