Skip to content

Commit

Permalink
pw_multibuf: Remove MultiBuf's Chunk iterator wrappers
Browse files Browse the repository at this point in the history
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 <hepler@google.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed-service-accounts.iam.gserviceaccount.com>
Pigweed-Auto-Submit: Wyatt Hepler <hepler@google.com>
Reviewed-by: Taylor Cramer <cramertj@google.com>
Lint: Lint 🤖 <android-build-ayeaye@system.gserviceaccount.com>
  • Loading branch information
255 authored and CQ Bot Account committed Sep 21, 2024
1 parent 6b20a02 commit e31487f
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 26 deletions.
2 changes: 1 addition & 1 deletion pw_channel/epoll_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ async2::Poll<Result<multibuf::MultiBuf>> 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) {
Expand Down
11 changes: 6 additions & 5 deletions pw_multibuf/multibuf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -196,7 +197,7 @@ std::optional<MultiBuf> 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();
Expand Down
4 changes: 2 additions & 2 deletions pw_multibuf/multibuf_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ using namespace pw::multibuf::test_utils;
#if __cplusplus >= 202002L
static_assert(std::forward_iterator<MultiBuf::iterator>);
static_assert(std::forward_iterator<MultiBuf::const_iterator>);
static_assert(std::forward_iterator<MultiBuf::ChunkIterator>);
static_assert(std::forward_iterator<MultiBuf::ConstChunkIterator>);
static_assert(std::forward_iterator<MultiBufChunks::iterator>);
static_assert(std::forward_iterator<MultiBufChunks::const_iterator>);
#endif // __cplusplus >= 202002L

static_assert(
Expand Down
22 changes: 4 additions & 18 deletions pw_multibuf/public/pw_multibuf/multibuf.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<iterator, OwnedChunk> take(iterator position);

protected:
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 ------------------------//
Expand Down

0 comments on commit e31487f

Please sign in to comment.