From 1cf9f407f2314a34926261d34fa02d90997b7cc3 Mon Sep 17 00:00:00 2001
From: Tor Brede Vekterli <vekterli@oath.com>
Date: Thu, 17 Jan 2019 15:20:28 +0000
Subject: Ensure that asciistream moves and swaps have expected semantics

Defaulted move ctor and assignment will not have the expected behavior
for the current _rbuf pointer when it points into a short-string
optimized _wbuf buffer. I.e. it will be pointing into the buffer in
the object that was just moved away from.

Update swap() to give the correct semantics when either/both arguments
point to a read-only buffer. Would previously reset _rbuf to _wbuf
unconditionally, effectively forgetting the string that was referenced.
---
 vespalib/src/tests/stllike/asciistream_test.cpp    | 19 ++++++++++++++
 .../src/vespa/vespalib/stllike/asciistream.cpp     | 29 ++++++++++++++++++++--
 vespalib/src/vespa/vespalib/stllike/asciistream.h  |  6 ++---
 3 files changed, 49 insertions(+), 5 deletions(-)

(limited to 'vespalib')

diff --git a/vespalib/src/tests/stllike/asciistream_test.cpp b/vespalib/src/tests/stllike/asciistream_test.cpp
index 955edc25d7e..2c89bab75fa 100644
--- a/vespalib/src/tests/stllike/asciistream_test.cpp
+++ b/vespalib/src/tests/stllike/asciistream_test.cpp
@@ -23,6 +23,7 @@ public:
     void testWriteThenRead();
     void testGetLine();
     void testCopyConstruct();
+    void testMoveIsWellDefined();
     void testIllegalNumbers();
     void testDouble();
     void testStateSaver();
@@ -168,6 +169,23 @@ AsciistreamTest::testCopyConstruct()
     EXPECT_TRUE(os3.str() == os2.str());
 }
 
+void
+AsciistreamTest::testMoveIsWellDefined()
+{
+    asciistream read_only("hello world");
+    asciistream dest(std::move(read_only));
+    EXPECT_EQUAL("hello world", dest.str());
+
+    read_only = asciistream("a string long enough to not be short string optimized");
+    dest = std::move(read_only);
+    EXPECT_EQUAL("a string long enough to not be short string optimized", dest.str());
+
+    asciistream written_src;
+    written_src << "a foo walks into a bar";
+    dest = std::move(written_src);
+    EXPECT_EQUAL("a foo walks into a bar", dest.str());
+}
+
 void
 AsciistreamTest::testIntegerManip()
 {
@@ -518,6 +536,7 @@ AsciistreamTest::Main()
     TEST_DO(verifyBothWays<uint64_t>(7, "7"));
 
     testCopyConstruct();
+    testMoveIsWellDefined();
     testIntegerManip();
     testFill();
     testString();
diff --git a/vespalib/src/vespa/vespalib/stllike/asciistream.cpp b/vespalib/src/vespa/vespalib/stllike/asciistream.cpp
index 9b2cdad822a..35f7b0e1be7 100644
--- a/vespalib/src/vespa/vespalib/stllike/asciistream.cpp
+++ b/vespalib/src/vespa/vespalib/stllike/asciistream.cpp
@@ -102,17 +102,42 @@ asciistream & asciistream::operator = (const asciistream & rhs)
     return *this;
 }
 
-void asciistream::swap(asciistream & rhs)
+asciistream::asciistream(asciistream && rhs) noexcept
+    : asciistream()
+{
+    swap(rhs);
+}
+
+asciistream & asciistream::operator = (asciistream && rhs) noexcept
+{
+    if (this != &rhs) {
+        swap(rhs);
+    }
+    return *this;
+}
+
+void asciistream::swap(asciistream & rhs) noexcept
 {
     std::swap(_rPos, rhs._rPos);
+    // If read-only, _wbuf is empty and _rbuf is set
+    // If ever written to, _rbuf == _wbuf
+    const bool lhs_read_only = (_rbuf.data() != _wbuf.data());
+    const bool rhs_read_only = (rhs._rbuf.data() != rhs._wbuf.data());
     std::swap(_wbuf, rhs._wbuf);
+    std::swap(_rbuf, rhs._rbuf);
+    if (!lhs_read_only) {
+        rhs._rbuf = rhs._wbuf;
+    }
+    if (!rhs_read_only) {
+        _rbuf = _wbuf;
+    }
+
     std::swap(_base, rhs._base);
     std::swap(_floatSpec, rhs._floatSpec);
     std::swap(_floatModifier, rhs._floatModifier);
     std::swap(_width, rhs._width);
     std::swap(_precision, rhs._precision);
     std::swap(_fill, rhs._fill);
-    _rbuf = _wbuf;
 }
 
 namespace {
diff --git a/vespalib/src/vespa/vespalib/stllike/asciistream.h b/vespalib/src/vespa/vespalib/stllike/asciistream.h
index c225c371a78..428ed983e16 100644
--- a/vespalib/src/vespa/vespalib/stllike/asciistream.h
+++ b/vespalib/src/vespa/vespalib/stllike/asciistream.h
@@ -35,9 +35,9 @@ public:
     ~asciistream();
     asciistream(const asciistream & rhs);
     asciistream & operator = (const asciistream & rhs);
-    asciistream(asciistream &&) = default;
-    asciistream & operator = (asciistream &&) = default;
-    void swap(asciistream & rhs);
+    asciistream(asciistream &&) noexcept;
+    asciistream & operator = (asciistream &&) noexcept;
+    void swap(asciistream & rhs) noexcept;
     asciistream & operator << (bool v)                { if (v) { *this << '1'; } else { *this << '0'; } return *this; }
     asciistream & operator << (char v)                { doFill(1); write(&v, 1); return *this; }
     asciistream & operator << (unsigned char v)       { doFill(1); write(&v, 1); return *this; }
-- 
cgit v1.2.3