Merge pull request #435

.clang-tidy

@@ -4,6 +4,7 @@
           clang-analyzer-*,
           modernize-*,
           performance-*,
+          -performance-enum-size,
           -bugprone-easily-swappable-parameters,
           -bugprone-forward-declaration-namespace,
           -clang-analyzer-cplusplus.NewDeleteLeaks,

CHANGES

@@ -1,3 +1,22 @@
+2.8.0-dev.140 | 2024-11-25 19:00:28 +0100
+
+  * Fix invalid range clang-tidy findings (Dominik Charousset, Corelight)
+
+    Fix occurrences of "The value '0' provided to the cast expression is not
+    in the valid range of values for 'p2p_message_type'."
+
+  * Fix clang-tidy performance warnings (Dominik Charousset, Corelight)
+
+    Addressed warnings:
+
+    - parameter is copied for each invocation but only used as a const
+      reference
+    - do not use 'std::endl' with streams; use '\\n' instead
+
+    Furthermore, `performance-enum-size` warnings have been disabled. Broker
+    uses several enum types in types that are exchanged over the network.
+    Changing the enum types would be a breaking change on the wire format.
+
 2.8.0-dev.137 | 2024-11-07 19:06:35 +0100
 
   * Update `clang-format` on CI (Dominik Charousset)

VERSION

@@ -1 +1 @@
-2.8.0-dev.137
+2.8.0-dev.140

libbroker/broker/configuration.cc

@@ -57,7 +57,7 @@ bool openssl_options::authentication_enabled() const noexcept {
 namespace {
 
 template <class... Ts>
-auto concat(Ts... xs) {
+auto concat(const Ts&... xs) {
   std::string result;
   ((result += xs), ...);
   return result;

libbroker/broker/detail/appliers.hh

@@ -125,7 +125,7 @@ struct retriever {
 
   template <class T>
   result_type operator()(const T& x) const {
-    return x;
+    return data{x};
   }
 
   static result_type at_index(const vector& v, count index) {
@@ -149,7 +149,7 @@ struct retriever {
   }
 
   result_type operator()(const set& s) const {
-    return s.count(aspect) == 1;
+    return data{s.count(aspect) == 1};
   }
 
   result_type operator()(const table& t) const {

libbroker/broker/detail/die.hh

@@ -21,7 +21,7 @@ void render(T&& x, Ts&&... xs) {
 template <class... Ts>
 [[noreturn]] void die(Ts&&... xs) {
   render(std::forward<Ts>(xs)...);
-  std::cerr << std::endl;
+  std::cerr << '\n';
   std::abort();
 }
 

libbroker/broker/envelope.cc

@@ -46,7 +46,7 @@ std::string to_string(envelope_type x) {
 }
 
 bool from_string(std::string_view str, envelope_type& x) {
-  auto tmp = p2p_message_type{0};
+  auto tmp = p2p_message_type::data;
   if (from_string(str, tmp) && static_cast<uint8_t>(tmp) <= 5) {
     x = static_cast<envelope_type>(tmp);
     return true;
@@ -57,7 +57,7 @@ bool from_string(std::string_view str, envelope_type& x) {
 
 bool from_integer(uint8_t val, envelope_type& x) {
   if (val <= 0x04) {
-    auto tmp = p2p_message_type{0};
+    auto tmp = p2p_message_type::data;
     if (from_integer(val, tmp)) {
       x = static_cast<envelope_type>(tmp);
       return true;
@@ -134,7 +134,7 @@ expected<envelope_ptr> envelope::deserialize(const std::byte* data,
     case envelope_type::data:
       if (auto res = data_envelope::deserialize(sender, receiver, ttl,
                                                 topic_str, data, size))
-        return *res;
+        return envelope_ptr{std::move(*res)};
       else
         return res.error();
     case envelope_type::command:
@@ -180,7 +180,7 @@ expected<envelope_ptr> envelope::deserialize_json(const char* data,
   // Note: must manually "unbox" the expected to convert from
   // expected<data_envelope_ptr> to expected<envelope_ptr>.
   if (res)
-    return *res;
+    return envelope_ptr{std::move(*res)};
   else
     return res.error();
 }

libbroker/broker/expected.hh

@@ -40,12 +40,6 @@ public:
 
   // -- constructors, destructors, and assignment operators --------------------
 
-  template <class U>
-  expected(U x, std::enable_if_t<std::is_convertible_v<U, T>>* = nullptr)
-    : engaged_(true) {
-    new (std::addressof(value_)) T(std::move(x));
-  }
-
   expected(T&& x) noexcept(nothrow_move) : engaged_(true) {
     new (std::addressof(value_)) T(std::move(x));
   }

libbroker/broker/internal/wire_format.cc

@@ -168,7 +168,7 @@ std::string stringify(const var_msg& msg) {
 
 var_msg decode(caf::const_byte_span bytes) {
   format::bin::v1::decoder src{bytes.data(), bytes.size()};
-  auto msg_type = p2p_message_type{0};
+  auto msg_type = p2p_message_type::data;
   if (!src.apply(msg_type)) {
     BROKER_ERROR("decode: failed to read the type tag");
     return make_var_msg_error(ec::invalid_message, "invalid message type tag"s);

libbroker/broker/ping_envelope.cc

@@ -80,8 +80,9 @@ expected<envelope_ptr> ping_envelope::deserialize(
   const endpoint_id& sender, const endpoint_id& receiver, uint16_t ttl,
   std::string_view topic_str, const std::byte* payload, size_t payload_size) {
   using impl_ptr = intrusive_ptr<envelope::deserialized<ping_envelope>>;
-  return impl_ptr::make(sender, receiver, ttl, topic_str, payload,
-                        payload_size);
+  auto ptr = impl_ptr::make(sender, receiver, ttl, topic_str, payload,
+                            payload_size);
+  return envelope_ptr{std::move(ptr)};
 }
 
 } // namespace broker

libbroker/broker/pong_envelope.cc

@@ -85,8 +85,9 @@ expected<envelope_ptr> pong_envelope::deserialize(
   const endpoint_id& sender, const endpoint_id& receiver, uint16_t ttl,
   std::string_view topic_str, const std::byte* payload, size_t payload_size) {
   using impl_ptr = intrusive_ptr<envelope::deserialized<pong_envelope>>;
-  return impl_ptr::make(sender, receiver, ttl, topic_str, payload,
-                        payload_size);
+  auto ptr = impl_ptr::make(sender, receiver, ttl, topic_str, payload,
+                            payload_size);
+  return envelope_ptr{std::move(ptr)};
 }
 
 } // namespace broker