From 4f3ce27ddcee38ba2c23b264ea60b90138efcf17 Mon Sep 17 00:00:00 2001 From: Karl Wiberg Date: Wed, 17 Oct 2018 13:34:33 +0200 Subject: [PATCH] rtc::Buffer: Handle move self-assignment The object should end up in a valid state, just like after being moved from. Bug: webrtc:9857 Change-Id: Ia11f9b8e3191ffe749e4a0640cad946038f494a4 Reviewed-on: https://webrtc-review.googlesource.com/c/106701 Reviewed-by: Niels Moller Commit-Queue: Karl Wiberg Cr-Commit-Position: refs/heads/master@{#25233} --- rtc_base/buffer.h | 5 ++++- rtc_base/buffer_unittest.cc | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/rtc_base/buffer.h b/rtc_base/buffer.h index 9bf96cd146..2707cdfb86 100644 --- a/rtc_base/buffer.h +++ b/rtc_base/buffer.h @@ -152,7 +152,9 @@ class BufferT { RTC_DCHECK(buf.IsConsistent()); size_ = buf.size_; capacity_ = buf.capacity_; - data_ = std::move(buf.data_); + using std::swap; + swap(data_, buf.data_); + buf.data_.reset(); buf.OnMovedFrom(); return *this; } @@ -399,6 +401,7 @@ class BufferT { // Called when *this has been moved from. Conceptually it's a no-op, but we // can mutate the state slightly to help subsequent sanity checks catch bugs. void OnMovedFrom() { + RTC_DCHECK(!data_); // Our heap block should have been stolen. #if RTC_DCHECK_IS_ON // Ensure that *this is always inconsistent, to provoke bugs. size_ = 1; diff --git a/rtc_base/buffer_unittest.cc b/rtc_base/buffer_unittest.cc index 1c3abfd08f..b2f47c16ec 100644 --- a/rtc_base/buffer_unittest.cc +++ b/rtc_base/buffer_unittest.cc @@ -185,6 +185,17 @@ TEST(BufferTest, TestMoveAssign) { EXPECT_TRUE(buf1.empty()); } +TEST(BufferTest, TestMoveAssignSelf) { + // Move self-assignment isn't required to produce a meaningful state, but + // should not leave the object in an inconsistent state. (Such inconsistent + // state could be caught by the DCHECKs and/or by the leak checker.) We need + // to be sneaky when testing this; if we're doing a too-obvious + // move-assign-to-self, clang's -Wself-move triggers at compile time. + Buffer buf(kTestData, 3, 40); + Buffer* buf_ptr = &buf; + buf = std::move(*buf_ptr); +} + TEST(BufferTest, TestSwap) { Buffer buf1(kTestData, 3); Buffer buf2(kTestData, 6, 40);