Merge pull request #11381 from dotnwat/bytes-nits

A few v::bytes clean-ups

src/v/bytes/details/io_fragment.h

@@ -20,32 +20,27 @@
 namespace details {
 class io_fragment {
 public:
-    struct full {};
-    struct empty {};
-
-    io_fragment(ss::temporary_buffer<char> buf, full)
+    /**
+     * Initialize fragment from the provided temporary buffer.
+     */
+    explicit io_fragment(ss::temporary_buffer<char> buf)
       : _buf(std::move(buf))
       , _used_bytes(_buf.size()) {}
-    io_fragment(ss::temporary_buffer<char> buf, empty)
-      : _buf(std::move(buf))
+
+    /**
+     * Initialize an empty fragment of a given size.
+     */
+    explicit io_fragment(size_t size)
+      : _buf(size)
       , _used_bytes(0) {}
+
     io_fragment(io_fragment&& o) noexcept = delete;
     io_fragment& operator=(io_fragment&& o) noexcept = delete;
     io_fragment(const io_fragment& o) = delete;
     io_fragment& operator=(const io_fragment& o) = delete;
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Warray-bounds"
     ~io_fragment() noexcept = default;
-#pragma GCC diagnostic pop
 
-    bool operator==(const io_fragment& o) const {
-        return _used_bytes == o._used_bytes && _buf == o._buf;
-    }
-    bool operator!=(const io_fragment& o) const { return !(*this == o); }
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Warray-bounds"
     bool is_empty() const { return _used_bytes == 0; }
-#pragma GCC diagnostic pop
     size_t available_bytes() const { return _buf.size() - _used_bytes; }
     void reserve(size_t reservation) {
         check_out_of_range(reservation, available_bytes());

src/v/bytes/iobuf.cc

@@ -152,8 +152,8 @@ iobuf iobuf_copy(iobuf::iterator_consumer& in, size_t len) {
 
         bytes_left -= buf.size();
 
-        auto f = new iobuf::fragment(std::move(buf), iobuf::fragment::full{});
-        ret.append_take_ownership(f);
+        auto f = std::make_unique<iobuf::fragment>(std::move(buf));
+        ret.append(std::move(f));
     }
 
     vassert(bytes_left == 0, "Bytes remaining to be copied");
@@ -178,8 +178,8 @@ iobuf iobuf::share(size_t pos, size_t len) {
             left_in_frag = left;
             left = 0;
         }
-        auto f = new fragment(frag.share(pos, left_in_frag), fragment::full{});
-        ret.append_take_ownership(f);
+        auto f = std::make_unique<fragment>(frag.share(pos, left_in_frag));
+        ret.append(std::move(f));
         pos = 0;
     }
     return ret;

src/v/bytes/iobuf.h

@@ -139,8 +139,11 @@ class iobuf {
      */
     void append_fragments(iobuf);
 
-    /// \brief trims the back, and appends direct.
-    void append_take_ownership(fragment*);
+    /**
+     * Append a fragment.
+     */
+    void append(std::unique_ptr<fragment>);
+
     /// prepends the _the buffer_ as iobuf::details::io_fragment::full{}
     void prepend(ss::temporary_buffer<char>);
     /// prepends the arg to this as iobuf::details::io_fragment::full{}
@@ -220,14 +223,13 @@ inline size_t iobuf::last_allocation_size() const {
     return _frags.empty() ? details::io_allocation_size::default_chunk_size
                           : _frags.back().capacity();
 }
-/// \brief trims the back, and appends direct.
-inline void iobuf::append_take_ownership(fragment* f) {
+inline void iobuf::append(std::unique_ptr<fragment> f) {
     if (!_frags.empty()) {
         _frags.back().trim();
     }
     // NOTE: this _must_ be size and _not_ capacity
     _size += f->size();
-    _frags.push_back(*f);
+    _frags.push_back(*f.release());
 }
 inline void iobuf::prepend_take_ownership(fragment* f) {
     _size += f->size();
@@ -238,8 +240,7 @@ inline void iobuf::create_new_fragment(size_t sz) {
     oncore_debug_verify(_verify_shard);
     auto chunk_max = std::max(sz, last_allocation_size());
     auto asz = details::io_allocation_size::next_allocation_size(chunk_max);
-    auto f = new fragment(ss::temporary_buffer<char>(asz), fragment::empty{});
-    append_take_ownership(f);
+    append(std::make_unique<fragment>(asz));
 }
 /// only ensures that a segment of at least reservation is avaible
 /// as an empty details::io_fragment
@@ -258,7 +259,7 @@ inline void iobuf::reserve_memory(size_t reservation) {
     if (unlikely(!b.size())) {
         return;
     }
-    auto f = new fragment(std::move(b), fragment::full{});
+    auto f = new fragment(std::move(b));
     prepend_take_ownership(f);
 }
 [[gnu::always_inline]] void inline iobuf::prepend(iobuf b) {
@@ -317,9 +318,7 @@ inline void iobuf::reserve_memory(size_t reservation) {
             _frags.back().trim();
         }
     }
-    // intrusive list manages the lifetime
-    auto f = new fragment(std::move(b), fragment::full{});
-    append_take_ownership(f);
+    append(std::make_unique<fragment>(std::move(b)));
 }
 /// appends the contents of buffer; might pack values into existing space
 inline void iobuf::append(iobuf o) {
@@ -336,8 +335,7 @@ inline void iobuf::append_fragments(iobuf o) {
     oncore_debug_verify(_verify_shard);
     while (!o._frags.empty()) {
         o._frags.pop_front_and_dispose([this](fragment* f) {
-            auto frag = new fragment(f->share(), fragment::full{});
-            append_take_ownership(frag);
+            append(std::make_unique<fragment>(f->share()));
             details::dispose_io_fragment(f);
         });
     }

src/v/http/tests/framing_test.cc

@@ -78,11 +78,10 @@ class fragmented_test_data_generator {
         for (size_t fragment_size : fragmentation_hint) {
             ss::temporary_buffer<char> buf(fragment_size);
             std::generate_n(buf.get_write(), fragment_size, std::ref(*this));
-            auto fragm = new details::io_fragment(
-              std::move(buf), details::io_fragment::full{});
+            auto fragm = std::make_unique<details::io_fragment>(std::move(buf));
             // Make sure that fragmentation is not reduced by memory opt. in
             // iobuf
-            result.append_take_ownership(fragm);
+            result.append(std::move(fragm));
         }
         return result;
     }

src/v/storage/batch_cache.cc

@@ -28,9 +28,8 @@ namespace storage {
 
 batch_cache::range::range(batch_cache_index& index)
   : _index(index) {
-    auto f = new details::io_fragment(
-      ss::temporary_buffer<char>(range_size), details::io_fragment::empty{});
-    _arena.append_take_ownership(f);
+    auto f = std::make_unique<details::io_fragment>(range_size);
+    _arena.append(std::move(f));
 }
 
 batch_cache::range::range(