chore: minor bandwidth code housekeeping (#6306)

libtransmission/bandwidth.cc

@@ -28,14 +28,14 @@ using namespace libtransmission::Values;
 
 Speed tr_bandwidth::get_speed(RateControl& r, unsigned int interval_msec, uint64_t now)
 {
-    if (now == 0)
+    if (now == 0U)
     {
         now = tr_time_msec();
     }
 
     if (now != r.cache_time_)
     {
-        uint64_t bytes = 0;
+        uint64_t bytes = 0U;
         uint64_t const cutoff = now - interval_msec;
 
         for (int i = r.newest_; r.date_[i] > cutoff;)
@@ -78,15 +78,15 @@ void tr_bandwidth::notify_bandwidth_consumed_bytes(uint64_t const now, RateContr
     }
 
     /* invalidate cache_val*/
-    r.cache_time_ = 0;
+    r.cache_time_ = 0U;
 }
 
 // ---
 
-tr_bandwidth::tr_bandwidth(tr_bandwidth* new_parent, bool is_group)
+tr_bandwidth::tr_bandwidth(tr_bandwidth* parent, bool is_group)
     : priority_(is_group ? TR_PRI_NONE : TR_PRI_NORMAL)
 {
-    this->set_parent(new_parent);
+    set_parent(parent);
 }
 
 // ---
@@ -136,7 +136,7 @@ void tr_bandwidth::set_parent(tr_bandwidth* new_parent)
 #endif
 
         new_parent->children_.push_back(this);
-        this->parent_ = new_parent;
+        parent_ = new_parent;
     }
 }
 
