fix: Bitfield.getRaw() regression (#2023)

* fix: Bitfield.getRaw() regression

libtransmission/bitfield.cc

@@ -7,47 +7,67 @@
  */
 
 #include <algorithm>
-#include <iterator> // std::back_inserter
+#include <vector>
-#include <numeric> // std::accumulate
 
 #include "transmission.h"
+
 #include "bitfield.h"
 #include "tr-assert.h"
-#include "span.h"
+#include "utils.h" /* tr_new0() */
 
 /****
 *****
 ****/
 
-std::array<int8_t const, 256> Bitfield::true_bits_lookup_ = {
+namespace
-    0, 1, 1, 2, 1, 2, 2, 3, 1, 2, 2, 3, 2, 3, 3, 4, //
-    1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, //
-    1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, //
-    2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, //
-    1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, //
-    2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, //
-    2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, //
-    3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7, //
-    1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, //
-    2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, //
-    2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, //
-    3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7, //
-    2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, //
-    3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7, //
-    3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7, //
-    4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8, //
-};
-
-size_t Bitfield::countArray() const
 {
-    return std::accumulate(
+
-        std::begin(bits_),
+constexpr size_t getBytesNeeded(size_t bit_count)
-        std::end(bits_),
+{
-        0,
+    return (bit_count >> 3) + ((bit_count & 7) != 0 ? 1 : 0);
-        [](auto acc, auto item) { return acc + true_bits_lookup_[item]; });
 }
 
-size_t Bitfield::countRangeImpl(size_t begin, size_t end) const
+static void setAllTrue(uint8_t* array, size_t bit_count)
+{
+    uint8_t constexpr Val = 0xFF;
+    size_t const n = getBytesNeeded(bit_count);
+
+    if (n > 0)
+    {
+        std::fill_n(array, n, Val);
+        array[n - 1] = Val << (n * 8 - bit_count);
+    }
+}
+
+constexpr int8_t const trueBitCount[256] = {
+    0, 1, 1, 2, 1, 2, 2, 3, 1, 2, 2, 3, 2, 3, 3, 4, 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, 1, 2, 2, 3, 2,
+    3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 1, 2, 2, 3, 2, 3, 3, 4, 2, 3,
+    3, 4, 3, 4, 4, 5, 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5,
+    6, 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7, 1, 2, 2, 3, 2, 3, 3, 4, 2, 3, 3, 4, 3, 4, 4, 5, 2, 3, 3, 4,
+    3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 3, 4, 4, 5, 4, 5, 5, 6, 4,
+    5, 5, 6, 5, 6, 6, 7, 2, 3, 3, 4, 3, 4, 4, 5, 3, 4, 4, 5, 4, 5, 5, 6, 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6,
+    6, 7, 3, 4, 4, 5, 4, 5, 5, 6, 4, 5, 5, 6, 5, 6, 6, 7, 4, 5, 5, 6, 5, 6, 6, 7, 5, 6, 6, 7, 6, 7, 7, 8
+};
+
+} // namespace
+
+/****
+*****
+****/
+
+size_t tr_bitfield::countFlags() const
+{
+    size_t ret = 0;
+
+    for (auto ch : flags_)
+    {
+        ret += trueBitCount[ch];
+    }
+
+    return ret;
+}
+
+size_t tr_bitfield::countFlags(size_t begin, size_t end) const
 {
     size_t ret = 0;
     size_t const first_byte = begin >> 3U;
@@ -58,81 +78,85 @@ size_t Bitfield::countRangeImpl(size_t begin, size_t end) const
         return 0;
     }
 
-    if (first_byte >= std::size(bits_))
+    if (first_byte >= std::size(flags_))
     {
         return 0;
     }
 
     TR_ASSERT(begin < end);
+    TR_ASSERT(!std::empty(flags_));
 
     if (first_byte == last_byte)
     {
-        uint8_t val = bits_[first_byte];
+        int i;
+        uint8_t val = flags_[first_byte];
 
-        int i = begin - (first_byte * 8);
+        i = begin - (first_byte * 8);
         val <<= i;
         val >>= i;
         i = (last_byte + 1) * 8 - end;
         val >>= i;
         val <<= i;
 
-        ret += true_bits_lookup_[val];
+        ret += trueBitCount[val];
     }
     else
     {
-        size_t const walk_end = std::min(std::size(bits_), last_byte);
+        uint8_t val;
+        size_t const walk_end = std::min(std::size(flags_), last_byte);
 
         /* first byte */
         size_t const first_shift = begin - (first_byte * 8);
-        uint8_t val = bits_[first_byte];
+        val = flags_[first_byte];
         val <<= first_shift;
         val >>= first_shift;
-        ret += true_bits_lookup_[val];
+        ret += trueBitCount[val];
 
         /* middle bytes */
         for (size_t i = first_byte + 1; i < walk_end; ++i)
         {
-            ret += true_bits_lookup_[bits_[i]];
+            ret += trueBitCount[flags_[i]];
         }
 
         /* last byte */
-        if (last_byte < std::size(bits_))
+        if (last_byte < std::size(flags_))
         {
             size_t const last_shift = (last_byte + 1) * 8 - end;
-            val = bits_[last_byte];
+            val = flags_[last_byte];
             val >>= last_shift;
             val <<= last_shift;
-            ret += true_bits_lookup_[val];
+            ret += trueBitCount[val];
         }
     }
 
-    TR_ASSERT(ret <= (end - begin));
+    TR_ASSERT(ret <= (begin - end));
     return ret;
 }
 
-bool Bitfield::readBit(size_t n) const
+size_t tr_bitfield::count(size_t begin, size_t end) const
 {
-    switch (mode_)
+    if (hasAll())
     {
-    case OperationMode::Normal:
+        return end - begin;
-        {
+    }
-            auto const byte_offset = n >> 3;
 
-            if (byte_offset >= std::size(bits_))
+    if (hasNone())
+    {
+        return 0;
+    }
+
+    return countFlags(begin, end);
+}
+
+bool tr_bitfield::testFlag(size_t n) const
+{
+    if (n >> 3U >= std::size(flags_))
     {
         return false;
     }
 
-            return ((bits_[byte_offset] << (n & 7)) & 0x80) != 0;
+    bool ret = (flags_[n >> 3U] << (n & 7U) & 0x80) != 0;
-        }
+    return ret;
-    case OperationMode::All:
-        return true;
-    case OperationMode::None:
-        return false;
-    case OperationMode::Start:
-        TR_UNREACHABLE();
-    }
-    return false;
 }
 
 /***
@@ -141,294 +165,272 @@ bool Bitfield::readBit(size_t n) const
 
 #ifdef TR_ENABLE_ASSERTS
 
-bool Bitfield::isValid() const
+bool tr_bitfield::assertValid() const
 {
-    TR_ASSERT_MSG(
+    TR_ASSERT(std::empty(flags_) || true_count_ == countFlags());
-        std::size(bits_) == 0 || true_count_ == countArray(),
-        "Invalid Bitfield state: bits.size()=%zu bit_count=%zu true_count=%zu countArray()=%zu",
-        std::size(bits_),
-        bit_count_,
-        true_count_,
-        countArray());
 
     return true;
 }
 
 #endif
 
-size_t Bitfield::countBits() const
+std::vector<uint8_t> tr_bitfield::raw() const
 {
-    TR_ASSERT(isValid());
+    auto const n = getBytesNeeded(bit_count_);
 
-    return true_count_;
+    if (!std::empty(flags_))
+    {
+        return flags_;
     }
 
-void Bitfield::setBitsInArray(std::vector<uint8_t>& array, size_t bit_count)
+    auto raw = std::vector<uint8_t>(n);
-{
-    TR_ASSERT(getStorageSize(bit_count) == std::size(array));
 
-    if (!std::empty(array) && bit_count > 0)
+    if (hasAll())
     {
-        auto const last_byte_index = getStorageSize(bit_count) - 1;
+        setAllTrue(std::data(raw), std::size(raw));
+    }
 
-        std::fill_n(std::begin(array), last_byte_index, 0xFF);
+    return raw;
-        array[last_byte_index] = 0xFF << (last_byte_index * 8 - bit_count);
+}
+
+void tr_bitfield::ensureBitsAlloced(size_t n)
+{
+    bool const has_all = hasAll();
+
+    size_t const bytes_needed = has_all ? getBytesNeeded(std::max(n, true_count_)) : getBytesNeeded(n);
+
+    if (std::size(flags_) < bytes_needed)
+    {
+        flags_.resize(bytes_needed);
+
+        if (has_all)
+        {
+            setAllTrue(std::data(flags_), true_count_);
+        }
     }
 }
 
-std::vector<uint8_t> Bitfield::getRaw() const
+bool tr_bitfield::ensureNthBitAlloced(size_t nth)
 {
-    TR_ASSERT(bit_count_ > 0);
+    // count is zero-based, so we need to allocate nth+1 bits before setting the nth */
-
+    if (nth == SIZE_MAX)
-    size_t const n = getStorageSize(bit_count_);
-    auto new_bits = std::vector<uint8_t>(n);
-
-    if (!std::empty(bits_))
     {
-        TR_ASSERT(std::size(bits_) <= n);
+        return false;
-        std::copy(std::cbegin(bits_), std::cend(bits_), std::back_inserter(new_bits));
-    }
-    else if (hasAll())
-    {
-        setBitsInArray(new_bits, bit_count_);
     }
 
