From 73ca9948fec907e118c973700b34ae96c69791c0 Mon Sep 17 00:00:00 2001
From: effzeh <effzeh@protonmail.com>
Date: Thu, 6 Apr 2017 19:02:03 +0200
Subject: [PATCH] Fix FormatBuf implementation (#491)

Fixes #491 (and probably #480) Before, the put-area of the custom streambuf
implementation was (sometimes) incorrectly extended beyond the writeable buffer.
The new implementation is in some cases not as efficient as the old, but avoids
to write into uninitialized memory.
---
 fmt/ostream.h        | 39 +++++++++++++++------------------------
 test/ostream-test.cc | 23 +++++++++++++++++++++++
 2 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/fmt/ostream.h b/fmt/ostream.h
index 7e13a5a7..f529e241 100644
--- a/fmt/ostream.h
+++ b/fmt/ostream.h
@@ -24,32 +24,27 @@ class FormatBuf : public std::basic_streambuf<Char> {
   typedef typename std::basic_streambuf<Char>::traits_type traits_type;
 
   Buffer<Char> &buffer_;
-  Char *start_;
 
  public:
-  FormatBuf(Buffer<Char> &buffer) : buffer_(buffer), start_(&buffer[0]) {
-    this->setp(start_, start_ + buffer_.capacity());
-  }
+  FormatBuf(Buffer<Char> &buffer) : buffer_(buffer) {}
 
-  FormatBuf(Buffer<Char> &buffer, Char *start) : buffer_(buffer) , start_(start) {
-    this->setp(start_, start_ + buffer_.capacity());
-  }
+ protected:
+  // The put-area is actually always empty. This makes the implementation
+  // simpler and has the advantage that the streambuf and the buffer are always
+  // in sync and sputc never writes into uninitialized memory. The obvious
+  // disadvantage is that each call to sputc always results in a (virtual) call
+  // to overflow. There is no disadvantage here for sputn since this always
+  // results in a call to xsputn.
 
   int_type overflow(int_type ch = traits_type::eof()) FMT_OVERRIDE {
-    if (!traits_type::eq_int_type(ch, traits_type::eof())) {
-      size_t buf_size = size();
-      buffer_.resize(buf_size);
-      buffer_.reserve(buf_size * 2);
-
-      start_ = &buffer_[0];
-      start_[buf_size] = traits_type::to_char_type(ch);
-      this->setp(start_+ buf_size + 1, start_ + buf_size * 2);
-    }
+    if (!traits_type::eq_int_type(ch, traits_type::eof()))
+      buffer_.push_back(ch);
     return ch;
   }
 
-  size_t size() const {
-    return to_unsigned(this->pptr() - start_);
+  std::streamsize xsputn(const Char *s, std::streamsize count) FMT_OVERRIDE {
+    buffer_.append(s, s + count);
+    return count;
   }
 };
 
@@ -99,7 +94,7 @@ void format_arg(BasicFormatter<Char, ArgFormatter_> &f,
   std::basic_ostream<Char> output(&format_buf);
   output << value;
 
-  BasicStringRef<Char> str(&buffer[0], format_buf.size());
+  BasicStringRef<Char> str(&buffer[0], buffer.size());
   typedef internal::MakeArg< BasicFormatter<Char> > MakeArg;
   format_str = f.format(format_str, MakeArg(str));
 }
@@ -128,14 +123,10 @@ typename std::enable_if<
 operator<<(BasicWriter<Char> &writer, const T &value) {
   FMT_STATIC_ASSERT(internal::is_streamable<T>::value, "T must be Streamable");
 
-  auto &buffer = writer.buffer();
-  Char *start = &buffer[0] + buffer.size();
-
-  internal::FormatBuf<Char> format_buf(buffer, start);
+  internal::FormatBuf<Char> format_buf(writer.buffer());
   std::basic_ostream<Char> output(&format_buf);
   output << value;
 
-  buffer.resize(buffer.size() + format_buf.size());
   return writer;
 }
 #endif
diff --git a/test/ostream-test.cc b/test/ostream-test.cc
index a7341065..0e84f527 100644
--- a/test/ostream-test.cc
+++ b/test/ostream-test.cc
@@ -191,3 +191,26 @@ TEST(OStreamTest, WriteToOStreamMaxSize) {
   } while (size != 0);
   fmt::internal::write(os, w);
 }
+
+#if __cplusplus >= 201103L
+struct Xs {
+  const size_t size;
+  const std::string s;
+  Xs() : size(200), s(size, 'x') {}
+};
+
+inline std::ostream& operator<<(std::ostream& os, Xs const& xs) {
+  return os << xs.s;
+}
+
+TEST(OStreamTest, FormatBuf1) {
+  Xs xs;
+  fmt::MemoryWriter w;
+  int n = fmt::internal::INLINE_BUFFER_SIZE / xs.size + 1;
+  for (int i = 0; i < n; ++i)
+    w << xs;
+  EXPECT_EQ(w.size(), size_t(n * xs.size));
+  w << xs;
+  EXPECT_EQ(w.size(), size_t((n + 1) * xs.size));
+}
+#endif