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

@@ -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]) {
+  FormatBuf(Buffer<Char> &buffer) : buffer_(buffer) {}
-    this->setp(start_, start_ + buffer_.capacity());
-  }
 
-  FormatBuf(Buffer<Char> &buffer, Char *start) : buffer_(buffer) , start_(start) {
+ protected:
-    this->setp(start_, start_ + buffer_.capacity());
+  // 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())) {
+    if (!traits_type::eq_int_type(ch, traits_type::eof()))
-      size_t buf_size = size();
+      buffer_.push_back(ch);
-      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);
-    }
     return ch;
   }
 
-  size_t size() const {
+  std::streamsize xsputn(const Char *s, std::streamsize count) FMT_OVERRIDE {
-    return to_unsigned(this->pptr() - start_);
+    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();
+  internal::FormatBuf<Char> format_buf(writer.buffer());
-  Char *start = &buffer[0] + buffer.size();
-
-  internal::FormatBuf<Char> format_buf(buffer, start);
   std::basic_ostream<Char> output(&format_buf);
   output << value;
 
-  buffer.resize(buffer.size() + format_buf.size());
   return writer;
 }
 #endif

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