-    return new_bits;
+    ensureBitsAlloced(nth + 1);
+    return true;
 }
 
-void Bitfield::ensureNthBitFits(size_t n)
+void tr_bitfield::freeArray()
 {
-    TR_ASSERT_MSG(mode_ == OperationMode::Normal, "Can only reallocate storage in Normal mode");
+    flags_ = std::vector<uint8_t>{};
-
-    size_t bytes_needed = getStorageSize(std::max(n, true_count_));
-
-    if (std::size(bits_) < bytes_needed)
-    {
-        bits_.resize(bytes_needed);
 }
+
+void tr_bitfield::setTrueCount(size_t n)
+{
+    TR_ASSERT(bit_count_ == 0 || n <= bit_count_);
+
+    true_count_ = n;
+    have_all_hint_ = n == bit_count_;
+    have_none_hint_ = n == 0;
+
+    if (hasAll() || hasNone())
+    {
+        freeArray();
+    }
+
+    TR_ASSERT(assertValid());
+}
+
+void tr_bitfield::rebuildTrueCount()
+{
+    setTrueCount(countFlags());
+}
+
+void tr_bitfield::incrementTrueCount(size_t inc)
+{
+    TR_ASSERT(bit_count_ == 0 || inc <= bit_count_);
+    TR_ASSERT(bit_count_ == 0 || true_count_ <= bit_count_ - inc);
+
+    setTrueCount(true_count_ + inc);
+}
+
+void tr_bitfield::decrementTrueCount(size_t dec)
+{
+    TR_ASSERT(bit_count_ == 0 || dec <= bit_count_);
+    TR_ASSERT(bit_count_ == 0 || true_count_ >= dec);
+
+    setTrueCount(true_count_ - dec);
 }
 
 /****
 *****
 ****/
 
-Bitfield::Bitfield(size_t bit_count)
+tr_bitfield::tr_bitfield(size_t bit_count)
+    : bit_count_{ bit_count }
 {
-    bit_count_ = bit_count;
+    TR_ASSERT(assertValid());
+}
+
+void tr_bitfield::setHasNone()
+{
+    freeArray();
     true_count_ = 0;
-    setMode(OperationMode::Normal);
+    have_all_hint_ = false;
+    have_none_hint_ = true;
 
-    TR_ASSERT(isValid());
+    TR_ASSERT(assertValid());
 }
 
-void Bitfield::setMode(Bitfield::OperationMode new_mode)
+void tr_bitfield::setHasAll()
 {
-    switch (new_mode)
+    freeArray();
-    {
-    case OperationMode::Normal:
-        switch (mode_)
-        {
-        case OperationMode::All:
-            {
-                // Switching from ALL mode to NORMAL, should set the bits
-                mode_ = OperationMode::Normal;
-                ensureNthBitFits(bit_count_);
-                setBitRangeImpl(0, bit_count_ - 1);
-                true_count_ = bit_count_; // switching from mode ALL, all bits are set
-                break;
-            }
-        case OperationMode::None:
-            {
-                // Switching from ALL mode to NORMAL, should set the bits
-                mode_ = OperationMode::Normal;
-                ensureNthBitFits(bit_count_);
-                clearBitRangeImpl(0, bit_count_ - 1);
-                true_count_ = 0; // switching from mode NONE, all bits are not set
-                break;
-            }
-        case OperationMode::Start:
-            mode_ = OperationMode::Normal;
-            // fall through
-        case OperationMode::Normal:
-            break;
-        }
-        break;
-    case OperationMode::All:
-        clearStorage();
     true_count_ = bit_count_;
-        mode_ = OperationMode::All;
+    have_all_hint_ = true;
-        break;
+    have_none_hint_ = false;
-    case OperationMode::None:
+
-        clearStorage();
+    TR_ASSERT(assertValid());
-        true_count_ = 0;
-        mode_ = OperationMode::None;
-        break;
-    case OperationMode::Start:
-        TR_UNREACHABLE();
-        break;
 }
 
-    TR_ASSERT(isValid());
+void tr_bitfield::setRaw(uint8_t const* raw, size_t byte_count, bool bounded)
-}
-
-Bitfield::Bitfield(Span<uint8_t> new_bits, size_t bit_count, bool bounded)
-    : bit_count_(bit_count)
 {
-    true_count_ = 0;
+    if (bounded)
-    setMode(OperationMode::Normal);
+    {
+        byte_count = std::min(byte_count, getBytesNeeded(bit_count_));
+    }
 
-    // Having bounded=true, limits the amount of moved data to available storage size
+    flags_ = std::vector<uint8_t>(raw, raw + byte_count);
-    size_t byte_count = bounded ? std::min(std::size(new_bits), getStorageSize(bit_count_)) : std::size(new_bits);
-
-    bits_.resize(byte_count);
-    std::copy_n(std::begin(new_bits), byte_count, std::begin(bits_));
 
     if (bounded)
     {
-        /* ensure the excess new_bits are set to '0' */
+        /* ensure the excess bits are set to '0' */
-        int const excess_bit_count = bit_count_ & 7;
+        int const excess_bit_count = byte_count * 8 - bit_count_;
+
+        TR_ASSERT(excess_bit_count >= 0);
+        TR_ASSERT(excess_bit_count <= 7);
 
         if (excess_bit_count != 0)
         {
-            bits_[byte_count - 1] &= 0xFF << excess_bit_count;
+            flags_.back() &= 0xff << excess_bit_count;
         }
     }
 
-    setTrueCount(countArray());
+    rebuildTrueCount();
 }
 
-Bitfield::Bitfield(bool const* flags, size_t n)
+void tr_bitfield::setFromBools(bool const* flags, size_t n)
-    : bit_count_{ n }
-    , true_count_{ size_t(std::count(flags, flags + n, true)) }
 {
-    if (true_count_ == 0)
+    size_t trueCount = 0;
+
+    freeArray();
+    ensureBitsAlloced(n);
+
+    for (size_t i = 0; i < n; ++i)
     {
-        mode_ = OperationMode::None;
+        if (flags[i])
+        {
+            ++trueCount;
+            flags_[i >> 3U] |= (0x80 >> (i & 7U));
         }
-    else if (true_count_ == bit_count_)
+    }
+
+    setTrueCount(trueCount);
+}
+
+void tr_bitfield::set(size_t nth, bool value)
 {
-        mode_ = OperationMode::All;
+    if (test(nth) == value)
+    {
+        return;
+    }
+
+    if (!ensureNthBitAlloced(nth))
+    {
+        return;
+    }
+
+    if (value)
+    {
+        flags_[nth >> 3U] |= 0x80 >> (nth & 7U);
+        incrementTrueCount(1);
     }
     else
     {
-        mode_ = OperationMode::Normal;
+        flags_[nth >> 3U] &= 0xff7f >> (nth & 7U);
-        ensureNthBitFits(n);
+        decrementTrueCount(1);
-        TR_ASSERT(std::size(bits_) >= getStorageSize(n));
-        for (size_t index = 0; index < n; ++index)
-        {
-            if (flags[index])
-            {
-                bits_[index >> 3] |= (0x80 >> (index & 7));
-            }
-        }
-    }
-
-    TR_ASSERT(isValid());
-}
-
-void Bitfield::setBit(size_t bit_index)
-{
-    switch (mode_)
-    {
-    case OperationMode::Normal:
-        {
-            setBitImpl(bit_index);
-            break;
-        }
-    case OperationMode::All:
-        TR_ASSERT(bit_index <= bit_count_);
-        break;
-    case OperationMode::None:
-        setMode(OperationMode::Normal);
-        setBitImpl(bit_index);
-        break;
-    case OperationMode::Start:
-        TR_UNREACHABLE();
-        break;
     }
 }
 
-void Bitfield::setBitRange(size_t begin, size_t end)
+/* Sets bit range [begin, end) to 1 */
+void tr_bitfield::setRange(size_t begin, size_t end, bool value)
 {
-    if (mode_ == OperationMode::All)
+    // did anything change?
+    size_t const old_count = count(begin, end);
+    size_t const new_count = value ? (end - begin) : 0;
+    if (old_count == new_count)
     {
         return;
     }
 
-    if (mode_ == OperationMode::None)
+    // bounds check
-    {
+    --end;
-        setMode(OperationMode::Normal);
-    }
-
-    size_t const true_bits_difference = (end - begin) - countRange(begin, end);
-
-    if (true_bits_difference == 0)
-    {
-        return;
-    }
-
-    end--;
-
     if (end >= bit_count_ || begin > end)
     {
         return;
     }
 
-    setBitRangeImpl(begin, end);
+    if (!ensureNthBitAlloced(end))
-
-    TR_ASSERT(true_count_ + true_bits_difference <= bit_count_);
-    setTrueCount(true_count_ + true_bits_difference);
-}
-
-void Bitfield::clearBit(size_t bit)
-{
-    TR_ASSERT(isValid());
-
-    switch (mode_)
-    {
-    case OperationMode::Normal:
-        clearBitImpl(bit);
-        break;
-    case OperationMode::All:
-        setMode(OperationMode::Normal);
-        clearBitImpl(bit);
-        break;
-    case OperationMode::None:
-        break;
-    case OperationMode::Start:
-        TR_UNREACHABLE();
-        break;
-    }
-}
-
-void Bitfield::clearBitRange(size_t begin, size_t end)
-{
-    if (mode_ == OperationMode::None)
     {
         return;
     }
 
-    if (mode_ == OperationMode::All)
+    size_t walk = begin >> 3;
+    size_t const last_byte = end >> 3;
+
+    if (value)
     {
-        setMode(OperationMode::Normal);
+        unsigned char const first_mask = ~(0xff << (8 - (begin & 7)));
-    }
+        unsigned char const last_mask = 0xff << (7 - (end & 7));
 
-    size_t const true_bits_difference = countRange(begin, end); // all true bits in range will be gone
+        if (walk == last_byte)
-
-    if (true_bits_difference == 0)
         {
-        return;
+            flags_[walk] |= first_mask & last_mask;
         }
-
+        else
-    end--;
-
-    if (end >= bit_count_ || begin > end)
         {
-        return;
+            flags_[walk] |= first_mask;
+            flags_[last_byte] |= last_mask;
+
+            if (++walk < last_byte)
+            {
+                std::fill_n(std::begin(flags_) + walk, last_byte - walk, 0xff);
+            }
         }
 
-    clearBitRangeImpl(begin, end);
+        incrementTrueCount(new_count - old_count);
-
+    }
-    TR_ASSERT(true_count_ >= true_bits_difference);
+    else
-    setTrueCount(true_count_ - true_bits_difference);
+    {
+        unsigned char const first_mask = 0xff << (8 - (begin & 7));
+        unsigned char const last_mask = ~(0xff << (7 - (end & 7)));
+
+        if (walk == last_byte)
+        {
+            flags_[walk] &= first_mask | last_mask;
+        }
+        else
+        {
+            flags_[walk] &= first_mask;
+            flags_[last_byte] &= last_mask;
+
+            if (++walk < last_byte)
+            {
+                std::fill_n(std::begin(flags_) + walk, last_byte - walk, 0);
+            }
+        }
+
+        decrementTrueCount(old_count);
+    }
 }

