From e31487f84caccbbfc986d8c6327c278ff1548c7d Mon Sep 17 00:00:00 2001 From: Wyatt Hepler Date: Sat, 21 Sep 2024 05:29:17 +0000 Subject: [PATCH] pw_multibuf: Remove MultiBuf's Chunk iterator wrappers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead, use the iterators in MultiBufChunks directly. This avoids having two different iterator types in one class. Change-Id: I51c9e05558cdfbc7312702a3d23ca3b023a74018 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/230893 Docs-Not-Needed: Wyatt Hepler Commit-Queue: Auto-Submit Pigweed-Auto-Submit: Wyatt Hepler Reviewed-by: Taylor Cramer Lint: Lint 🤖 --- pw_channel/epoll_channel.cc | 2 +- pw_multibuf/multibuf.cc | 11 ++++++----- pw_multibuf/multibuf_test.cc | 4 ++-- pw_multibuf/public/pw_multibuf/multibuf.h | 22 ++++------------------ 4 files changed, 13 insertions(+), 26 deletions(-) diff --git a/pw_channel/epoll_channel.cc b/pw_channel/epoll_channel.cc index db7f13f33d..59fce367de 100644 --- a/pw_channel/epoll_channel.cc +++ b/pw_channel/epoll_channel.cc @@ -61,7 +61,7 @@ async2::Poll> EpollChannel::DoPendRead( } multibuf::MultiBuf buf = std::move(**maybe_multibuf); - multibuf::Chunk& chunk = *buf.ChunkBegin(); + multibuf::Chunk& chunk = *buf.Chunks().begin(); int bytes_read = read(channel_fd_, chunk.data(), chunk.size()); if (bytes_read >= 0) { diff --git a/pw_multibuf/multibuf.cc b/pw_multibuf/multibuf.cc index abbdc5b43c..dbba233a00 100644 --- a/pw_multibuf/multibuf.cc +++ b/pw_multibuf/multibuf.cc @@ -78,8 +78,8 @@ bool MultiBuf::ClaimSuffix(size_t bytes_to_claim) { void MultiBuf::DiscardPrefix(size_t bytes_to_discard) { PW_DCHECK(bytes_to_discard <= size()); while (bytes_to_discard != 0) { - if (ChunkBegin()->size() > bytes_to_discard) { - ChunkBegin()->DiscardPrefix(bytes_to_discard); + if (Chunks().begin()->size() > bytes_to_discard) { + Chunks().begin()->DiscardPrefix(bytes_to_discard); return; } OwnedChunk front_chunk = TakeFrontChunk(); @@ -127,7 +127,7 @@ void MultiBufChunks::PushSuffix(MultiBufChunks&& tail) { StatusWithSize MultiBuf::CopyTo(ByteSpan dest, const size_t position) const { const_iterator byte_in_chunk = begin() + position; - ConstChunkIterator chunk(byte_in_chunk.chunk()); + MultiBufChunks::const_iterator chunk(byte_in_chunk.chunk()); size_t chunk_offset = byte_in_chunk.byte_index(); size_t bytes_copied = 0; @@ -163,7 +163,8 @@ StatusWithSize MultiBuf::CopyFromAndOptionallyTruncate(ConstByteSpan source, size_t chunk_offset = byte_in_chunk.byte_index(); size_t bytes_copied = 0; - for (ChunkIterator chunk(byte_in_chunk.chunk()); chunk != Chunks().end(); + for (MultiBufChunks::iterator chunk(byte_in_chunk.chunk()); + chunk != Chunks().end(); ++chunk) { if (chunk->empty()) { continue; @@ -196,7 +197,7 @@ std::optional MultiBuf::TakePrefix(size_t bytes_to_take) { } // Pointer to the last element of `front`, allowing constant-time appending. Chunk* last_front_chunk = nullptr; - while (bytes_to_take > ChunkBegin()->size()) { + while (bytes_to_take > Chunks().begin()->size()) { OwnedChunk new_chunk = TakeFrontChunk().Take(); Chunk* new_chunk_ptr = &*new_chunk; bytes_to_take -= new_chunk.size(); diff --git a/pw_multibuf/multibuf_test.cc b/pw_multibuf/multibuf_test.cc index 38ed2d21d0..b196ab3934 100644 --- a/pw_multibuf/multibuf_test.cc +++ b/pw_multibuf/multibuf_test.cc @@ -29,8 +29,8 @@ using namespace pw::multibuf::test_utils; #if __cplusplus >= 202002L static_assert(std::forward_iterator); static_assert(std::forward_iterator); -static_assert(std::forward_iterator); -static_assert(std::forward_iterator); +static_assert(std::forward_iterator); +static_assert(std::forward_iterator); #endif // __cplusplus >= 202002L static_assert( diff --git a/pw_multibuf/public/pw_multibuf/multibuf.h b/pw_multibuf/public/pw_multibuf/multibuf.h index ac09037a90..55133ab084 100644 --- a/pw_multibuf/public/pw_multibuf/multibuf.h +++ b/pw_multibuf/public/pw_multibuf/multibuf.h @@ -180,7 +180,7 @@ class MultiBufChunks { // // Implementation note: `Chunks().size()` should be remain relatively small, // but this could be made `O(1)` in the future by adding a `prev` pointer to - // the `ChunkIterator`. + // the `iterator`. iterator insert(iterator position, OwnedChunk&& chunk); /// Removes and returns `Chunk` from the specified position. @@ -192,7 +192,7 @@ class MultiBufChunks { // // Implementation note: `Chunks().size()` should be remain relatively small, // but this could be made `O(1)` in the future by adding a `prev` pointer to - // the `ChunkIterator`. + // the `iterator`. std::tuple take(iterator position); protected: @@ -247,9 +247,6 @@ class MultiBuf : private MultiBufChunks { class iterator; class const_iterator; - using ChunkIterator = MultiBufChunks::iterator; - using ConstChunkIterator = MultiBufChunks::const_iterator; - constexpr MultiBuf() : MultiBufChunks(nullptr) {} static MultiBuf FromChunk(OwnedChunk&& chunk) { @@ -529,19 +526,8 @@ class MultiBuf : private MultiBufChunks { /// Returns a `const Chunk`-oriented view of this `MultiBuf`. constexpr const MultiBufChunks& Chunks() const { return *this; } - /// Returns an iterator pointing to the first `Chunk` in this `MultiBuf`. - constexpr ChunkIterator ChunkBegin() { return Chunks().begin(); } - /// Returns an iterator pointing to the end of the `Chunk`s in this - /// `MultiBuf`. - constexpr ChunkIterator ChunkEnd() { return Chunks().end(); } - /// Returns a const iterator pointing to the first `Chunk` in this - /// `MultiBuf`. - constexpr ConstChunkIterator ConstChunkBegin() const { - return Chunks().begin(); - } - /// Returns a const iterator pointing to the end of the `Chunk`s in this - /// `MultiBuf`. - constexpr ConstChunkIterator ConstChunkEnd() const { return Chunks().end(); } + /// Returns a `const Chunk`-oriented view of this `MultiBuf`. + constexpr const MultiBufChunks& ConstChunks() const { return *this; } /////////////////////////////////////////////////////////////////// //--------------------- Iterator details ------------------------//