Merge remote-tracking branch 'origin/topic/neverlord/bugprone-findings'

* origin/topic/neverlord/bugprone-findings:
  Fix bugprone and modernize clang-tidy findings

CHANGES

@@ -1,3 +1,9 @@
+2.8.0-dev.121 | 2024-10-18 10:12:55 -0700
+
+  * Fix bugprone and modernize clang-tidy findings (Dominik Charousset)
+
+  * Pin prometheus to 1.2.4 (Arne Welzel, Corelight)
+
 2.8.0-dev.117 | 2024-09-14 15:58:08 +0200
 
   * Fix type 3 messages in broker-throughput (Dominik Charousset)

VERSION

@@ -1 +1 @@
-2.8.0-dev.117
+2.8.0-dev.121

libbroker/broker/detail/memory_backend.cc

@@ -109,7 +109,7 @@ expected<expirables> memory_backend::expiries() const {
 
   for (auto& p : store_) {
     if (p.second.second)
-      rval.emplace_back(expirable(p.first, *p.second.second));
+      rval.emplace_back(p.first, *p.second.second);
   }
 
   return {std::move(rval)};

libbroker/broker/error.cc

@@ -216,7 +216,8 @@ bool convertible_to_ec(uint8_t src) noexcept {
 }
 
 template <class List>
-bool convertible_to_error_impl(const List& xs) noexcept {
+bool convertible_to_error_impl(const List& xs) noexcept(
+  std::is_same_v<List, variant_list>) {
   if (!contains<std::string, ec, any_type>(xs)) {
     // There is one special case: default errors with enum value "none" fail to
     // convert to ec but are still legal.
@@ -234,11 +235,11 @@ bool convertible_to_error_impl(const List& xs) noexcept {
          || contains<endpoint_info, std::string>(args);
 }
 
-bool convertible_to_error(const vector& xs) noexcept {
+bool convertible_to_error(const vector& xs) {
   return convertible_to_error_impl(xs);
 }
 
-bool convertible_to_error(const data& src) noexcept {
+bool convertible_to_error(const data& src) {
   return convertible_to_error_impl(src.to_list());
 }
 

libbroker/broker/error.hh

@@ -276,22 +276,22 @@ struct can_convert_predicate<ec> {
 
 /// Checks whethter `src` is convertible to a `caf::error` with
 /// `category() == caf::atom("broker")`.
-bool convertible_to_error(const data& src) noexcept;
+bool convertible_to_error(const data& src);
+
+/// @copydoc convertible_to_error
+bool convertible_to_error(const vector& xs);
 
 /// Checks whethter `src` is convertible to a `caf::error` with
 /// `category() == caf::atom("broker")`.
 bool convertible_to_error(const variant& src) noexcept;
 
-/// @copydoc convertible_to_error
-bool convertible_to_error(const vector& xs) noexcept;
-
 template <>
 struct can_convert_predicate<error> {
-  static bool check(const data& src) noexcept {
+  static bool check(const data& src) {
     return convertible_to_error(src);
   }
 
-  static bool check(const vector& src) noexcept {
+  static bool check(const vector& src) {
     return convertible_to_error(src);
   }
 

libbroker/broker/format/bin.hh

@@ -203,7 +203,7 @@ std::enable_if_t<std::is_arithmetic_v<T>, OutIter> encode(T value,
   } else if constexpr (std::is_unsigned_v<T>) {
     return write_unsigned(value, out);
   } else {
-    static_assert(std::is_signed<T>::value);
+    static_assert(std::is_signed_v<T>);
     using unsigned_t = std::make_unsigned_t<T>;
     return write_unsigned(static_cast<unsigned_t>(value), out);
   }
@@ -435,8 +435,8 @@ public:
   }
 
   template <class T>
-  const T& field(std::string_view, const T& value) {
-    return value;
+  decltype(auto) field(std::string_view, T&& value) {
+    return std::forward<T>(value);
   }
 
   bool fields() {

libbroker/broker/internal/channel.hh

@@ -198,9 +198,10 @@ public:
       }
 
       void shipped(int64_t num) {
+        auto dbl_num = static_cast<double>(num);
         if (unacknowledged) {
-          unacknowledged->Decrement(num);
-          processed->Increment(num);
+          unacknowledged->Decrement(dbl_num);
+          processed->Increment(dbl_num);
         }
       }
     };
@@ -844,8 +845,9 @@ public:
     void try_consume_buffer() {
       auto i = buf_.begin();
       for (; i != buf_.end() && i->seq == next_seq_; ++i) {
-        if (i->content) {
-          backend_->consume(this, *i->content);
+        auto& content = i->content;
+        if (content) {
+          backend_->consume(this, *content);
         } else {
           if (auto err = backend_->consume_nil(this)) {
             buf_.erase(buf_.begin(), i);

libbroker/broker/internal/flow_scope.hh

@@ -50,7 +50,7 @@ public:
     if (deregister_cb_) {
       try {
         deregister_cb_(stats_);
-      } catch (...) {
+      } catch (...) { // NOLINT
         // The callbacks may not throw. However, we can't specify them noexcept
         // because std::function does not support noexcept signatures. Hence,
         // this catch-all to silence tool warnings.

libbroker/broker/internal/master_actor.cc

@@ -76,7 +76,7 @@ master_state::master_state(
     detail::die("failed to get master expiries while initializing");
   }
   if (auto entries = backend->size(); entries && *entries > 0) {
-    metrics.entries->Set(*entries);
+    metrics.entries->Set(static_cast<double>(*entries));
   }
   BROKER_INFO("attached master" << id << "to" << store_name);
 }

libbroker/broker/subnet.cc

@@ -78,8 +78,8 @@ bool convert(const std::string& str, subnet& sn) {
         sn = subnet{addr, static_cast<uint8_t>(len)};
         return true;
       }
-    } catch (std::exception&) {
-      // nop
+    } catch (const std::exception&) {
+      return false;
     }
   }
   return false;