libtransmission/bitfield.h

@@ -12,253 +12,111 @@
 #error only libtransmission should #include this header.
 #endif
 
-#include <array>
+#include <cstddef>
+#include <cstdint>
 #include <vector>
 
-#include "transmission.h"
-#include "tr-macros.h"
 #include "tr-assert.h"
-#include "span.h"
 
-/// @brief Implementation of the BitTorrent spec's Bitfield array of bits
+/**
-struct Bitfield
+ * @brief Implementation of the BitTorrent spec's Bitfield array of bits.
+ *
+ * This is for tracking the pieces a peer has. Its functionality is like
+ * a bitset or vector<bool> with some added use cases:
+ *
+ * - It needs to be able to read/write the left-to-right bitfield format
+ *   specified in the bittorrent spec. This is what raw() and getRaw()
+ *   are for.
+ *
+ * - "Have all" is a special case where we know the peer has all the
+ *   pieces and don't need to check the bit array. This is useful since
+ *   (a) it's very common (i.e. seeds) and saves memory and work of
+ *   allocating a bit array and doing lookups, and (b) if we have a
+ *   magnet link and haven't gotten the metainfo yet, we may not know
+ *   how many pieces there are -- but we can still know "this peer has
+ *   all of them".
+ *
+ * - "Have none" is another special case that has the same advantages
+ *   and motivations as "Have all".
+ */
+class tr_bitfield
 {
 public:
-    /// @brief State diagram for modes of operation: None -> Normal <==> All
+    explicit tr_bitfield(size_t bit_count);
-    /// ALL and NONE: Special cases for when full or empty but we don't know the bitCount.
-    /// This occurs when a magnet link's peers send have all / have none
-    enum struct OperationMode
-    {
-        /// @brief State at the creation
-        Start,
-        /// @brief Normal operation: storage of bytes contains bits to set or clear
-        Normal,
-        /// @brief If bit_count_==0, storage is inactive, consider all bits to be 1
-        All,
-        /// @brief If bit_count_==0, storage is inactive, consider all bits to be 0
-        None,
-    };
 
-    /***
+    void setHasAll();
-    ****  life cycle
+    void setHasNone();
-    ***/
-    explicit Bitfield(size_t bit_count);
 
-    Bitfield()
+    // set one or more bits
-        : Bitfield(0)
+    void set(size_t bit, bool value = true);
+    void setRange(size_t begin, size_t end, bool value = true);
+    void unset(size_t bit)
     {
+        set(bit, false);
     }
-
+    void unsetRange(size_t begin, size_t end)
-    /// @brief Builds bits from array of boolean flags
-    Bitfield(bool const* bytes, size_t n);
-
-    ~Bitfield()
     {
-        setMode(OperationMode::None);
+        setRange(begin, end, false);
     }
+    void setFromBools(bool const* bytes, size_t n);
 
-    /***
+    // "raw" here is in BEP0003 format: "The first byte of the bitfield
-    ****
+    // corresponds to indices 0 - 7 from high bit to low bit, respectively.
-    ***/
+    // The next one 8-15, etc. Spare bits at the end are set to zero.
+    void setRaw(uint8_t const* bits, size_t byte_count, bool bounded);
+    std::vector<uint8_t> raw() const;
 
-    /// @brief Creates new Bitfield with same count of bits as *this and replaces data from new_bits
-    void setFrom(Span<uint8_t> new_bits, bool bounded)
-    {
-        *this = Bitfield(new_bits, this->bit_count_, bounded);
-    }
-
-    /// @brief Change the state (mode of operation)
-    void setMode(OperationMode new_mode);
-
-    /// @brief Sets one bit
-    void setBit(size_t bit_index);
-
-    /// @brief Sets bit range [begin, end) to 1
-    void setBitRange(size_t begin, size_t end);
-
-    /// @brief Clears one bit
-    void clearBit(size_t bit);
-
-    /// @brief Clears bit range [begin, end) to 0
-    void clearBitRange(size_t begin, size_t end);
-
-    /***
-    ****
-    ***/
-
-    [[nodiscard]] size_t countRange(size_t begin, size_t end) const
-    {
-        if (hasAll())
-        {
-            return end - begin;
-        }
-
-        if (hasNone())
-        {
-            return 0;
-        }
-
-        return countRangeImpl(begin, end);
-    }
-
-    [[nodiscard]] size_t countBits() const;
-
-    /// @brief Returns whether all bits are set, or mode is ALL. Always false for zero sized bitfield.
     [[nodiscard]] constexpr bool hasAll() const
     {
-        return ((bit_count_ != 0) && (true_count_ == bit_count_)) || (mode_ == OperationMode::All);
+        return have_all_hint_ || (bit_count_ > 0 && bit_count_ == true_count_);
     }
 
-    /// @brief Returns whether all bits are clear, or mode is NONE. Always false for zero sized bitfield.
     [[nodiscard]] constexpr bool hasNone() const
     {
-        return ((bit_count_ != 0) && (true_count_ == 0)) || (mode_ == OperationMode::None);
+        return have_none_hint_ || (bit_count_ > 0 && true_count_ == 0);
     }
 
-    [[nodiscard]] bool readBit(size_t n) const;
+    [[nodiscard]] bool test(size_t bit) const
+    {
+        return hasAll() || (!hasNone() && testFlag(bit));
+    }
 
-    /***
+    [[nodiscard]] constexpr size_t count() const
-    ****
+    {
-    ***/
+        return true_count_;
+    }
 