@@ -147,7 +147,7 @@ void tr_bandwidth::allocate_bandwidth(
     uint64_t period_msec,
     std::vector<std::shared_ptr<tr_peerIo>>& peer_pool)
 {
-    auto const priority = std::min(parent_priority, this->priority_);
+    auto const priority = std::min(parent_priority, priority_);
 
     // set the available bandwidth
     for (auto const dir : { TR_UP, TR_DOWN })
@@ -160,7 +160,7 @@ void tr_bandwidth::allocate_bandwidth(
     }
 
     // add this bandwidth's peer, if any, to the peer pool
-    if (auto shared = this->peer_.lock(); shared)
+    if (auto shared = peer_.lock(); shared)
     {
         TR_ASSERT(tr_isPriority(priority));
         shared->set_priority(priority);
@@ -168,7 +168,7 @@ void tr_bandwidth::allocate_bandwidth(
     }
 
     // traverse & repeat for the subtree
-    for (auto* child : this->children_)
+    for (auto* child : children_)
     {
         child->allocate_bandwidth(priority, period_msec, peer_pool);
     }
@@ -188,7 +188,7 @@ void tr_bandwidth::phase_one(std::vector<tr_peerIo*>& peers, tr_direction dir)
     // process until we run out of bandwidth and/or peers that can use it.
     for (size_t n_unfinished = std::size(peers); n_unfinished > 0U;)
     {
-        for (size_t i = 0; i < n_unfinished;)
+        for (size_t i = 0U; i < n_unfinished;)
         {
             // Value of 3000 bytes chosen so that when using µTP we'll send a full-size
             // frame right away and leave enough buffered data for the next frame to go
@@ -225,7 +225,7 @@ void tr_bandwidth::allocate(uint64_t period_msec)
     // allocateBandwidth () is a helper function with two purposes:
     // 1. allocate bandwidth to b and its subtree
     // 2. accumulate an array of all the peerIos from b and its subtree.
-    this->allocate_bandwidth(TR_PRI_NONE, period_msec, refs);
+    allocate_bandwidth(TR_PRI_NONE, period_msec, refs);
 
     for (auto const& io : refs)
     {
@@ -278,14 +278,14 @@ size_t tr_bandwidth::clamp(tr_direction const dir, size_t byte_count) const noex
 {
     TR_ASSERT(tr_isDirection(dir));
 
-    if (this->band_[dir].is_limited_)
+    if (band_[dir].is_limited_)
     {
-        byte_count = std::min(byte_count, this->band_[dir].bytes_left_);
+        byte_count = std::min(byte_count, band_[dir].bytes_left_);
     }
 
-    if (this->parent_ != nullptr && this->band_[dir].honor_parent_limits_ && byte_count > 0)
+    if (parent_ != nullptr && band_[dir].honor_parent_limits_ && byte_count > 0U)
     {
-        byte_count = this->parent_->clamp(dir, byte_count);
+        byte_count = parent_->clamp(dir, byte_count);
     }
 
     return byte_count;
@@ -295,39 +295,23 @@ void tr_bandwidth::notify_bandwidth_consumed(tr_direction dir, size_t byte_count
 {
     TR_ASSERT(tr_isDirection(dir));
 
-    Band* band = &this->band_[dir];
+    auto& band = band_[dir];
 
-    if (band->is_limited_ && is_piece_data)
+    if (band.is_limited_ && is_piece_data)
     {
-        band->bytes_left_ -= std::min(band->bytes_left_, byte_count);
+        band.bytes_left_ -= std::min(band.bytes_left_, byte_count);
     }
 
-#ifdef DEBUG_DIRECTION
+    notify_bandwidth_consumed_bytes(now, band.raw_, byte_count);
-
-    if (dir == DEBUG_DIRECTION && band_->isLimited)
-    {
-        fprintf(
-            stderr,
-            "%p consumed %5zu bytes of %5s data... was %6zu, now %6zu left\n",
-            this,
-            byte_count,
-            is_piece_data ? "piece" : "raw",
-            oldBytesLeft,
-            band_->bytesLeft);
-    }
-
-#endif
-
-    notify_bandwidth_consumed_bytes(now, band->raw_, byte_count);
 
     if (is_piece_data)
     {
-        notify_bandwidth_consumed_bytes(now, band->piece_, byte_count);
+        notify_bandwidth_consumed_bytes(now, band.piece_, byte_count);
     }
 
-    if (this->parent_ != nullptr)
+    if (parent_ != nullptr)
     {
-        this->parent_->notify_bandwidth_consumed(dir, byte_count, is_piece_data, now);
+        parent_->notify_bandwidth_consumed(dir, byte_count, is_piece_data, now);
     }
 }
 
@@ -336,17 +320,17 @@ void tr_bandwidth::notify_bandwidth_consumed(tr_direction dir, size_t byte_count
 tr_bandwidth_limits tr_bandwidth::get_limits() const
 {
     auto limits = tr_bandwidth_limits{};
-    limits.up_limit = this->get_desired_speed(TR_UP);
+    limits.up_limit = get_desired_speed(TR_UP);
-    limits.down_limit = this->get_desired_speed(TR_DOWN);
+    limits.down_limit = get_desired_speed(TR_DOWN);
-    limits.up_limited = this->is_limited(TR_UP);
+    limits.up_limited = is_limited(TR_UP);
-    limits.down_limited = this->is_limited(TR_DOWN);
+    limits.down_limited = is_limited(TR_DOWN);
     return limits;
 }
 
 void tr_bandwidth::set_limits(tr_bandwidth_limits const& limits)
 {
-    this->set_desired_speed(TR_UP, limits.up_limit);
+    set_desired_speed(TR_UP, limits.up_limit);
-    this->set_desired_speed(TR_DOWN, limits.down_limit);
+    set_desired_speed(TR_DOWN, limits.down_limit);
-    this->set_limited(TR_UP, limits.up_limited);
+    set_limited(TR_UP, limits.up_limited);
-    this->set_limited(TR_DOWN, limits.down_limited);
+    set_limited(TR_DOWN, limits.down_limited);
 }

libtransmission/bandwidth.h

@@ -80,12 +80,12 @@ struct tr_bandwidth
 private:
     using Speed = libtransmission::Values::Speed;
 
-    static constexpr size_t HistoryMSec = 2000U;
+    static constexpr auto HistoryMSec = 2000U;
-    static constexpr size_t GranularityMSec = 250U;
+    static constexpr auto GranularityMSec = 250U;
-    static constexpr size_t HistorySize = HistoryMSec / GranularityMSec;
+    static constexpr auto HistorySize = HistoryMSec / GranularityMSec;
 
 public:
-    explicit tr_bandwidth(tr_bandwidth* new_parent, bool is_group = false);
+    explicit tr_bandwidth(tr_bandwidth* parent, bool is_group = false);
 
     explicit tr_bandwidth(bool is_group = false)
         : tr_bandwidth{ nullptr, is_group }
@@ -107,7 +107,7 @@ public:
      */
     void set_peer(std::weak_ptr<tr_peerIo> peer) noexcept
     {
-        this->peer_ = std::move(peer);
+        peer_ = std::move(peer);
     }
 
     /**
@@ -123,14 +123,14 @@ public:
 
     void set_parent(tr_bandwidth* new_parent);
 
-    [[nodiscard]] constexpr tr_priority_t get_priority() const noexcept
+    [[nodiscard]] constexpr auto get_priority() const noexcept
     {
-        return this->priority_;
+        return priority_;
     }
 
     constexpr void set_priority(tr_priority_t prio) noexcept
     {
-        this->priority_ = prio;
+        priority_ = prio;
     }
 
     /**
@@ -143,7 +143,7 @@ public:
     {
         TR_ASSERT(tr_isDirection(dir));
 
-        return get_speed(this->band_[dir].raw_, HistoryMSec, now);
+        return get_speed(band_[dir].raw_, HistoryMSec, now);
     }
 
     /** @brief Get the number of piece data bytes read or sent by this bandwidth subtree. */
@@ -151,7 +151,7 @@ public:
     {
         TR_ASSERT(tr_isDirection(dir));
 
-        return get_speed(this->band_[dir].piece_, HistoryMSec, now);
+        return get_speed(band_[dir].piece_, HistoryMSec, now);
     }
 
     /**
@@ -161,8 +161,8 @@ public:
      */
     constexpr bool set_desired_speed(tr_direction dir, Speed desired_speed)
     {
-        auto& value = this->band_[dir].desired_speed_;
+        auto& value = band_[dir].desired_speed_;
-        bool const did_change = desired_speed != value;
+        auto const did_change = desired_speed != value;
         value = desired_speed;
         return did_change;
     }
@@ -173,7 +173,7 @@ public:
      */
     [[nodiscard]] constexpr auto get_desired_speed(tr_direction dir) const
     {
-        return this->band_[dir].desired_speed_;
+        return band_[dir].desired_speed_;
     }
 
     [[nodiscard]] bool is_maxed_out(tr_direction dir, uint64_t now_msec) const noexcept
@@ -193,8 +193,8 @@ public:
      */
     constexpr bool set_limited(tr_direction dir, bool is_limited)
     {
-        bool& value = this->band_[dir].is_limited_;
+        auto& value = band_[dir].is_limited_;
-        bool const did_change = is_limited != value;
+        auto const did_change = is_limited != value;
         value = is_limited;
         return did_change;
     }
@@ -204,7 +204,7 @@ public:
      */
     [[nodiscard]] constexpr bool is_limited(tr_direction dir) const noexcept
     {
-        return this->band_[dir].is_limited_;
+        return band_[dir].is_limited_;
     }
 
     /**
@@ -215,17 +215,15 @@ public:
      */
     constexpr bool honor_parent_limits(tr_direction direction, bool is_enabled)
     {
-        bool& value = this->band_[direction].honor_parent_limits_;
+        auto& value = band_[direction].honor_parent_limits_;
-        bool const did_change = is_enabled != value;
+        auto const did_change = is_enabled != value;
         value = is_enabled;
         return did_change;
     }
 
     [[nodiscard]] constexpr bool are_parent_limits_honored(tr_direction direction) const
     {
-        TR_ASSERT(tr_isDirection(direction));
+        return band_[direction].honor_parent_limits_;
-
-        return this->band_[direction].honor_parent_limits_;
     }
 
     [[nodiscard]] tr_bandwidth_limits get_limits() const;
@@ -279,7 +277,7 @@ private:
 
 /* @} */
 
-constexpr bool tr_isPriority(tr_priority_t p)
+constexpr auto tr_isPriority(tr_priority_t p)
 {
     return p == TR_PRI_LOW || p == TR_PRI_NORMAL || p == TR_PRI_HIGH;
 }

libtransmission/tr-buffer.h

@@ -217,7 +217,9 @@ public:
     size_t add_socket(tr_socket_t sockfd, size_t n_bytes, tr_error* error = nullptr)
     {
         auto const [buf, buflen] = reserve_space(n_bytes);
-        auto const n_read = recv(sockfd, reinterpret_cast<char*>(buf), std::min(n_bytes, buflen), 0);
+        n_bytes = std::min(n_bytes, buflen);
+        TR_ASSERT(n_bytes > 0U);
+        auto const n_read = recv(sockfd, reinterpret_cast<char*>(buf), n_bytes, 0);
         auto const err = sockerrno;
 
         if (n_read > 0)