Skip to content

Replace custom CRing circular buffer — fix in place, std::deque, or boost::circular_buffer

Classification

Abandoned by operator decision on 2026-05-21. This historical record preserves the old Boost/POCO analysis only as provenance; do not promote it without a new active item.

BUG-007 has already fixed the Beta 0.7.3 correctness defects in place on main; this deferred item is only about a possible future cleanup that would delete the custom ring implementation.

Summary

srchybrid/ring.h contains a 100-line custom CRing<T> template with manual pointer arithmetic over a heap-allocated array (m_pHead, m_pTail, m_pEnd). BUG-007 documents three UB and correctness bugs in this implementation.

Three options are documented: fix the bugs in place (lowest risk), replace with std::deque (no dependency), or replace with boost::circular_buffer (if Boost is adopted).

Current Code (ring.h:24-50)

template<class TYPE> class CRing
{
    UINT_PTR  m_nCount;
    UINT_PTR  m_nIncrement;
    UINT_PTR  m_nSize;
    TYPE     *m_pData;   // raw heap array
    TYPE     *m_pEnd;    // one-past-end
    TYPE     *m_pHead;   // read pointer
    TYPE     *m_pTail;   // write pointer — BUGS here (see BUG-007)
};

Consumers

// UploadQueue.cpp:64
: average_ur_hist(512, 512)
, activeClients_hist(512, 512)

Option A — Fix CRing in place (lowest risk)

No new dependency. Minimal diff. Described in detail in BUG-007.

This option is complete on main through BUG-007. The accepted empty-ring contract keeps m_pHead == m_pData and m_pTail == m_pEnd until the next insert; it does not use a before-the-buffer sentinel such as dst - 1 or m_pData - 1.

Pros: Minimal change; no dependency; preserves existing API exactly.
Cons: Still a custom 100-line template that has to be maintained; misses the opportunity to delete the code entirely.

Option B — Replace with std::deque wrapper (no Boost dependency)

Zero new dependencies. Cleaner than Option A.

#include <deque>

template<class TYPE>
class CRing {
    std::deque<TYPE> m_buf;
    size_t           m_capacity;
public:
    explicit CRing(size_t capacity, size_t /*ignored increment*/ = 0)
        : m_capacity(capacity) {}

    void AddTail(const TYPE& v) {
        if (m_buf.size() == m_capacity)
            m_buf.pop_front();   // overwrite oldest — same as CRing behaviour
        m_buf.push_back(v);
    }
    void RemoveAll()               { m_buf.clear(); }
    void RemoveHead()              { if (!m_buf.empty()) m_buf.pop_front(); }
    size_t GetSize() const         { return m_buf.size(); }
    bool   IsEmpty() const         { return m_buf.empty(); }
    const TYPE& operator[](size_t i) const {
        ASSERT(i < m_buf.size());
        return m_buf[i];
    }
    // Forward iterators:
    auto begin() { return m_buf.begin(); }
    auto end()   { return m_buf.end(); }
};

Drop-in replacement — existing call sites (average_ur_hist.AddTail(...), activeClients_hist[i]) compile unchanged.

Pros: Deletes the custom pointer arithmetic entirely; std::deque is well-tested; iterators work with <algorithm>.
Cons: std::deque has higher per-element overhead than a fixed array (more cache misses for large N). For the current use case (N=512), this is irrelevant.

Option C — boost::circular_buffer (requires Boost)

Use only if Boost is already adopted for REF-008.

#include <boost/circular_buffer.hpp>

// Replace CRing<TransferredData> with 512-element ring
boost::circular_buffer<TransferredData> average_ur_hist(512);
boost::circular_buffer<uint32_t>        activeClients_hist(512);

// Usage identical to std::deque
average_ur_hist.push_back(newSample);
if (!average_ur_hist.empty())
    auto oldest = average_ur_hist.front();

Pros: Fixed-capacity ring with O(1) push/pop; overwrites oldest automatically when full; compatible with std::accumulate, std::copy.
Cons: Requires Boost; functionally equivalent to Option B for this use case.

Recommendation

  • BUG-007 is already closed through Option A; do not reopen it through this deferred idea.
  • If future cleanup is promoted to active work, use Option B (std::deque wrapper) — deletes the custom code with zero new dependency.
  • If Boost is adopted: Option C is marginally cleaner than B but not worth adding Boost just for this.

Files

  • srchybrid/ring.h — fix or delete
  • srchybrid/ring.cpp (if it exists) — delete if Option B or C chosen
  • srchybrid/UploadQueue.cpp — primary consumer

Acceptance Criteria

  • [x] BUG-007 UB bugs fixed regardless of which option is chosen
  • [x] operator[] with out-of-bounds index asserts/throws rather than reading garbage
  • [x] RemoveAll() leaves the ring in a consistent empty state
  • [x] UploadQueue.cpp ring usage compiles and passes all tests
  • [ ] If Option B/C chosen: ring.h deleted from the tree