-    [[nodiscard]] std::vector<uint8_t> getRaw() const;
+    [[nodiscard]] size_t count(size_t begin, size_t end) const;
 
-    [[nodiscard]] size_t getBitCount() const
+    [[nodiscard]] constexpr size_t size() const
     {
         return bit_count_;
     }
 
-private:
-    /// @brief Copies bits from the readonly view new_bits. Use Bitfield::setFrom to access this constructor
-    /// @param bounded Whether incoming data is constrained by our memory and bit size
-    Bitfield(Span<uint8_t> new_bits, size_t bit_count, bool bounded);
-
-    /// @brief Contains lookup table for how many set bits are there in 0..255
-    static std::array<int8_t const, 256> true_bits_lookup_;
-
-    static constexpr size_t getStorageSize(size_t bit_count)
-    {
-        return 1 + ((bit_count + 7) >> 3);
-    }
-
-    [[nodiscard]] size_t countArray() const;
-    [[nodiscard]] size_t countRangeImpl(size_t begin, size_t end) const;
-
-    /// @brief Given bit count, sets that many bits in the array, assumes array size is big enough.
-    static void setBitsInArray(std::vector<uint8_t>& array, size_t bit_count);
-
-    void ensureNthBitFits(size_t n);
-
-    inline void setTrueCount(size_t n)
-    {
-        TR_ASSERT(mode_ == OperationMode::Normal);
-        TR_ASSERT(n <= bit_count_);
-
-        true_count_ = n;
-
-        TR_ASSERT(isValid());
-    }
-
 #ifdef TR_ENABLE_ASSERTS
-    [[nodiscard]] bool isValid() const;
+    bool assertValid() const;
 #endif
 
-    /// @brief Set the bit
+private:
-    inline void setBitImpl(size_t bit)
+    std::vector<uint8_t> flags_;
-    {
+    [[nodiscard]] size_t countFlags() const;
-        TR_ASSERT_MSG(mode_ == OperationMode::Normal, "Can only set bits in Normal operation mode");
+    [[nodiscard]] size_t countFlags(size_t begin, size_t end) const;
-        TR_ASSERT(isValid());
+    [[nodiscard]] bool testFlag(size_t bit) const;
 
-        if (!readBit(bit))
+    void ensureBitsAlloced(size_t n);
-        {
+    [[nodiscard]] bool ensureNthBitAlloced(size_t nth);
-            ensureNthBitFits(bit);
+    void freeArray();
 
-            auto const byte_offset = bit >> 3;
+    void setTrueCount(size_t n);
-            size_t bit_value = size_t{ 0x80U } >> (bit & 7);
+    void rebuildTrueCount();
+    void incrementTrueCount(size_t inc);
+    void decrementTrueCount(size_t dec);
 
-            bits_[byte_offset] |= bit_value;
-            setTrueCount(true_count_ + 1);
-        }
-    }
-
-    /// @brief Clear the bit
-    inline void clearBitImpl(size_t bit)
-    {
-        TR_ASSERT_MSG(mode_ == OperationMode::Normal, "Can only set bits in Normal operation mode");
-        TR_ASSERT(isValid());
-
-        if (readBit(bit))
-        {
-            ensureNthBitFits(bit);
-
-            size_t const byte_mask = size_t{ 0xFF7FU } >> (bit & 7U);
-            bits_[bit >> 3] &= byte_mask;
-
-            TR_ASSERT(true_count_ > 0);
-            setTrueCount(true_count_ - 1);
-        }
-    }
-
-    /// @brief Ensure that the memory is properly deallocated and size becomes zero
-    inline void clearStorage()
-    {
-        bits_ = std::vector<uint8_t>();
-    }
-
-    inline void setBitRangeImpl(size_t begin, size_t end)
-    {
-        size_t start_byte = begin >> 3;
-        size_t start_mask = ~(size_t{ 0xFFU } << (8 - (begin & 7)));
-
-        size_t end_byte = end >> 3;
-        size_t end_mask = size_t{ 0xFFU } << (7 - (end & 7));
-
-        ensureNthBitFits(end);
-
-        if (start_byte == end_byte)
-        {
-            bits_[start_byte] |= start_mask & end_mask;
-        }
-        else
-        {
-            bits_[start_byte] |= start_mask;
-            bits_[end_byte] |= end_mask;
-
-            if (++start_byte < end_byte)
-            {
-                std::fill_n(std::begin(bits_) + start_byte, end_byte - start_byte, 0xFF);
-            }
-        }
-    }
-
-    inline void clearBitRangeImpl(size_t begin, size_t end)
-    {
-        size_t start_byte = begin >> 3;
-        size_t start_mask = size_t{ 0xFFU } << (8 - (begin & 7));
-
-        size_t end_byte = end >> 3;
-        size_t end_mask = ~(size_t{ 0xFFU } << (7 - (end & 7)));
-
-        ensureNthBitFits(end);
-
-        if (start_byte == end_byte)
-        {
-            bits_[start_byte] &= start_mask | end_mask;
-        }
-        else
-        {
-            bits_[start_byte] &= start_mask;
-            bits_[end_byte] &= end_mask;
-
-            if (++start_byte < end_byte)
-            {
-                std::fill_n(std::begin(bits_) + start_byte, end_byte - start_byte, 0);
-            }
-        }
-    }
-
-    std::vector<uint8_t> bits_;
     size_t bit_count_ = 0;
     size_t true_count_ = 0;
-    OperationMode mode_ = OperationMode::Start;
+
+    /* Special cases for when full or empty but we don't know the bitCount.
+       This occurs when a magnet link's peers send have all / have none */
+    bool have_all_hint_ = false;
+    bool have_none_hint_ = false;
 };

libtransmission/completion.cc

@@ -21,17 +21,17 @@ static void tr_cpReset(tr_completion* cp)
     cp->sizeNow = 0;
     cp->sizeWhenDoneIsDirty = true;
     cp->haveValidIsDirty = true;
-    cp->blockBitfield->setMode(Bitfield::OperationMode::None);
+    cp->blockBitfield->setHasNone();
 }
 
 void tr_cpConstruct(tr_completion* cp, tr_torrent* tor)
 {
     cp->tor = tor;
-    cp->blockBitfield = new Bitfield(tor->blockCount);
+    cp->blockBitfield = new tr_bitfield(tor->blockCount);
     tr_cpReset(cp);
 }
 
