From b9698210ef4b243f6c1d31227120fd771f0bdd9a Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 16 May 2023 21:46:41 -0500 Subject: [PATCH] refactor: restore part of Buffer::reserve_space() (#5529) --- libtransmission/peer-msgs.cc | 6 +- libtransmission/tr-buffer.h | 210 ++++++++++++++------------- tests/libtransmission/buffer-test.cc | 37 ++++- 3 files changed, 150 insertions(+), 103 deletions(-) diff --git a/libtransmission/peer-msgs.cc b/libtransmission/peer-msgs.cc index b1d7c35d1..781ac8d8a 100644 --- a/libtransmission/peer-msgs.cc +++ b/libtransmission/peer-msgs.cc @@ -1039,7 +1039,7 @@ void parseLtepHandshake(tr_peerMsgsImpl* msgs, libtransmission::Buffer& payload) { msgs->peerSentLtepHandshake = true; - auto const handshake_sv = payload.pullup_sv(); + auto const handshake_sv = payload.to_string_view(); auto val = tr_variant{}; if (!tr_variantFromBuf(&val, TR_VARIANT_PARSE_BENC | TR_VARIANT_PARSE_INPLACE, handshake_sv) || !tr_variantIsDict(&val)) @@ -1151,7 +1151,7 @@ void parseUtMetadata(tr_peerMsgsImpl* msgs, libtransmission::Buffer& payload_in) int64_t piece = -1; int64_t total_size = 0; - auto const tmp = payload_in.pullup_sv(); + auto const tmp = payload_in.to_string_view(); auto const* const msg_end = std::data(tmp) + std::size(tmp); auto dict = tr_variant{}; @@ -1208,7 +1208,7 @@ void parseUtPex(tr_peerMsgsImpl* msgs, libtransmission::Buffer& payload) return; } - auto const tmp = payload.pullup_sv(); + auto const tmp = payload.to_string_view(); if (tr_variant val; tr_variantFromBuf(&val, TR_VARIANT_PARSE_BENC | TR_VARIANT_PARSE_INPLACE, tmp)) { diff --git a/libtransmission/tr-buffer.h b/libtransmission/tr-buffer.h index e8c1d4577..24e84c489 100644 --- a/libtransmission/tr-buffer.h +++ b/libtransmission/tr-buffer.h @@ -5,10 +5,12 @@ #pragma once +#include // for std::copy_n #include #include #include #include +#include #include #include @@ -23,21 +25,79 @@ namespace libtransmission { -template +template +class BufferReader +{ +public: + virtual ~BufferReader() = default; + virtual void drain(size_t n_bytes) = 0; + [[nodiscard]] virtual size_t size() const noexcept = 0; + [[nodiscard]] virtual value_type const* data() const = 0; + + [[nodiscard]] auto empty() const noexcept + { + return size() == 0; + } + + [[nodiscard]] auto to_string_view() const + { + return std::string_view{ reinterpret_cast(data()), size() }; + } + + [[nodiscard]] auto to_string() const + { + return std::string{ to_string_view() }; + } + + void to_buf(void* tgt, size_t n_bytes) + { + n_bytes = std::min(n_bytes, size()); + std::copy_n(data(), n_bytes, static_cast(tgt)); + drain(n_bytes); + } + + [[nodiscard]] auto to_uint8() + { + auto tmp = uint8_t{}; + to_buf(&tmp, sizeof(tmp)); + return tmp; + } + + [[nodiscard]] uint16_t to_uint16() + { + auto tmp = uint16_t{}; + to_buf(&tmp, sizeof(tmp)); + return ntohs(tmp); + } + + [[nodiscard]] uint32_t to_uint32() + { + auto tmp = uint32_t{}; + to_buf(&tmp, sizeof(tmp)); + return ntohl(tmp); + } + + [[nodiscard]] uint64_t to_uint64() + { + auto tmp = uint64_t{}; + to_buf(&tmp, sizeof(tmp)); + return tr_ntohll(tmp); + } +}; + +template class BufferWriter { public: - BufferWriter(T* out) - : out_{ out } - { - static_assert(sizeof(ValueType) == 1); - } + virtual ~BufferWriter() = default; + virtual std::pair reserve_space(size_t n_bytes) = 0; + virtual void commit_space(size_t n_bytes) = 0; void add(void const* span_begin, size_t span_len) { - auto const* const begin = reinterpret_cast(span_begin); - auto const* const end = begin + span_len; - out_->insert(std::end(*out_), begin, end); + auto [buf, buflen] = reserve_space(span_len); + std::copy_n(reinterpret_cast(span_begin), span_len, buf); + commit_space(span_len); } template @@ -90,19 +150,20 @@ public: add_uint64(hll); } - void add_port(tr_port const& port) + void add_port(tr_port port) { auto nport = port.network(); add(&nport, sizeof(nport)); } - -private: - T* out_; }; -class Buffer : public BufferWriter +class Buffer final + : public BufferReader + , public BufferWriter { public: + using value_type = std::byte; + class Iterator { public: @@ -240,43 +301,57 @@ public: size_t buf_offset_ = 0; }; - Buffer() - : BufferWriter{ this } - { - } - - Buffer(Buffer&& that) - : BufferWriter(this) - , buf_{ std::move(that.buf_) } - { - } - - Buffer& operator=(Buffer&& that) - { - buf_ = std::move(that.buf_); - return *this; - } - + Buffer() = default; + Buffer(Buffer&&) = default; Buffer(Buffer const&) = delete; + Buffer& operator=(Buffer&&) = default; Buffer& operator=(Buffer const&) = delete; template explicit Buffer(T const& data) - : BufferWriter{ this } { add(data); } - [[nodiscard]] auto size() const noexcept + // -- BufferReader + + [[nodiscard]] size_t size() const noexcept override { return evbuffer_get_length(buf_.get()); } - [[nodiscard]] auto empty() const noexcept + [[nodiscard]] std::byte const* data() const override { - return evbuffer_get_length(buf_.get()) == 0; + return reinterpret_cast(evbuffer_pullup(buf_.get(), -1)); } + void drain(size_t n_bytes) override + { + evbuffer_drain(buf_.get(), n_bytes); + } + + // -- BufferWriter + + [[nodiscard]] std::pair reserve_space(size_t n_bytes) override + { + auto iov = evbuffer_iovec{}; + evbuffer_reserve_space(buf_.get(), n_bytes, &iov, 1); + TR_ASSERT(iov.iov_len >= n_bytes); + reserved_space_ = iov; + return { static_cast(iov.iov_base), static_cast(iov.iov_len) }; + } + + void commit_space(size_t n_bytes) override + { + TR_ASSERT(reserved_space_); + TR_ASSERT(reserved_space_->iov_len >= n_bytes); + reserved_space_->iov_len = n_bytes; + evbuffer_commit_space(buf_.get(), &*reserved_space_, 1); + reserved_space_.reset(); + } + + // + [[nodiscard]] auto begin() noexcept { return Iterator{ buf_.get(), 0U }; @@ -306,52 +381,6 @@ public: return n_bytes <= size() && std::equal(needle_begin, needle_end, cbegin()); } - [[nodiscard]] std::string to_string() const - { - auto str = std::string{}; - str.resize(size()); - evbuffer_copyout(buf_.get(), std::data(str), std::size(str)); - return str; - } - - auto to_buf(void* tgt, size_t n_bytes) - { - return evbuffer_remove(buf_.get(), tgt, n_bytes); - } - - [[nodiscard]] auto to_uint8() - { - auto tmp = uint8_t{}; - to_buf(&tmp, sizeof(tmp)); - return tmp; - } - - [[nodiscard]] uint16_t to_uint16() - { - auto tmp = uint16_t{}; - to_buf(&tmp, sizeof(tmp)); - return ntohs(tmp); - } - - [[nodiscard]] uint32_t to_uint32() - { - auto tmp = uint32_t{}; - to_buf(&tmp, sizeof(tmp)); - return ntohl(tmp); - } - - [[nodiscard]] uint64_t to_uint64() - { - auto tmp = uint64_t{}; - to_buf(&tmp, sizeof(tmp)); - return tr_ntohll(tmp); - } - - void drain(size_t n_bytes) - { - evbuffer_drain(buf_.get(), n_bytes); - } - void clear() { drain(size()); @@ -376,17 +405,6 @@ public: return { reinterpret_cast(evbuffer_pullup(buf_.get(), -1)), size() }; } - [[nodiscard]] std::byte const* data() const - { - return reinterpret_cast(evbuffer_pullup(buf_.get(), -1)); - } - - [[nodiscard]] auto pullup_sv() - { - auto const [buf, buflen] = pullup(); - return std::string_view{ reinterpret_cast(buf), buflen }; - } - void reserve(size_t n_bytes) { evbuffer_expand(buf_.get(), n_bytes - size()); @@ -415,15 +433,9 @@ public: return {}; } - template - void insert([[maybe_unused]] Iterator iter, T const* const begin, T const* const end) - { - TR_ASSERT(iter == this->end()); // tr_buffer only supports appending - evbuffer_add(buf_.get(), begin, end - begin); - } - private: evhelpers::evbuffer_unique_ptr buf_{ evbuffer_new() }; + std::optional reserved_space_; [[nodiscard]] Iterator cbegin() const noexcept { diff --git a/tests/libtransmission/buffer-test.cc b/tests/libtransmission/buffer-test.cc index 3e3576bc3..1d9d0f5db 100644 --- a/tests/libtransmission/buffer-test.cc +++ b/tests/libtransmission/buffer-test.cc @@ -108,6 +108,40 @@ TEST_F(BufferTest, Move) EXPECT_EQ(3U, std::size(a)); } +TEST_F(BufferTest, Numbers) +{ + for (auto i = 0; i < 100; ++i) + { + auto const expected_u8 = tr_rand_obj(); + auto const expected_u16 = tr_rand_obj(); + auto const expected_u32 = tr_rand_obj(); + auto const expected_u64 = tr_rand_obj(); + + auto buf = Buffer{}; + + buf.add_uint8(expected_u8); + buf.add_uint16(expected_u16); + buf.add_uint32(expected_u32); + buf.add_uint64(expected_u64); + + EXPECT_EQ(expected_u8, buf.to_uint8()); + EXPECT_EQ(expected_u16, buf.to_uint16()); + EXPECT_EQ(expected_u32, buf.to_uint32()); + EXPECT_EQ(expected_u64, buf.to_uint64()); + + buf.add_uint64(expected_u64); + buf.add_uint32(expected_u32); + buf.add_uint16(expected_u16); + buf.add_uint8(expected_u8); + + EXPECT_EQ(expected_u64, buf.to_uint64()); + EXPECT_EQ(expected_u32, buf.to_uint32()); + EXPECT_EQ(expected_u16, buf.to_uint16()); + EXPECT_EQ(expected_u8, buf.to_uint8()); + } +} + +#if 0 TEST_F(BufferTest, NonBufferWriter) { auto constexpr Hello = "Hello, "sv; @@ -137,7 +171,8 @@ TEST_F(BufferTest, NonBufferWriter) out1.add(Bang); out2.add(Bang); - auto const result1 = out1.pullup_sv(); + auto const result1 = out1.to_string_view(); auto const result2 = std::string_view{ reinterpret_cast(std::data(out2_vec)), std::size(out2_vec) }; EXPECT_EQ(result1, result2); } +#endif