From 9d89db29f3bf6c826293350f8f1a8559ec906b24 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Mon, 16 Jan 2023 22:00:38 -0500 Subject: [PATCH] clang-tidy: add a subset of cppcoreguidelines and other style checks MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Enforce some of the coding style rules using clang-tidy. The changes to the code concern cppcoreguidelines-special-member-functions. Change-Id: I47ec220505a625814b18b42ae9a070b8bf413337 Signed-off-by: Jérémie Galarneau --- .clang-tidy | 13 +++++ src/bin/lttng-sessiond/clock-class.hpp | 19 +++++--- src/bin/lttng-sessiond/event-class.hpp | 7 ++- src/bin/lttng-sessiond/field.hpp | 48 +++++++++++++++---- src/bin/lttng-sessiond/stream-class.hpp | 4 ++ src/bin/lttng-sessiond/trace-class.hpp | 15 +++++- src/bin/lttng-sessiond/ust-field-convert.cpp | 2 +- .../lttng-sessiond/ust-registry-channel.hpp | 4 ++ src/bin/lttng-sessiond/ust-registry-event.hpp | 4 ++ .../lttng-sessiond/ust-registry-session.hpp | 5 ++ src/bin/lttng-sessiond/ust-registry.hpp | 4 ++ src/common/file-descriptor.hpp | 2 + src/common/pthread-lock.hpp | 19 ++++++-- src/common/scope-exit.hpp | 2 + src/common/urcu.hpp | 15 ++++-- 15 files changed, 134 insertions(+), 29 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index ff404e436..75f559c5d 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -56,15 +56,28 @@ Checks: '-*, bugprone-virtual-near-miss, bugprone-unused-raii, bugprone-use-after-move, + cppcoreguidelines-pro-type-const-cast, + cppcoreguidelines-slicing, + cppcoreguidelines-special-member-functions, + cppcoreguidelines-virtual-class-destructor, google-build-explicit-make-pair, google-explicit-constructor, + misc-const-correctness, + misc-misleading-identifier, + misc-non-copyable-objects, + misc-throw-by-value-catch-by-reference, + misc-unused-parameters, + misc-unused-using-decls, modernize-avoid-bind, modernize-concat-nested-namespaces, modernize-loop-convert, modernize-make-shared, modernize-make-unique, + modernize-pass-by-value, modernize-redundant-void-arg, + modernize-replace-auto-ptr, modernize-replace-random-shuffle, + modernize-replace-auto-ptr, modernize-shrink-to-fit, modernize-use-bool-literals, modernize-use-default-member-init, diff --git a/src/bin/lttng-sessiond/clock-class.hpp b/src/bin/lttng-sessiond/clock-class.hpp index 58adc8eac..fd130fc18 100644 --- a/src/bin/lttng-sessiond/clock-class.hpp +++ b/src/bin/lttng-sessiond/clock-class.hpp @@ -32,21 +32,26 @@ public: using scycles_t = int64_t; using cuptr = std::unique_ptr; + virtual ~clock_class() = default; + clock_class(const clock_class&) = delete; + clock_class(clock_class&&) = delete; + clock_class& operator=(clock_class&&) = delete; + clock_class& operator=(const clock_class&) = delete; + + virtual void accept(trace_class_visitor& visitor) const; + const std::string name; const std::string description; const nonstd::optional uuid; const scycles_t offset; const cycles_t frequency; - virtual void accept(trace_class_visitor& visitor) const; - virtual ~clock_class() = default; - protected: clock_class(std::string name, - std::string description, - nonstd::optional uuid, - scycles_t offset, - cycles_t frequency); + std::string description, + nonstd::optional uuid, + scycles_t offset, + cycles_t frequency); }; } /* namespace trace */ diff --git a/src/bin/lttng-sessiond/event-class.hpp b/src/bin/lttng-sessiond/event-class.hpp index d42d764aa..2a5e63e62 100644 --- a/src/bin/lttng-sessiond/event-class.hpp +++ b/src/bin/lttng-sessiond/event-class.hpp @@ -23,9 +23,14 @@ class trace_class_visitor; class event_class { public: - virtual void accept(trace_class_visitor& visitor) const; + event_class(const event_class&) = delete; + event_class(event_class&&) = delete; + event_class& operator=(event_class&&) = delete; + event_class& operator=(const event_class&) = delete; virtual ~event_class() = default; + virtual void accept(trace_class_visitor& visitor) const; + const unsigned int id; const unsigned int stream_class_id; const int log_level; diff --git a/src/bin/lttng-sessiond/field.hpp b/src/bin/lttng-sessiond/field.hpp index 207592b3f..dc418b661 100644 --- a/src/bin/lttng-sessiond/field.hpp +++ b/src/bin/lttng-sessiond/field.hpp @@ -65,7 +65,12 @@ public: bool operator==(const type& other) const noexcept; bool operator!=(const type& other) const noexcept; + virtual ~type(); + type(const type&) = delete; + type(type&&) = delete; + type& operator=(type&&) = delete; + type& operator=(const type&) = delete; /* Obtain an independent copy of `type`. */ virtual type::cuptr copy() const = 0; @@ -177,13 +182,20 @@ private: }; class enumeration_type : public integer_type { +public: + ~enumeration_type() override = default; + enumeration_type(const enumeration_type&) = delete; + enumeration_type(enumeration_type&&) = delete; + enumeration_type& operator=(enumeration_type&&) = delete; + enumeration_type& operator=(const enumeration_type&) = delete; + protected: enumeration_type(unsigned int alignment, - enum byte_order byte_order, - unsigned int size, - enum signedness signedness, - enum base base, - integer_type::roles roles = {}); + enum byte_order byte_order, + unsigned int size, + enum signedness signedness, + enum base base, + integer_type::roles roles = {}); void accept(type_visitor& visitor) const override = 0; }; @@ -214,20 +226,26 @@ class enumeration_mapping { public: using range_t = enumeration_mapping_range; - enumeration_mapping(const enumeration_mapping& other) = default; - enumeration_mapping(const enumeration_mapping&& other) noexcept : - name{ std::move(other.name) }, range{ other.range } + enumeration_mapping(std::string in_name, MappingIntegerType value) : + name{ std::move(in_name) }, range{ value, value } { } - enumeration_mapping(std::string in_name, MappingIntegerType value) : name{std::move(in_name)}, range{value, value} + enumeration_mapping(std::string in_name, range_t in_range) : + name{ std::move(in_name) }, range{ in_range } { } - enumeration_mapping(std::string in_name, range_t in_range) : name{std::move(in_name)}, range{in_range} + enumeration_mapping(const enumeration_mapping& other) = default; + enumeration_mapping(enumeration_mapping&& other) noexcept : + name{ std::move(other.name) }, range{ other.range } { } + enumeration_mapping& operator=(enumeration_mapping&&) = delete; + enumeration_mapping& operator=(const enumeration_mapping&) = delete; + ~enumeration_mapping() = default; + const std::string name; /* * Only one range per mapping is supported for the moment as @@ -519,6 +537,11 @@ private: class field_visitor { public: virtual ~field_visitor() = default; + field_visitor(field_visitor&&) = delete; + field_visitor(const field_visitor&) = delete; + field_visitor& operator=(const field_visitor&) = delete; + field_visitor& operator=(field_visitor&&) = delete; + virtual void visit(const field& field) = 0; protected: @@ -528,6 +551,11 @@ protected: class type_visitor { public: virtual ~type_visitor() = default; + type_visitor(type_visitor&&) = delete; + type_visitor(const type_visitor&) = delete; + type_visitor& operator=(const type_visitor&) = delete; + type_visitor& operator=(type_visitor&&) = delete; + virtual void visit(const integer_type& type) = 0; virtual void visit(const floating_point_type& type) = 0; virtual void visit(const signed_enumeration_type& type) = 0; diff --git a/src/bin/lttng-sessiond/stream-class.hpp b/src/bin/lttng-sessiond/stream-class.hpp index 754b723fa..d80ad2e0b 100644 --- a/src/bin/lttng-sessiond/stream-class.hpp +++ b/src/bin/lttng-sessiond/stream-class.hpp @@ -30,6 +30,10 @@ public: */ void accept(trace_class_visitor& visitor) const; virtual ~stream_class() = default; + stream_class(const stream_class&) = delete; + stream_class(stream_class&&) = delete; + stream_class& operator=(stream_class&&) = delete; + stream_class& operator=(const stream_class&) = delete; virtual const type* packet_context() const; virtual const type* event_header() const; diff --git a/src/bin/lttng-sessiond/trace-class.hpp b/src/bin/lttng-sessiond/trace-class.hpp index 72ba7559d..cbd411486 100644 --- a/src/bin/lttng-sessiond/trace-class.hpp +++ b/src/bin/lttng-sessiond/trace-class.hpp @@ -46,8 +46,11 @@ public: class trace_class { public: - virtual ~trace_class() = default; + trace_class(const trace_class&) = delete; + trace_class(trace_class&&) = delete; + trace_class& operator=(trace_class&&) = delete; + trace_class& operator=(const trace_class&) = delete; /* * Derived classes must implement the _accept_on_*() @@ -68,7 +71,12 @@ protected: class trace_class_environment_visitor { public: + trace_class_environment_visitor() = default; virtual ~trace_class_environment_visitor() = default; + trace_class_environment_visitor(const trace_class_environment_visitor&) = delete; + trace_class_environment_visitor(trace_class_environment_visitor&&) = delete; + trace_class_environment_visitor& operator=(trace_class_environment_visitor&&) = delete; + trace_class_environment_visitor& operator=(const trace_class_environment_visitor&) = delete; virtual void visit(const environment_field& field) = 0; virtual void visit(const environment_field& field) = 0; @@ -79,7 +87,12 @@ class trace_class_visitor { public: using cuptr = std::unique_ptr; + trace_class_visitor() = default; virtual ~trace_class_visitor() = default; + trace_class_visitor(const trace_class_visitor&) = delete; + trace_class_visitor(trace_class_visitor&&) = delete; + trace_class_visitor& operator=(trace_class_visitor&&) = delete; + trace_class_visitor& operator=(const trace_class_visitor&) = delete; virtual void visit(const lttng::sessiond::trace::trace_class& trace_class) = 0; virtual void visit(const lttng::sessiond::trace::clock_class& clock_class) = 0; diff --git a/src/bin/lttng-sessiond/ust-field-convert.cpp b/src/bin/lttng-sessiond/ust-field-convert.cpp index 62ac4825b..3b8eb2f51 100644 --- a/src/bin/lttng-sessiond/ust-field-convert.cpp +++ b/src/bin/lttng-sessiond/ust-field-convert.cpp @@ -631,7 +631,7 @@ create_typed_variant_choices(const lttng_ust_ctl_field *current, end, session_attributes, next_ust_ctl_field, - [&choices, typed_enumeration, &selector_field, quirks]( + [&choices, &typed_enumeration, &selector_field, quirks]( lst::field::uptr field) { /* * Find the enumeration mapping that matches the diff --git a/src/bin/lttng-sessiond/ust-registry-channel.hpp b/src/bin/lttng-sessiond/ust-registry-channel.hpp index cbe1ab5af..4e7229a69 100644 --- a/src/bin/lttng-sessiond/ust-registry-channel.hpp +++ b/src/bin/lttng-sessiond/ust-registry-channel.hpp @@ -47,6 +47,10 @@ public: const ust_app& app, uint32_t& out_event_id); ~registry_channel() override; + registry_channel(const registry_channel&) = delete; + registry_channel(registry_channel&&) = delete; + registry_channel& operator=(registry_channel&&) = delete; + registry_channel& operator=(const registry_channel&) = delete; const lttng::sessiond::trace::type *event_context() const final; void event_context(lttng::sessiond::trace::type::cuptr context); diff --git a/src/bin/lttng-sessiond/ust-registry-event.hpp b/src/bin/lttng-sessiond/ust-registry-event.hpp index b38b09553..fe325fced 100644 --- a/src/bin/lttng-sessiond/ust-registry-event.hpp +++ b/src/bin/lttng-sessiond/ust-registry-event.hpp @@ -37,6 +37,10 @@ public: int loglevel_value, nonstd::optional model_emf_uri); ~registry_event() override = default; + registry_event(const registry_event&) = delete; + registry_event(registry_event&&) = delete; + registry_event& operator=(registry_event&&) = delete; + registry_event& operator=(const registry_event&) = delete; /* Both objd are set by the tracer. */ const int session_objd; diff --git a/src/bin/lttng-sessiond/ust-registry-session.hpp b/src/bin/lttng-sessiond/ust-registry-session.hpp index 03ffc29f1..5a26d1aab 100644 --- a/src/bin/lttng-sessiond/ust-registry-session.hpp +++ b/src/bin/lttng-sessiond/ust-registry-session.hpp @@ -60,7 +60,12 @@ public: const char *enum_name, uint64_t enum_id) const; void regenerate_metadata(); + ~registry_session() override; + registry_session(const registry_session&) = delete; + registry_session(registry_session&&) = delete; + registry_session& operator=(registry_session&&) = delete; + registry_session& operator=(const registry_session&) = delete; const lttng::sessiond::trace::type *packet_header() const noexcept override; diff --git a/src/bin/lttng-sessiond/ust-registry.hpp b/src/bin/lttng-sessiond/ust-registry.hpp index 29872a20f..5d8bdb7ce 100644 --- a/src/bin/lttng-sessiond/ust-registry.hpp +++ b/src/bin/lttng-sessiond/ust-registry.hpp @@ -82,6 +82,10 @@ public: registry_enum(std::string name, enum lttng::sessiond::trace::integer_type::signedness signedness); virtual ~registry_enum() = default; + registry_enum(const registry_enum&) = delete; + registry_enum(registry_enum&&) = delete; + registry_enum& operator=(registry_enum&&) = delete; + registry_enum& operator=(const registry_enum&) = delete; std::string name; enum lttng::sessiond::trace::integer_type::signedness signedness; diff --git a/src/common/file-descriptor.hpp b/src/common/file-descriptor.hpp index 54c2e182a..2a2d21b1e 100644 --- a/src/common/file-descriptor.hpp +++ b/src/common/file-descriptor.hpp @@ -26,6 +26,8 @@ public: } file_descriptor(const file_descriptor&) = delete; + file_descriptor& operator=(const file_descriptor&) = delete; + file_descriptor& operator=(file_descriptor&&) = delete; file_descriptor(file_descriptor&& other) noexcept { LTTNG_ASSERT(_is_valid_fd(_raw_fd)); diff --git a/src/common/pthread-lock.hpp b/src/common/pthread-lock.hpp index 824b2f2a9..d77218717 100644 --- a/src/common/pthread-lock.hpp +++ b/src/common/pthread-lock.hpp @@ -26,13 +26,17 @@ namespace details { */ class mutex { public: - explicit mutex(pthread_mutex_t& mutex_p) : _mutex{mutex_p} + explicit mutex(pthread_mutex_t& mutex_p) : _mutex{ mutex_p } { } + ~mutex() = default; + /* "Not copyable" and "not moveable" Mutex requirements. */ - mutex(mutex const &) = delete; - mutex &operator=(mutex const &) = delete; + mutex(mutex const&) = delete; + mutex(mutex&&) = delete; + mutex& operator=(mutex const&) = delete; + mutex& operator=(mutex&&) = delete; void lock() { @@ -76,11 +80,16 @@ private: */ class lock_guard { public: - explicit lock_guard(pthread_mutex_t& mutex) : _mutex{mutex}, _guard(_mutex) + explicit lock_guard(pthread_mutex_t& mutex) : _mutex{ mutex }, _guard(_mutex) { } - lock_guard(const lock_guard &) = delete; + ~lock_guard() = default; + + lock_guard(const lock_guard&) = delete; + lock_guard(lock_guard&&) = delete; + lock_guard& operator=(const lock_guard&) = delete; + lock_guard& operator=(lock_guard&&) = delete; private: details::mutex _mutex; diff --git a/src/common/scope-exit.hpp b/src/common/scope-exit.hpp index e73b5792b..20d9b892d 100644 --- a/src/common/scope-exit.hpp +++ b/src/common/scope-exit.hpp @@ -58,6 +58,8 @@ public: * also propagate the scope_exit to another scope, should it be needed. */ scope_exit(const scope_exit&) = delete; + scope_exit& operator=(const scope_exit&) = delete; + scope_exit& operator=(scope_exit&&) = delete; scope_exit() = delete; void disarm() noexcept diff --git a/src/common/urcu.hpp b/src/common/urcu.hpp index abbf71679..edb53b7b5 100644 --- a/src/common/urcu.hpp +++ b/src/common/urcu.hpp @@ -27,10 +27,13 @@ namespace details { class read_lock { public: read_lock() = default; + ~read_lock() = default; /* "Not copyable" and "not moveable" Mutex requirements. */ - read_lock(read_lock const &) = delete; - read_lock &operator=(read_lock const &) = delete; + read_lock(read_lock const&) = delete; + read_lock(read_lock&&) = delete; + read_lock& operator=(read_lock&&) = delete; + read_lock& operator=(const read_lock&) = delete; void lock() { @@ -58,12 +61,16 @@ public: class read_lock_guard { public: read_lock_guard() = default; + ~read_lock_guard() = default; - read_lock_guard(const read_lock_guard &) = delete; + read_lock_guard(const read_lock_guard&) = delete; + read_lock_guard(read_lock_guard&&) = delete; + read_lock_guard& operator=(read_lock_guard&&) = delete; + read_lock_guard& operator=(const read_lock_guard&) = delete; private: details::read_lock _lock; - std::lock_guard _guard{_lock}; + std::lock_guard _guard{ _lock }; }; using unique_read_lock = std::unique_lock; -- 2.34.1