From: Radoslaw Zarzynski Date: Thu, 23 Jan 2020 12:22:45 +0000 (+0100) Subject: common/bl: drop get_raw() from the public buffer::ptr interface. X-Git-Tag: v15.1.1~584^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ad335a7c4746ecc1cd923e7717e28823bdbd6d3f;p=ceph-ci.git common/bl: drop get_raw() from the public buffer::ptr interface. Signed-off-by: Radoslaw Zarzynski --- diff --git a/src/common/buffer.cc b/src/common/buffer.cc index 5ad2fa50225..b5a1169b4e8 100644 --- a/src/common/buffer.cc +++ b/src/common/buffer.cc @@ -746,7 +746,7 @@ static ceph::spinlock debug_lock; { if (p == ls->end()) throw end_of_buffer(); - return p->get_raw() == other.get_raw(); + return p->_raw == other._raw; } // copy data out. @@ -1147,14 +1147,14 @@ static ceph::spinlock debug_lock; void buffer::list::reassign_to_mempool(int pool) { for (auto& p : _buffers) { - p.get_raw()->reassign_to_mempool(pool); + p._raw->reassign_to_mempool(pool); } } void buffer::list::try_assign_to_mempool(int pool) { for (auto& p : _buffers) { - p.get_raw()->try_assign_to_mempool(pool); + p._raw->try_assign_to_mempool(pool); } } @@ -1166,7 +1166,7 @@ static ceph::spinlock debug_lock; std::vector raw_vec; raw_vec.reserve(_buffers.size()); for (const auto& p : _buffers) - raw_vec.push_back(p.get_raw()); + raw_vec.push_back(p._raw); std::sort(raw_vec.begin(), raw_vec.end()); uint64_t total = 0; @@ -1311,7 +1311,7 @@ static ceph::spinlock debug_lock; auto curbuf_prev = bl._buffers.before_begin(); while (curbuf != bl._buffers.end()) { - const auto* const raw = curbuf->get_raw(); + const auto* const raw = curbuf->_raw; if (unlikely(raw && !raw->is_shareable())) { auto* clone = ptr_node::copy_hypercombined(*curbuf); curbuf = bl._buffers.erase_after_and_dispose(curbuf_prev); @@ -1485,8 +1485,7 @@ static ceph::spinlock debug_lock; ceph_assert(len+off <= bp.length()); if (!_buffers.empty()) { ptr &l = _buffers.back(); - if (l.get_raw() == bp.get_raw() && - l.end() == bp.start() + off) { + if (l._raw == bp._raw && l.end() == bp.start() + off) { // yay contiguous with tail bp! l.set_length(l.length()+len); _len += len; @@ -2023,7 +2022,7 @@ __u32 buffer::list::crc32c(__u32 crc) const for (const auto& node : _buffers) { if (node.length()) { - raw* const r = node.get_raw(); + raw* const r = node._raw; pair ofs(node.offset(), node.offset() + node.length()); pair ccrc; if (r->get_crc(ofs, &ccrc)) { @@ -2067,9 +2066,8 @@ __u32 buffer::list::crc32c(__u32 crc) const void buffer::list::invalidate_crc() { for (const auto& node : _buffers) { - raw* const r = node.get_raw(); - if (r) { - r->invalidate_crc(); + if (node._raw) { + node._raw->invalidate_crc(); } } } @@ -2193,7 +2191,7 @@ bool buffer::ptr_node::dispose_if_hypercombined( buffer::ptr_node* const delete_this) { const bool is_hypercombined = static_cast(delete_this) == \ - static_cast(&delete_this->get_raw()->bptr_storage); + static_cast(&delete_this->_raw->bptr_storage); if (is_hypercombined) { ceph_assert_always("hypercombining is currently disabled" == nullptr); delete_this->~ptr_node(); @@ -2217,14 +2215,14 @@ buffer::ptr_node* buffer::ptr_node::copy_hypercombined( // FIXME: we don't currently hypercombine buffers due to crashes // observed in the rados suite. After fixing we'll use placement // new to create ptr_node on buffer::raw::bptr_storage. - auto raw_new = copy_this.get_raw()->clone(); + auto raw_new = copy_this._raw->clone(); return new ptr_node(copy_this, std::move(raw_new)); } buffer::ptr_node* buffer::ptr_node::cloner::operator()( const buffer::ptr_node& clone_this) { - const raw* const raw_this = clone_this.get_raw(); + const raw* const raw_this = clone_this._raw; if (likely(!raw_this || raw_this->is_shareable())) { return new ptr_node(clone_this); } else { diff --git a/src/include/buffer.h b/src/include/buffer.h index 8e91aafa47b..07bff344fd3 100644 --- a/src/include/buffer.h +++ b/src/include/buffer.h @@ -183,6 +183,8 @@ inline namespace v14_2_0 { * a buffer pointer. references (a subsequence of) a raw buffer. */ class CEPH_BUFFER_API ptr { + friend class list; + protected: raw *_raw; public: // dirty hack for testing; if it works, this will be abstracted unsigned _off, _len; @@ -306,7 +308,6 @@ inline namespace v14_2_0 { void try_assign_to_mempool(int pool); // accessors - raw *get_raw() const { return _raw; } const char *c_str() const; char *c_str(); const char *end_c_str() const; diff --git a/src/test/bufferlist.cc b/src/test/bufferlist.cc index 5387437a3d3..85f4cbfe09d 100644 --- a/src/test/bufferlist.cc +++ b/src/test/bufferlist.cc @@ -44,6 +44,12 @@ static char cmd[128]; +struct instrumented_bptr : public ceph::buffer::ptr { + const ceph::buffer::raw* get_raw() const { + return _raw; + } +}; + TEST(Buffer, constructors) { unsigned len = 17; // @@ -146,7 +152,7 @@ TEST(Buffer, BenchAlloc) { TEST(BufferRaw, ostream) { bufferptr ptr(1); std::ostringstream stream; - stream << *ptr.get_raw(); + stream << *static_cast(ptr).get_raw(); EXPECT_GT(stream.str().size(), stream.str().find("buffer::raw(")); EXPECT_GT(stream.str().size(), stream.str().find("len 1 nref 1)")); } @@ -215,7 +221,8 @@ TEST(BufferPtr, constructors) { bufferptr original(str.c_str(), len); bufferptr ptr(original); EXPECT_TRUE(ptr.have_raw()); - EXPECT_EQ(original.get_raw(), ptr.get_raw()); + EXPECT_EQ(static_cast(original).get_raw(), + static_cast(ptr).get_raw()); EXPECT_EQ(2, ptr.raw_nref()); EXPECT_EQ(0, ::memcmp(original.c_str(), ptr.c_str(), len)); } @@ -227,7 +234,8 @@ TEST(BufferPtr, constructors) { bufferptr original(str.c_str(), len); bufferptr ptr(original, 0, 0); EXPECT_TRUE(ptr.have_raw()); - EXPECT_EQ(original.get_raw(), ptr.get_raw()); + EXPECT_EQ(static_cast(original).get_raw(), + static_cast(ptr).get_raw()); EXPECT_EQ(2, ptr.raw_nref()); EXPECT_EQ(0, ::memcmp(original.c_str(), ptr.c_str(), len)); PrCtl unset_dumpable; @@ -286,7 +294,8 @@ TEST(BufferPtr, assignment) { original.set_length(length); same_raw = original; ASSERT_EQ(2, original.raw_nref()); - ASSERT_EQ(same_raw.get_raw(), original.get_raw()); + ASSERT_EQ(static_cast(same_raw).get_raw(), + static_cast(original).get_raw()); ASSERT_EQ(same_raw.offset(), original.offset()); ASSERT_EQ(same_raw.length(), original.length()); } @@ -317,7 +326,8 @@ TEST(BufferPtr, assignment) { bufferptr ptr; ptr = original; ASSERT_EQ(2, original.raw_nref()); - ASSERT_EQ(ptr.get_raw(), original.get_raw()); + ASSERT_EQ(static_cast(ptr).get_raw(), + static_cast(original).get_raw()); ASSERT_EQ(original.offset(), ptr.offset()); ASSERT_EQ(original.length(), ptr.length()); } @@ -410,7 +420,7 @@ TEST(BufferPtr, accessors) { ptr[1] = 'Y'; const bufferptr const_ptr(ptr); - EXPECT_NE((void*)NULL, (void*)ptr.get_raw()); + EXPECT_NE((void*)nullptr, (void*)static_cast(ptr).get_raw()); EXPECT_EQ('X', ptr.c_str()[0]); { bufferptr ptr; @@ -1633,7 +1643,9 @@ TEST(BufferList, push_back) { EXPECT_EQ((unsigned)(1 + len), bl.length()); EXPECT_EQ((unsigned)2, bl.get_num_buffers()); EXPECT_EQ('B', bl.back()[0]); - EXPECT_EQ(ptr.get_raw(), bl.back().get_raw()); + const bufferptr& back_bp = bl.back(); + EXPECT_EQ(static_cast(ptr).get_raw(), + static_cast(back_bp).get_raw()); } // // void push_back(ptr&& bp) @@ -1654,7 +1666,7 @@ TEST(BufferList, push_back) { EXPECT_EQ((unsigned)(1 + len), bl.length()); EXPECT_EQ((unsigned)2, bl.buffers().size()); EXPECT_EQ('B', bl.buffers().back()[0]); - EXPECT_FALSE(ptr.get_raw()); + EXPECT_FALSE(static_cast(ptr).get_raw()); } } @@ -2054,7 +2066,7 @@ TEST(BufferList, append) { bl.append(std::move(ptr)); EXPECT_EQ((unsigned)1, bl.buffers().size()); EXPECT_EQ((unsigned)3, bl.length()); - EXPECT_FALSE(ptr.get_raw()); + EXPECT_FALSE(static_cast(ptr).get_raw()); } } }