-void tr_cpBlockInit(tr_completion* cp, Bitfield const& b)
+void tr_cpBlockInit(tr_completion* cp, tr_bitfield const& b)
 {
     tr_cpReset(cp);
 
@@ -39,11 +39,11 @@ void tr_cpBlockInit(tr_completion* cp, Bitfield const& b)
     *(cp->blockBitfield) = b;
 
     // set sizeNow
-    cp->sizeNow = cp->blockBitfield->countBits();
+    cp->sizeNow = cp->blockBitfield->count();
     TR_ASSERT(cp->sizeNow <= cp->tor->blockCount);
     cp->sizeNow *= cp->tor->blockSize;
 
-    if (b.readBit(cp->tor->blockCount - 1))
+    if (b.test(cp->tor->blockCount - 1))
     {
         cp->sizeNow -= (cp->tor->blockSize - cp->tor->lastBlockSize);
     }
@@ -93,7 +93,7 @@ void tr_cpPieceRem(tr_completion* cp, tr_piece_index_t piece)
 
     cp->haveValidIsDirty = true;
     cp->sizeWhenDoneIsDirty = true;
-    cp->blockBitfield->clearBitRange(f, l + 1);
+    cp->blockBitfield->setRange(f, l + 1, false);
 }
 
 void tr_cpPieceAdd(tr_completion* cp, tr_piece_index_t piece)
@@ -116,7 +116,7 @@ void tr_cpBlockAdd(tr_completion* cp, tr_block_index_t block)
     {
         tr_piece_index_t const piece = tr_torBlockPiece(cp->tor, block);
 
-        cp->blockBitfield->setBit(block);
+        cp->blockBitfield->set(block);
         cp->sizeNow += tr_torBlockCountBytes(tor, block);
 
         cp->haveValidIsDirty = true;
@@ -182,10 +182,10 @@ uint64_t tr_cpSizeWhenDone(tr_completion const* ccp)
                     auto l = tr_block_index_t{};
                     tr_torGetPieceBlockRange(cp->tor, p, &f, &l);
 
-                    n = cp->blockBitfield->countRange(f, l + 1);
+                    n = cp->blockBitfield->count(f, l + 1);
                     n *= cp->tor->blockSize;
 
-                    if (l == cp->tor->blockCount - 1 && cp->blockBitfield->readBit(l))
+                    if (l == cp->tor->blockCount - 1 && cp->blockBitfield->test(l))
                     {
                         n -= cp->tor->blockSize - cp->tor->lastBlockSize;
                     }
@@ -232,7 +232,7 @@ void tr_cpGetAmountDone(tr_completion const* cp, float* tab, int tabCount)
             auto f = tr_block_index_t{};
             auto l = tr_block_index_t{};
             tr_torGetPieceBlockRange(cp->tor, piece, &f, &l);
-            tab[i] = cp->blockBitfield->countRange(f, l + 1) / (float)(l + 1 - f);
+            tab[i] = cp->blockBitfield->count(f, l + 1) / (float)(l + 1 - f);
         }
     }
 }
@@ -247,7 +247,7 @@ size_t tr_cpMissingBlocksInPiece(tr_completion const* cp, tr_piece_index_t piece
     auto f = tr_block_index_t{};
     auto l = tr_block_index_t{};
     tr_torGetPieceBlockRange(cp->tor, piece, &f, &l);
-    return (l + 1 - f) - cp->blockBitfield->countRange(f, l + 1);
+    return (l + 1 - f) - cp->blockBitfield->count(f, l + 1);
 }
 
 size_t tr_cpMissingBytesInPiece(tr_completion const* cp, tr_piece_index_t piece)
@@ -268,11 +268,11 @@ size_t tr_cpMissingBytesInPiece(tr_completion const* cp, tr_piece_index_t piece)
         /* nb: we don't pass the usual l+1 here to Bitfield::countRange().
            It's faster to handle the last block separately because its size
            needs to be checked separately. */
-        haveBytes = cp->blockBitfield->countRange(f, l);
+        haveBytes = cp->blockBitfield->count(f, l);
         haveBytes *= cp->tor->blockSize;
     }
 
-    if (cp->blockBitfield->readBit(l)) /* handle the last block */
+    if (cp->blockBitfield->test(l)) /* handle the last block */
     {
         haveBytes += tr_torBlockCountBytes(cp->tor, l);
     }
@@ -291,7 +291,7 @@ bool tr_cpFileIsComplete(tr_completion const* cp, tr_file_index_t i)
     auto f = tr_block_index_t{};
     auto l = tr_block_index_t{};
     tr_torGetFileBlockRange(cp->tor, i, &f, &l);
-    return cp->blockBitfield->countRange(f, l + 1) == (l + 1 - f);
+    return cp->blockBitfield->count(f, l + 1) == (l + 1 - f);
 }
 
 std::vector<uint8_t> tr_cpCreatePieceBitfield(tr_completion const* cp)
@@ -300,11 +300,11 @@ std::vector<uint8_t> tr_cpCreatePieceBitfield(tr_completion const* cp)
 
     auto const n = cp->tor->info.pieceCount;
 
-    Bitfield pieces(n);
+    auto pieces = tr_bitfield{ n };
 
     if (tr_cpHasAll(cp))
     {
-        pieces.setMode(Bitfield::OperationMode::All);
+        pieces.setHasAll();
     }
     else if (!tr_cpHasNone(cp))
     {
@@ -315,11 +315,11 @@ std::vector<uint8_t> tr_cpCreatePieceBitfield(tr_completion const* cp)
             flags[i] = tr_cpPieceIsComplete(cp, i);
         }
 
-        pieces = Bitfield(flags, n);
+        pieces.setFromBools(flags, n);
         tr_free(flags);
     }
 
-    return pieces.getRaw();
+    return pieces.raw();
 }
 
 double tr_cpPercentComplete(tr_completion const* cp)

libtransmission/completion.h

@@ -22,7 +22,7 @@ struct tr_completion
 
     // Changed to non-owning pointer temporarily till tr_completion becomes C++-constructible and destructible
     // TODO: remove * and own the value
-    Bitfield* blockBitfield;
+    tr_bitfield* blockBitfield;
 
     /* number of bytes we'll have when done downloading. [0..info.totalSize]
        DON'T access this directly; it's a lazy field.
@@ -50,7 +50,7 @@ struct tr_completion
 
 void tr_cpConstruct(tr_completion*, tr_torrent*);
 
-void tr_cpBlockInit(tr_completion* cp, Bitfield const& blocks);
+void tr_cpBlockInit(tr_completion* cp, tr_bitfield const& blocks);
 
 static inline void tr_cpDestruct(tr_completion* cp)
 {
@@ -115,7 +115,7 @@ void tr_cpBlockAdd(tr_completion* cp, tr_block_index_t i);
 
 static inline bool tr_cpBlockIsComplete(tr_completion const* cp, tr_block_index_t i)
 {
-    return cp->blockBitfield->readBit(i);
+    return cp->blockBitfield->test(i);
 }
 
 /***

libtransmission/peer-common.h

@@ -57,7 +57,7 @@ struct tr_peer_event
     PeerEventType eventType;
 
     uint32_t pieceIndex; /* for GOT_BLOCK, GOT_HAVE, CANCEL, ALLOWED, SUGGEST */
-    Bitfield* bitfield; /* for GOT_BITFIELD */
+    tr_bitfield* bitfield; /* for GOT_BITFIELD */
     uint32_t offset; /* for GOT_BLOCK */
     uint32_t length; /* for GOT_BLOCK + GOT_PIECE_DATA */
     int err; /* errno for GOT_ERROR */
@@ -103,8 +103,8 @@ public:
     /** how complete the peer's copy of the torrent is. [0.0...1.0] */
     float progress = 0.0f;
 
-    Bitfield blame;
+    tr_bitfield blame;
-    Bitfield have;
+    tr_bitfield have;
 
     /* the client name.
        For BitTorrent peers, this is the app name derived from the `v' string in LTEP's handshake dictionary */

