Check for overflow when parsing argument index. Improve error handling. Fix overload issues. More tests.

format.cc

@@ -6,16 +6,22 @@
 #include "format.h"
 
 #include <cassert>
+#include <climits>
 #include <cstring>
 #include <algorithm>
 
 using std::size_t;
 
-static void CheckClosingBrace(const char *s) {
+// Throws Exception(message) if s contains '}' and FormatError reporting
+// unmatched '{' otherwise. The idea is that unmatched '{' should override
+// other errors.
+template <typename Exception>
+static void Throw(const char *s, const char *message) {
   while (*s && *s != '}')
     ++s;
   if (!*s)
     throw fmt::FormatError("unmatched '{' in format");
+  throw Exception(message);
 }
 
 template <typename T>
@@ -54,18 +60,17 @@ void fmt::Formatter::Format() {
     // Parse argument index.
     unsigned arg_index = 0;
     if ('0' <= *s && *s <= '9') {
-      // TODO: check overflow
       do {
-        arg_index = arg_index * 10 + (*s++ - '0');
+        unsigned index = arg_index * 10 + (*s++ - '0');
+        if (index < arg_index)  // Check if index wrapped around.
+          Throw<FormatError>(s, "argument index is too big"); // TODO: test
+        arg_index = index;
       } while ('0' <= *s && *s <= '9');
     } else {
-      CheckClosingBrace(s);
+      Throw<FormatError>(s, "missing argument index in format string");
-      throw FormatError("missing argument index in format string");
-    }
-    if (arg_index >= args_.size()) {
-      CheckClosingBrace(s);
-      throw std::out_of_range("argument index is out of range in format");
     }
+    if (arg_index >= args_.size())
+      Throw<std::out_of_range>(s, "argument index is out of range in format");
 
     char arg_format[8];  // longest format: %+0*.*ld
     char *arg_format_ptr = arg_format;
@@ -86,6 +91,7 @@ void fmt::Formatter::Format() {
         *arg_format_ptr++ = '*';
         width = 0;
         do {
+          // TODO: check overflow
           width = width * 10 + (*s++ - '0');
         } while ('0' <= *s && *s <= '9');
       }
@@ -98,6 +104,7 @@ void fmt::Formatter::Format() {
         precision = 0;
         if ('0' <= *s && *s <= '9') {
           do {
+            // TODO: check overflow
             precision = precision * 10 + (*s++ - '0');
           } while ('0' <= *s && *s <= '9');
         } else {
@@ -180,6 +187,9 @@ void fmt::Formatter::Format() {
       *arg_format_ptr = '\0';
       FormatArg(arg_format, arg.pointer_value, width, precision);
       break;
+    case OTHER:
+      (this->*arg.format)(arg.other_value);
+      break;
     default:
       assert(false);
       break;
@@ -192,4 +202,3 @@ fmt::ArgFormatter::~ArgFormatter() {
   if (!formatter_) return;
   FinishFormatting();
 }
-

format.h

@@ -31,7 +31,8 @@ class Formatter {
   std::vector<char> buffer_;  // Output buffer.
 
   enum Type {
-    CHAR, INT, UINT, LONG, ULONG, DOUBLE, LONG_DOUBLE, STRING, WSTRING, POINTER
+    CHAR, INT, UINT, LONG, ULONG, DOUBLE, LONG_DOUBLE,
+    STRING, WSTRING, POINTER, OTHER
   };
 
   struct Arg {
@@ -46,6 +47,10 @@ class Formatter {
       const char *string_value;
       const wchar_t *wstring_value;
       const void *pointer_value;
+      struct {
+        const void *other_value;
+        void (Formatter::*format)(const void *value);
+      };
     };
 
     explicit Arg(char value) : type(CHAR), int_value(value) {}
@@ -59,6 +64,8 @@ class Formatter {
     explicit Arg(const char *value) : type(STRING), string_value(value) {}
     explicit Arg(const wchar_t *value) : type(WSTRING), wstring_value(value) {}
     explicit Arg(const void *value) : type(POINTER), pointer_value(value) {}
+    explicit Arg(const void *value, void (Formatter::*format)(const void *))
+    : type(OTHER), format(format) {}
   };
 
   std::vector<Arg> args_;
@@ -76,6 +83,12 @@ class Formatter {
   template <typename T>
   void FormatArg(const char *format, const T &arg, int width, int precision);
 
+  template <typename T>
+  void FormatOtherArg(const void *value) {
+    const T &typed_value = *static_cast<const T*>(value);
+    // TODO: format value
+  }
+
   void Format();
 
  public:
@@ -88,8 +101,20 @@ class Formatter {
 
   const char *c_str() const { return &buffer_[0]; }
   std::size_t size() const { return buffer_.size() - 1; }
+
+  void Swap(Formatter &f) {
+    buffer_.swap(f.buffer_);
+    args_.swap(f.args_);
+  }
 };
 
+template <typename T>
+struct AddPtrConst { typedef T Value; };
+
+// Convert "T*" into "const T*".
+template <typename T>
+struct AddPtrConst<T*> { typedef const T* Value; };
+
 class ArgFormatter {
  private:
   friend class Formatter;
@@ -177,6 +202,15 @@ class ArgFormatter {
   // arbitrary pointers. If you want to output a pointer cast it to void*.
   template <typename T>
   ArgFormatter &operator<<(const T *value);
+
+  // If T is a pointer type, say "U*", AddPtrConst<T>::Value will be
+  // "const U*". This additional const ensures that operator<<(const void *)
+  // and not this method is called both for "const void*" and "void*".
+  template <typename T>
+  ArgFormatter &operator<<(const typename AddPtrConst<T>::Value &value) {
+    formatter_->Add(Formatter::Arg(&value, &Formatter::FormatOtherArg<T>));
+    return *this;
+  }
 };
 
 template <typename Callback>
@@ -203,24 +237,29 @@ Formatter::FormatWithCallback(const char *format) {
   return ArgFormatterWithCallback<Callback>(*this);
 }
 
-class Format : public ArgFormatter {
+class FullFormat : public ArgFormatter {
  private:
-  Formatter formatter_;
+  mutable Formatter formatter_;
 
   // Do not implement.
-  Format(const Format&);
+  FullFormat& operator=(const FullFormat&);
-  Format& operator=(const Format&);
 
  public:
-  explicit Format(const char *format) : ArgFormatter(formatter_) {
+  explicit FullFormat(const char *format) : ArgFormatter(formatter_) {
     ArgFormatter::operator=(formatter_(format));
   }
 
-  ~Format() {
+  FullFormat(const FullFormat& other) : ArgFormatter(other) {
+    formatter_.Swap(other.formatter_);
+  }
+
+  ~FullFormat() {
     FinishFormatting();
   }
 };
 
+inline FullFormat Format(const char *format) { return FullFormat(format); }
+
 class Print : public ArgFormatter {
  private:
   Formatter formatter_;

format_test.cc

@@ -68,14 +68,31 @@ TEST(FormatterTest, FormatArgs) {
   EXPECT_EQ("abracadabra", str(Format("{0}{1}{0}") << "abra" << "cad"));
 }
 
-TEST(FormatterTest, InvalidFormat) {
+TEST(FormatterTest, FormatErrors) {
   //Format("{");
+  EXPECT_THROW_MSG(Format("{"), FormatError, "unmatched '{' in format");
   EXPECT_THROW_MSG(Format("{}"), FormatError,
       "missing argument index in format string");
-  EXPECT_THROW_MSG(Format("{"), FormatError, "unmatched '{' in format");
   EXPECT_THROW_MSG(Format("{0"), FormatError, "unmatched '{' in format");
   EXPECT_THROW_MSG(Format("{0}"), std::out_of_range,
       "argument index is out of range in format");
+  char format[256];
+  std::sprintf(format, "{%u", UINT_MAX);
+  EXPECT_THROW_MSG(Format(format), FormatError, "unmatched '{' in format");
+  std::sprintf(format, "{%u}", UINT_MAX);
+  EXPECT_THROW_MSG(Format(format), std::out_of_range,
+      "argument index is out of range in format");
+  if (ULONG_MAX > UINT_MAX) {
+    std::sprintf(format, "{%lu", UINT_MAX + 1l);
+    EXPECT_THROW_MSG(Format(format), FormatError, "unmatched '{' in format");
+    std::sprintf(format, "{%lu}", UINT_MAX + 1l);
+    EXPECT_THROW_MSG(Format(format), FormatError, "argument index is too big");
+  } else {
+    std::sprintf(format, "{%u0", UINT_MAX);
+    EXPECT_THROW_MSG(Format(format), FormatError, "unmatched '{' in format");
+    std::sprintf(format, "{%u0}", UINT_MAX);
+    EXPECT_THROW_MSG(Format(format), FormatError, "argument index is too big");
+  }
   // TODO
 }