libtransmission/peer-mgr.cc

@@ -418,7 +418,7 @@ static void replicationNew(tr_swarm* s)
         {
             auto const* const peer = static_cast<tr_peer const*>(tr_ptrArrayNth(&s->peers, peer_i));
 
-            if (peer->have.readBit(piece_i))
+            if (peer->have.test(piece_i))
             {
                 ++r;
             }
@@ -1189,7 +1189,7 @@ static void tr_incrReplicationOfPiece(tr_swarm* s, size_t const index)
 /**
  * Increases the replication count of pieces present in the bitfield
  */
-static void tr_incrReplicationFromBitfield(tr_swarm* s, Bitfield const* b)
+static void tr_incrReplicationFromBitfield(tr_swarm* s, tr_bitfield const* b)
 {
     TR_ASSERT(replicationExists(s));
 
@@ -1197,7 +1197,7 @@ static void tr_incrReplicationFromBitfield(tr_swarm* s, Bitfield const* b)
 
     for (size_t i = 0, n = s->tor->info.pieceCount; i < n; ++i)
     {
-        if (b->readBit(i))
+        if (b->test(i))
         {
             ++rep[i];
         }
@@ -1226,7 +1226,7 @@ static void tr_incrReplication(tr_swarm* s)
 /**
  * Decrease the replication count of pieces present in the bitset.
  */
-static void tr_decrReplicationFromBitfield(tr_swarm* s, Bitfield const* b)
+static void tr_decrReplicationFromBitfield(tr_swarm* s, tr_bitfield const* b)
 {
     TR_ASSERT(replicationExists(s));
     TR_ASSERT(s->pieceReplicationSize == s->tor->info.pieceCount);
@@ -1242,7 +1242,7 @@ static void tr_decrReplicationFromBitfield(tr_swarm* s, Bitfield const* b)
     {
         for (size_t i = 0; i < s->pieceReplicationSize; ++i)
         {
-            if (b->readBit(i))
+            if (b->test(i))
             {
                 --s->pieceReplication[i];
             }
@@ -1278,7 +1278,7 @@ void tr_peerMgrGetNextRequests(
     TR_ASSERT(tr_isTorrent(tor));
     TR_ASSERT(numwant > 0);
 
-    Bitfield const* const have = &peer->have;
+    tr_bitfield const* const have = &peer->have;
 
     /* walk through the pieces and find blocks that should be requested */
     tr_swarm* const s = tor->swarm;
@@ -1317,7 +1317,7 @@ void tr_peerMgrGetNextRequests(
         struct weighted_piece* p = pieces + i;
 
         /* if the peer has this piece that we want... */
-        if (have->readBit(p->index))
+        if (have->test(p->index))
         {
             auto first = tr_block_index_t{};
             auto last = tr_block_index_t{};
@@ -1650,7 +1650,7 @@ void tr_peerMgrPieceCompleted(tr_torrent* tor, tr_piece_index_t p)
 
         if (!pieceCameFromPeers)
         {
-            pieceCameFromPeers = peer->blame.readBit(p);
+            pieceCameFromPeers = peer->blame.test(p);
         }
     }
 
@@ -2200,7 +2200,7 @@ void tr_peerMgrGotBadPiece(tr_torrent* tor, tr_piece_index_t pieceIndex)
     {
         auto* const peer = static_cast<tr_peer*>(tr_ptrArrayNth(&s->peers, i));
 
-        if (peer->blame.readBit(pieceIndex))
+        if (peer->blame.test(pieceIndex))
         {
             tordbg(
                 s,
@@ -2470,7 +2470,7 @@ void tr_peerMgrRemoveTorrent(tr_torrent* tor)
 
 void tr_peerUpdateProgress(tr_torrent* tor, tr_peer* peer)
 {
-    Bitfield const* have = &peer->have;
+    auto const* have = &peer->have;
 
     if (have->hasAll())
     {
@@ -2482,7 +2482,7 @@ void tr_peerUpdateProgress(tr_torrent* tor, tr_peer* peer)
     }
     else
     {
-        float const true_count = have->countBits();
+        float const true_count = have->count();
 
         if (tr_torrentHasMetadata(tor))
         {
@@ -2490,20 +2490,12 @@ void tr_peerUpdateProgress(tr_torrent* tor, tr_peer* peer)
         }
         else /* without pieceCount, this result is only a best guess... */
         {
-            peer->progress = true_count / static_cast<float>(have->getBitCount() + 1);
+            peer->progress = true_count / static_cast<float>(have->size() + 1);
         }
     }
 
     /* clamp the progress range */
-    if (peer->progress < 0.0F)
+    peer->progress = std::clamp(peer->progress, 0.0F, 1.0F);
-    {
-        peer->progress = 0.0F;
-    }
-
-    if (peer->progress > 1.0F)
-    {
-        peer->progress = 1.0F;
-    }
 
     if (peer->atom != nullptr && peer->progress >= 1.0F)
     {
@@ -2562,7 +2554,7 @@ void tr_peerMgrTorrentAvailability(tr_torrent const* tor, int8_t* tab, unsigned
             {
                 for (int j = 0; j < peerCount; ++j)
                 {
-                    if (peers[j]->have.readBit(piece))
+                    if (peers[j]->have.test(piece))
                     {
                         ++tab[i];
                     }
@@ -2847,7 +2839,7 @@ static bool isPeerInteresting(tr_torrent* const tor, bool const* const piece_is_
 
     for (tr_piece_index_t i = 0; i < tor->info.pieceCount; ++i)
     {
-        if (piece_is_interesting[i] && peer->have.readBit(i))
+        if (piece_is_interesting[i] && peer->have.test(i))
         {
             return true;
         }

libtransmission/peer-msgs.cc

@@ -505,7 +505,7 @@ public:
         publish(e);
     }
 
-    void publishClientGotBitfield(Bitfield* bitfield)
+    void publishClientGotBitfield(tr_bitfield* bitfield)
     {
         auto e = tr_peer_event{};
         e.eventType = TR_PEER_CLIENT_GOT_BITFIELD;
@@ -1701,9 +1701,9 @@ static ReadState readBtMessage(tr_peerMsgsImpl* msgs, struct evbuffer* inbuf, si
         }
 
         /* a peer can send the same HAVE message twice... */
-        if (!msgs->have.readBit(ui32))
+        if (!msgs->have.test(ui32))
         {
-            msgs->have.setBit(ui32);
+            msgs->have.set(ui32);
             msgs->publishClientGotHave(ui32);
         }
 
@@ -1715,7 +1715,7 @@ static ReadState readBtMessage(tr_peerMsgsImpl* msgs, struct evbuffer* inbuf, si
             uint8_t* tmp = tr_new(uint8_t, msglen);
             dbgmsg(msgs, "got a bitfield");
             tr_peerIoReadBytes(msgs->io, inbuf, tmp, msglen);
-            msgs->have.setFrom(Span{ tmp, msglen }, tr_torrentHasMetadata(msgs->torrent));
+            msgs->have.setRaw(tmp, msglen, tr_torrentHasMetadata(msgs->torrent));
             msgs->publishClientGotBitfield(&msgs->have);
             updatePeerProgress(msgs);
             tr_free(tmp);
@@ -1809,8 +1809,7 @@ static ReadState readBtMessage(tr_peerMsgsImpl* msgs, struct evbuffer* inbuf, si
 
         if (fext)
         {
-            msgs->have.setMode(Bitfield::OperationMode::All);
+            msgs->have.setHasAll();
-            TR_ASSERT(msgs->have.hasAll());
             msgs->publishClientGotHaveAll();
             updatePeerProgress(msgs);
         }
@@ -1827,7 +1826,7 @@ static ReadState readBtMessage(tr_peerMsgsImpl* msgs, struct evbuffer* inbuf, si
 
         if (fext)
         {
-            msgs->have.setMode(Bitfield::OperationMode::None);
+            msgs->have.setHasNone();
             msgs->publishClientGotHaveNone();
             updatePeerProgress(msgs);
         }
@@ -1923,7 +1922,7 @@ static int clientGotBlock(tr_peerMsgsImpl* msgs, struct evbuffer* data, struct p
         return err;
     }
 
-    msgs->blame.setBit(req->index);
+    msgs->blame.set(req->index);
     msgs->publishGotBlock(req);
     return 0;
 }

libtransmission/resume.cc

@@ -466,7 +466,7 @@ static uint64_t loadFilenames(tr_variant* dict, tr_torrent* tor)
 ***/
 
 // TODO: Refactor this into a constructor for tr_variant
-static void bitfieldToBenc(Bitfield const* b, tr_variant* benc)
+static void bitfieldToBenc(tr_bitfield const* b, tr_variant* benc)
 {
     if (b->hasAll())
     {
@@ -478,7 +478,7 @@ static void bitfieldToBenc(Bitfield const* b, tr_variant* benc)
     }
     else
     {
-        auto const raw = b->getRaw();
+        auto const raw = b->raw();
         tr_variantInitRaw(benc, raw.data(), std::size(raw));
     }
 }
@@ -645,7 +645,7 @@ static uint64_t loadProgress(tr_variant* dict, tr_torrent* tor)
             }
         }
 
-        Bitfield blocks(tor->blockCount);
+        auto blocks = tr_bitfield{ tor->blockCount };
 
         auto rawlen = size_t{};
         char const* err = nullptr;
@@ -663,22 +663,22 @@ static uint64_t loadProgress(tr_variant* dict, tr_torrent* tor)
             }
             else if (buflen == 3 && memcmp(buf, "all", 3) == 0)
             {
-                blocks.setMode(Bitfield::OperationMode::All);
+                blocks.setHasAll();
             }
             else if (buflen == 4 && memcmp(buf, "none", 4) == 0)
             {
-                blocks.setMode(Bitfield::OperationMode::None);
+                blocks.setHasNone();
             }
             else
             {
-                blocks.setFrom(Span{ buf, buflen }, true);
+                blocks.setRaw(buf, buflen, true);
             }
         }
         else if (tr_variantDictFindStr(prog, TR_KEY_have, &str, nullptr))
         {
             if (strcmp(str, "all") == 0)
             {
-                blocks.setMode(Bitfield::OperationMode::All);
+                blocks.setHasAll();
             }
             else
             {
@@ -687,7 +687,7 @@ static uint64_t loadProgress(tr_variant* dict, tr_torrent* tor)
         }
         else if (tr_variantDictFindRaw(prog, TR_KEY_bitfield, &raw, &rawlen))
         {
-            blocks.setFrom(Span{ raw, rawlen }, true);
+            blocks.setRaw(raw, rawlen, true);
         }
         else
         {

libtransmission/session.cc

@@ -1473,7 +1473,7 @@ static auto constexpr MinutesPerWeek = int{ MinutesPerDay * 7 };
 
 static void turtleUpdateTable(struct tr_turtle_info* t)
 {
-    t->minutes->setMode(Bitfield::OperationMode::None);
+    t->minutes->setHasNone();
 
     for (int day = 0; day < 7; ++day)
     {
@@ -1489,7 +1489,7 @@ static void turtleUpdateTable(struct tr_turtle_info* t)
 
             for (time_t i = begin; i < end; ++i)
             {
-                t->minutes->setBit((i + day * MinutesPerDay) % MinutesPerWeek);
+                t->minutes->set((i + day * MinutesPerDay) % MinutesPerWeek);
             }
         }
     }
@@ -1541,7 +1541,7 @@ static bool getInTurtleTime(struct tr_turtle_info const* t)
         minute_of_the_week = MinutesPerWeek - 1;
     }
 
-    return t->minutes->readBit(minute_of_the_week);
+    return t->minutes->test(minute_of_the_week);
 }
 
 static constexpr tr_auto_switch_state_t autoSwitchState(bool enabled)
@@ -1573,7 +1573,7 @@ static void turtleBootstrap(tr_session* session, struct tr_turtle_info* turtle)
     turtle->changedByUser = false;
     turtle->autoTurtleState = TR_AUTO_SWITCH_UNUSED;
 
-    turtle->minutes = new Bitfield(MinutesPerWeek);
+    turtle->minutes = new tr_bitfield(MinutesPerWeek);
 
     turtleUpdateTable(turtle);
 

libtransmission/session.h

@@ -94,7 +94,7 @@ struct tr_turtle_info
      * limits on or off at that given minute in the week. */
     // Changed to non-owning pointer temporarily till tr_turtle_info becomes C++-constructible and destructible
     // TODO: remove * and own the value
-    Bitfield* minutes;
+    tr_bitfield* minutes = nullptr;
 
     /* recent action that was done by turtle's automatic switch */
     tr_auto_switch_state_t autoTurtleState;

libtransmission/span.h

@@ -1,45 +0,0 @@
-/*
-* This file Copyright (C) Mnemosyne LLC
-*
-* It may be used under the GNU GPL versions 2 or 3
-* or any future license endorsed by Mnemosyne LLC.
-*
-*/
-
-#pragma once
-
-#ifndef __TRANSMISSION__
-#error only libtransmission should #include this header.
-#endif
-
-/// @brief Readonly non-owning view into a provided memory block with start pointer and size.
-/// In C++20 this appears in standard library as std::span and remotely similar usage.
-template<typename T>
-class Span
-{
-public:
-    Span(T const* ptr, size_t size)
-        : ptr_{ ptr }
-        , size_{ size }
-    {
-    }
-
-    T const* begin() const
-    {
-        return this->ptr_;
-    }
-
-    T const* end() const
-    {
-        return this->ptr_ + this->size_;
-    }
-
-    [[nodiscard]] size_t size() const
-    {
-        return size_;
-    }
-
-private:
-    T const* ptr_;
-    size_t size_;
-};

libtransmission/torrent.cc

@@ -1454,7 +1454,7 @@ static uint64_t countFileBytesCompleted(tr_torrent const* tor, tr_file_index_t i
     // the middle blocks
     if (first + 1 < last)
     {
-        uint64_t u = tor->completion.blockBitfield->countRange(first + 1, last);
+        uint64_t u = tor->completion.blockBitfield->count(first + 1, last);
         u *= tor->blockSize;
         total += u;
     }

libtransmission/webseed.cc

@@ -67,7 +67,7 @@ public:
         , bandwidth(tor->bandwidth)
     {
         // init parent bits
-        have.setMode(Bitfield::OperationMode::All);
+        have.setHasAll();
         tr_peerUpdateProgress(tor, this);
 
         file_urls.resize(tr_torrentInfo(tor)->fileCount);

tests/libtransmission/bitfield-test.cc

@@ -24,11 +24,11 @@ TEST(Bitfield, countRange)
         int const bit_count = 100 + tr_rand_int_weak(1000);
 
         // generate a random bitfield
-        Bitfield bf(bit_count);
+        tr_bitfield bf(bit_count);
 
         for (int j = 0, n = tr_rand_int_weak(bit_count); j < n; ++j)
         {
-            bf.setBit(tr_rand_int_weak(bit_count));
+            bf.set(tr_rand_int_weak(bit_count));
         }
 
         int begin = tr_rand_int_weak(bit_count);
@@ -50,13 +50,13 @@ TEST(Bitfield, countRange)
         unsigned long count1 = {};
         for (auto j = begin; j < end; ++j)
         {
-            if (bf.readBit(j))
+            if (bf.test(j))
             {
                 ++count1;
             }
         }
 
-        auto const count2 = bf.countRange(begin, end);
+        auto const count2 = bf.count(begin, end);
         EXPECT_EQ(count1, count2);
     }
 }
@@ -76,163 +76,190 @@ TEST(Bitfield, ctorFromFlagArray)
         bool const have_all = true_count == n;
         bool const have_none = true_count == 0;
 
-        auto const bf = Bitfield(std::data(flags), std::size(flags));
+        auto bf = tr_bitfield(n);
+        bf.setFromBools(std::data(flags), std::size(flags));
 
-        EXPECT_EQ(n, bf.getBitCount());
+        EXPECT_EQ(n, bf.size());
         EXPECT_EQ(have_all, bf.hasAll());
         EXPECT_EQ(have_none, bf.hasNone());
-        EXPECT_EQ(true_count, bf.countBits());
+        EXPECT_EQ(true_count, bf.count());
 
         for (size_t i = 0; i < std::size(flags); ++i)
         {
-            EXPECT_EQ(flags[i], bf.readBit(i));
+            EXPECT_EQ(flags[i], bf.test(i));
         }
     }
 }
 
+TEST(Bitfield, setRaw)
+{
+    auto constexpr TestByte = uint8_t{ 10 };
+    auto constexpr TestByteTrueBits = 2;
+
+    auto const raw = std::vector<uint8_t>(100, TestByte);
+
+    auto bf = tr_bitfield(std::size(raw) * 8);
+    bf.setRaw(std::data(raw), std::size(raw), true);
+    EXPECT_EQ(TestByteTrueBits * std::size(raw), bf.count());
+
+    // The first byte of the bitfield corresponds to indices 0 - 7
+    // from high bit to low bit, respectively. The next one 8-15, etc.
+    // Spare bits at the end are set to zero.
+    auto test = uint8_t{};
+    for (int i = 0; i < 8; ++i)
+    {
+        if (bf.test(i))
+        {
+            test |= (1 << (7 - i));
+        }
+    }
+    EXPECT_EQ(TestByte, test);
+    EXPECT_EQ(raw, bf.raw());
+}
+
 TEST(Bitfield, bitfields)
 {
     unsigned int bitcount = 500;
-    Bitfield field(bitcount);
+    tr_bitfield field(bitcount);
 
-    /* test Bitfield::setBit */
+    // test tr_bitfield::set()
     for (unsigned int i = 0; i < bitcount; i++)
     {
         if (i % 7 == 0)
         {
-            field.setBit(i);
+            field.set(i);
         }
     }
 
     for (unsigned int i = 0; i < bitcount; i++)
     {
-        EXPECT_EQ(field.readBit(i), (i % 7 == 0));
+        EXPECT_EQ(field.test(i), (i % 7 == 0));
     }
 
-    /* test Bitfield::setBitRange */
+    /* test tr_bitfield::setRange */
-    field.setBitRange(0, bitcount);
+    field.setRange(0, bitcount);
 
     for (unsigned int i = 0; i < bitcount; i++)
     {
-        EXPECT_TRUE(field.readBit(i));
+        EXPECT_TRUE(field.test(i));
     }
 
-    /* test Bitfield::clearBit */
+    /* test tr_bitfield::clearBit */
     for (unsigned int i = 0; i < bitcount; i++)
     {
         if (i % 7 != 0)
         {
-            field.clearBit(i);
+            field.unset(i);
         }
     }
 
     for (unsigned int i = 0; i < bitcount; i++)
     {
-        EXPECT_EQ(field.readBit(i), (i % 7 == 0));
+        EXPECT_EQ(field.test(i), (i % 7 == 0));
     }
 
-    /* test Bitfield::clearBitRange in the middle of a boundary */
+    /* test tr_bitfield::clearBitRange in the middle of a boundary */
-    field.setBitRange(0, 64);
+    field.setRange(0, 64);
-    field.clearBitRange(4, 21);
+    field.unsetRange(4, 21);
 
     for (unsigned int i = 0; i < 64; i++)
     {
-        EXPECT_EQ(field.readBit(i), (i < 4 || i >= 21));
+        EXPECT_EQ(field.test(i), (i < 4 || i >= 21));
     }
 
-    /* test Bitfield::clearBitRange on the boundaries */
+    /* test tr_bitfield::clearBitRange on the boundaries */
-    field.setBitRange(0, 64);
+    field.setRange(0, 64);
-    field.clearBitRange(8, 24);
+    field.unsetRange(8, 24);
 
     for (unsigned int i = 0; i < 64; i++)
     {
-        EXPECT_EQ(field.readBit(i), (i < 8 || i >= 24));
+        EXPECT_EQ(field.test(i), (i < 8 || i >= 24));
     }
 
-    /* test Bitfield::clearBitRange when begin & end is on the same word */
+    /* test tr_bitfield::clearBitRange when begin & end is on the same word */
-    field.setBitRange(0, 64);
+    field.setRange(0, 64);
-    field.clearBitRange(4, 5);
+    field.unsetRange(4, 5);
 
     for (unsigned int i = 0; i < 64; i++)
     {
-        EXPECT_EQ(field.readBit(i), (i < 4 || i >= 5));
+        EXPECT_EQ(field.test(i), (i < 4 || i >= 5));
     }
 
-    /* test Bitfield::setBitRange */
+    /* test tr_bitfield::setRange */
-    field.clearBitRange(0, 64);
+    field.unsetRange(0, 64);
-    field.setBitRange(4, 21);
+    field.setRange(4, 21);
 
     for (unsigned int i = 0; i < 64; i++)
     {
-        EXPECT_EQ(field.readBit(i), (4 <= i && i < 21));
+        EXPECT_EQ(field.test(i), (4 <= i && i < 21));
     }
 
-    /* test Bitfield::setBitRange on the boundaries */
+    /* test tr_bitfield::setRange on the boundaries */
-    field.clearBitRange(0, 64);
+    field.unsetRange(0, 64);
-    field.setBitRange(8, 24);
+    field.setRange(8, 24);
 
     for (unsigned int i = 0; i < 64; i++)
     {
-        EXPECT_EQ(field.readBit(i), (8 <= i && i < 24));
+        EXPECT_EQ(field.test(i), (8 <= i && i < 24));
     }
 
-    /* test Bitfield::setBitRange when begin & end is on the same word */
+    /* test tr_bitfield::setRange when begin & end is on the same word */
-    field.clearBitRange(0, 64);
+    field.unsetRange(0, 64);
-    field.setBitRange(4, 5);
+    field.setRange(4, 5);
 
     for (unsigned int i = 0; i < 64; i++)
     {
-        EXPECT_EQ(field.readBit(i), (4 <= i && i < 5));
+        EXPECT_EQ(field.test(i), (4 <= i && i < 5));
     }
 }
 
 TEST(Bitfield, hasAllNone)
 {
     {
-        Bitfield field(3);
+        tr_bitfield field(3);
 
         EXPECT_TRUE(!field.hasAll());
         EXPECT_TRUE(field.hasNone());
 
-        field.setBit(0);
+        field.set(0);
         EXPECT_TRUE(!field.hasAll());
         EXPECT_TRUE(!field.hasNone());
 
-        field.clearBit(0);
+        field.unset(0);
-        field.setBit(1);
+        field.set(1);
         EXPECT_TRUE(!field.hasAll());
         EXPECT_TRUE(!field.hasNone());
 
-        field.clearBit(1);
+        field.unset(1);
-        field.setBit(2);
+        field.set(2);
         EXPECT_TRUE(!field.hasAll());
         EXPECT_TRUE(!field.hasNone());
 
-        field.setBit(0);
+        field.set(0);
-        field.setBit(1);
+        field.set(1);
         EXPECT_TRUE(field.hasAll());
         EXPECT_TRUE(!field.hasNone());
 
-        field.setMode(Bitfield::OperationMode::None);
+        field.setHasNone();
         EXPECT_TRUE(!field.hasAll());
         EXPECT_TRUE(field.hasNone());
 
-        field.setMode(Bitfield::OperationMode::All);
+        field.setHasAll();
         EXPECT_TRUE(field.hasAll());
         EXPECT_TRUE(!field.hasNone());
     }
 
     {
-        Bitfield field(0);
+        tr_bitfield field(0);
 
         EXPECT_TRUE(!field.hasAll());
         EXPECT_TRUE(!field.hasNone());
 
-        field.setMode(Bitfield::OperationMode::None);
+        field.setHasNone();
         EXPECT_TRUE(!field.hasAll());
         EXPECT_TRUE(field.hasNone());
 
-        field.setMode(Bitfield::OperationMode::All);
+        field.setHasAll();
         EXPECT_TRUE(field.hasAll());
         EXPECT_TRUE(!field.hasNone());
     }