Add type-safe wrapper around NT_Value and NT_String.

Change-Id: Ib7ef5a6de9c8c7a1f5f6432083d1fb38328438dc

src/Storage.cpp

@@ -21,6 +21,7 @@ Storage::Storage() {}
 
 Storage::~Storage() {}
 
+/* Escapes and writes a string, including start and end double quotes */
 static void WriteString(std::ostream& os, llvm::StringRef str) {
   os << '"';
   for (auto c : str) {
@@ -53,16 +54,17 @@ static void WriteString(std::ostream& os, llvm::StringRef str) {
 }
 
 void Storage::SavePersistent(std::ostream& os) const {
+  // header
   os << "[NetworkTables Storage 3.0]\n";
 
   for (auto& i : m_entries) {
     const StorageEntry& entry = i.getValue();
     // only write persistent-flagged values
-    if ((entry.flags & NT_PERSISTENT) == 0) continue;
+    if (!entry.IsPersistent()) continue;
 
     // type
-    const NT_Value& v = entry.value;
-    switch (v.type) {
+    const Value& v = entry.value();
+    switch (v.type()) {
       case NT_BOOLEAN:
         os << "boolean ";
         break;
@@ -95,15 +97,15 @@ void Storage::SavePersistent(std::ostream& os) const {
     os << '=';
 
     // value
-    switch (v.type) {
+    switch (v.type()) {
       case NT_BOOLEAN:
-        os << (v.data.v_boolean ? "true" : "false");
+        os << (v.GetBoolean() ? "true" : "false");
         break;
       case NT_DOUBLE:
-        os << v.data.v_double;
+        os << v.GetDouble();
         break;
       case NT_STRING:
-        WriteString(os, MakeStringRef(v.data.v_string));
+        WriteString(os, v.GetString());
         break;
       case NT_RAW: {
         char* buf = new char[Base64EncodeLen(v.data.v_raw.len)];
@@ -114,27 +116,39 @@ void Storage::SavePersistent(std::ostream& os) const {
         delete[] buf;
         break;
       }
-      case NT_BOOLEAN_ARRAY:
-        for (size_t i = 0; i < v.data.arr_boolean.size; ++i) {
-          os << (v.data.arr_boolean.arr[i] ? "true" : "false");
-          if (i != (v.data.arr_boolean.size - 1))
+      case NT_BOOLEAN_ARRAY: {
+        bool first = true;
+        for (auto elem : v.GetBooleanArray()) {
+          if (!first) {
             os << ',';
+            first = false;
+          }
+          os << (elem ? "true" : "false");
         }
         break;
-      case NT_DOUBLE_ARRAY:
-        for (size_t i = 0; i < v.data.arr_double.size; ++i) {
-          os << v.data.arr_double.arr[i];
-          if (i != (v.data.arr_double.size - 1))
+      }
+      case NT_DOUBLE_ARRAY: {
+        bool first = true;
+        for (auto elem : v.GetDoubleArray()) {
+          if (!first) {
             os << ',';
+            first = false;
+          }
+          os << elem;
         }
         break;
-      case NT_STRING_ARRAY:
-        for (size_t i = 0; i < v.data.arr_double.size; ++i) {
-          WriteString(os, MakeStringRef(v.data.arr_string.arr[i]));
-          if (i != (v.data.arr_double.size - 1))
+      }
+      case NT_STRING_ARRAY: {
+        bool first = true;
+        for (auto elem : v.GetStringArray()) {
+          if (!first) {
             os << ',';
+            first = false;
+          }
+          WriteString(os, elem);
         }
         break;
+      }
       default:
         break;
     }
@@ -144,28 +158,114 @@ void Storage::SavePersistent(std::ostream& os) const {
   }
 }
 
-void Storage::LoadPersistent(std::istream& is,
+/* Extracts an escaped string token.  Does not unescape the string.
+ * If a string cannot be matched, an empty string is returned.
+ * If the string is unterminated, an empty tail string is returned.
+ * The returned token includes the starting and trailing quotes (unless the
+ * string is unterminated).
+ * Returns a pair containing the extracted token (if any) and the remaining
+ * tail string.
+ */
+static std::pair<llvm::StringRef, llvm::StringRef> ReadStringToken(
+    llvm::StringRef source) {
+  // Match opening quote
+  if (source.empty() || source.front() != '"')
+    return std::make_pair(llvm::StringRef(), source);
+
+  // Scan for ending double quote, checking for escaped as we go.
+  std::size_t size = source.size();
+  std::size_t pos;
+  for (pos = 1; pos < size; ++pos) {
+    if (source[pos] == '"' && source[pos - 1] != '\\') {
+      ++pos;  // we want to include the trailing quote in the result
+      break;
+    }
+  }
+  return std::make_pair(source.slice(0, pos), source.substr(pos));
+}
+
+static int fromxdigit(char ch) {
+  if (ch >= 'a' && ch <= 'f')
+    return (ch - 'a' + 10);
+  else if (ch >= 'A' && ch <= 'F')
+    return (ch - 'A' + 10);
+  else
+    return ch - '0';
+}
+
+static void UnescapeString(llvm::StringRef source, std::string* dest) {
+  assert(source.size() > 2 && source.front() == '"' && source.back() == '"');
+  dest->clear();
+  dest->reserve(source.size() - 2);
+  for (auto s = source.begin() + 1, end = source.end() - 1; s != end; ++s) {
+    if (*s != '\\') {
+      dest->push_back(*s);
+      continue;
+    }
+    switch (*s++) {
+      case '\\':
+      case '"':
+        dest->push_back(s[-1]);
+        break;
+      case 't':
+        dest->push_back('\t');
+        break;
+      case 'n':
+        dest->push_back('\n');
+        break;
+      case 'x': {
+        if (!isxdigit(*s)) {
+          dest->push_back('x');  // treat it like a unknown escape
+          break;
+        }
+        int ch = fromxdigit(*s);
+        ++s;
+        if (isxdigit(*s)) {
+          ch <<= 4;
+          ch |= fromxdigit(*s);
+          ++s;
+        }
+        dest->push_back(static_cast<char>(ch));
+        break;
+      }
+      default:
+        dest->push_back(s[-1]);
+        break;
+    }
+  }
+}
+
+bool Storage::LoadPersistent(std::istream& is,
                              void (*warn)(std::size_t line, const char* msg)) {
   std::string line_str;
-  std::size_t line_num = 0;
+  std::size_t line_num = 1;
+  std::string name;
+
+  // header
+  std::getline(is, line_str);
+  if (line_str != "[NetworkTables Storage 3.0]") {
+    if (warn) warn(line_num, "header line mismatch, ignoring rest of file");
+    return false;
+  }
+
   while (std::getline(is, line_str)) {
-    llvm::StringRef line(line_str);
+    llvm::StringRef line = llvm::StringRef(line_str).trim();
     ++line_num;
 
     // type
-    llvm::StringRef type_str;
-    std::tie(type_str, line) = line.split(' ');
+    llvm::StringRef type_tok;
+    std::tie(type_tok, line) = line.split(' ');
     NT_Type type = NT_UNASSIGNED;
-    if (type_str == "boolean") type = NT_BOOLEAN;
-    else if (type_str == "double") type = NT_DOUBLE;
-    else if (type_str == "string") type = NT_STRING;
-    else if (type_str == "raw") type = NT_RAW;
-    else if (type_str == "array") {
-      llvm::StringRef array_str;
-      std::tie(array_str, line) = line.split(' ');
-      if (array_str == "boolean") type = NT_BOOLEAN_ARRAY;
-      else if (array_str == "double") type = NT_DOUBLE_ARRAY;
-      else if (array_str == "string") type = NT_STRING_ARRAY;
+    if (type_tok == "boolean") type = NT_BOOLEAN;
+    else if (type_tok == "double") type = NT_DOUBLE;
+    else if (type_tok == "string") type = NT_STRING;
+    else if (type_tok == "raw") type = NT_RAW;
+    else if (type_tok == "array") {
+      llvm::StringRef array_tok;
+      std::tie(array_tok, line) = line.split(' ');
+      if (array_tok == "boolean") type = NT_BOOLEAN_ARRAY;
+      else if (array_tok == "double") type = NT_DOUBLE_ARRAY;
+      else if (array_tok == "string") type = NT_STRING_ARRAY;
     }
     if (type == NT_UNASSIGNED) {
       if (warn) warn(line_num, "unrecognized type");
@@ -173,6 +273,74 @@ void Storage::LoadPersistent(std::istream& is,
     }
 
     // name
+    llvm::StringRef name_tok;
+    std::tie(name_tok, line) = ReadStringToken(line);
+    if (name_tok.empty()) {
+      if (warn) warn(line_num, "missing name");
+      continue;
+    }
+    if (name_tok.back() != '"') {
+      if (warn) warn(line_num, "unterminated name string");
+      continue;
+    }
+    UnescapeString(name_tok, &name);
+
+    // =
+    line = line.ltrim(" \t");
+    if (line.empty() || line.front() != '=') {
+      if (warn) warn(line_num, "expected = after name");
+      continue;
+    }
+    line = line.drop_front().ltrim(" \t");
+
+    // value
+    StorageEntry entry;
+    switch (type) {
+      case NT_BOOLEAN:
+        //os << (v.data.v_boolean ? "true" : "false");
+        //if (line == "true")
+          //entry.value.
+        break;
+      case NT_DOUBLE:
+        //os << v.data.v_double;
+        break;
+      case NT_STRING:
+        //WriteString(os, MakeStringRef(v.data.v_string));
+        break;
+      case NT_RAW: {
+        //char* buf = new char[Base64EncodeLen(v.data.v_raw.len)];
+        //Base64Encode(buf,
+                     //reinterpret_cast<const unsigned char*>(v.data.v_raw.str),
+                     //v.data.v_raw.len);
+        //os << buf;
+        //delete[] buf;
+        break;
+      }
+      case NT_BOOLEAN_ARRAY:
+        //for (size_t i = 0; i < v.data.arr_boolean.size; ++i) {
+          //os << (v.data.arr_boolean.arr[i] ? "true" : "false");
+          //if (i != (v.data.arr_boolean.size - 1))
+            //os << ',';
+        //}
+        break;
+      case NT_DOUBLE_ARRAY:
+        //for (size_t i = 0; i < v.data.arr_double.size; ++i) {
+          //os << v.data.arr_double.arr[i];
+          //if (i != (v.data.arr_double.size - 1))
+            //os << ',';
+        //}
+        break;
+      case NT_STRING_ARRAY:
+        //for (size_t i = 0; i < v.data.arr_double.size; ++i) {
+          //WriteString(os, MakeStringRef(v.data.arr_string.arr[i]));
+          //if (i != (v.data.arr_double.size - 1))
+            //os << ',';
+        //}
+        break;
+      default:
+        break;
+    }
 
   }
+  return true;
 }

src/Storage.h

@@ -13,27 +13,28 @@
 
 #include "ntcore.h"
 
+#include "Value.h"
 #include "llvm/StringMap.h"
 
 namespace ntimpl {
 
-inline llvm::StringRef MakeStringRef(const NT_String& str) {
-  return llvm::StringRef(str.str, str.len);
-}
-
 class StorageEntry {
  public:
-  StorageEntry() {
-    NT_InitValue(&value);
-    flags = 0;
-  }
-  ~StorageEntry() { NT_DisposeValue(&value); }
+  StorageEntry() { m_flags = 0; }
 
-  NT_Value value;
-  unsigned int flags;
+  Value& value() { return m_value; }
+  const Value& value() const { return m_value; }
+
+  unsigned int flags() const { return m_flags; }
+  void set_flags(unsigned int flags) { m_flags = flags; }
+  bool IsPersistent() const { return (m_flags & NT_PERSISTENT) != 0; }
 
   StorageEntry(const StorageEntry&) = delete;
   StorageEntry& operator=(const StorageEntry&) = delete;
+
+private:
+  Value m_value;
+  unsigned int m_flags;
 };
 
 class Storage {
@@ -49,7 +50,7 @@ class Storage {
   const EntriesMap& entries() const { return m_entries; }
 
   void SavePersistent(std::ostream& os) const;
-  void LoadPersistent(std::istream& is,
+  bool LoadPersistent(std::istream& is,
                       void (*warn)(std::size_t line, const char* msg));
 
  private:

src/Value.cpp

@@ -12,9 +12,9 @@
 
 #include "ntcore.h"
 
-bool operator==(const NT_Value& lhs, const NT_Value& rhs) {
-  if (lhs.type != rhs.type) return false;
-  switch (lhs.type) {
+bool ntimpl::operator==(const Value& lhs, const Value& rhs) {
+  if (lhs.type() != rhs.type()) return false;
+  switch (lhs.type()) {
     case NT_UNASSIGNED:
       return true;  // XXX: is this better being false instead?
     case NT_BOOLEAN:

src/Value.h

@@ -8,10 +8,99 @@
 #ifndef NT_VALUE_H_
 #define NT_VALUE_H_
 
-struct NT_Value;
+#include <cassert>
 
-namespace ntimpl {}  // namespace ntimpl
+#include "ntcore.h"
 
-bool operator==(const NT_Value& lhs, const NT_Value& rhs);
+#include "llvm/ArrayRef.h"
+#include "llvm/StringRef.h"
+
+namespace ntimpl {
+
+class Storage;
+class Value;
+
+class StringValue : private NT_String {
+  friend class Value;
+ public:
+  StringValue() { NT_InitString(this); }
+  ~StringValue() { NT_DisposeString(this); }
+
+  operator llvm::StringRef() const { return llvm::StringRef(str, len); }
+};
+
+class Value : private NT_Value {
+  friend class Storage;
+ public:
+  Value() { NT_InitValue(this); }
+  ~Value() { NT_DisposeValue(this); }
+
+  // A little ugly, as it hides the identically-named NT_Value::type.
+  NT_Type type() const { return NT_Value::type; }
+
+  /*
+   * Type-Safe Getters
+   */
+  bool GetBoolean() const {
+    assert(NT_Value::type == NT_BOOLEAN);
+    return data.v_boolean;
+  }
+  double GetDouble() const {
+    assert(NT_Value::type == NT_DOUBLE);
+    return data.v_double;
+  }
+  llvm::StringRef GetString() const {
+    assert(NT_Value::type == NT_STRING);
+    return static_cast<const StringValue&>(data.v_string);
+  }
+  llvm::StringRef GetRaw() const {
+    assert(NT_Value::type == NT_RAW);
+    return static_cast<const StringValue&>(data.v_raw);
+  }
+  // Ideally this would return llvm::ArrayRef<bool> but the C headers must
+  // use "int" and casting may be very unsafe.
+  llvm::ArrayRef<int> GetBooleanArray() const {
+    assert(NT_Value::type == NT_BOOLEAN_ARRAY);
+    return llvm::ArrayRef<int>(data.arr_boolean.arr, data.arr_boolean.size);
+  }
+  llvm::ArrayRef<double> GetDoubleArray() const {
+    assert(NT_Value::type == NT_DOUBLE_ARRAY);
+    return llvm::ArrayRef<double>(data.arr_double.arr, data.arr_double.size);
+  }
+  llvm::ArrayRef<StringValue> GetStringArray() const {
+    assert(NT_Value::type == NT_BOOLEAN_ARRAY);
+    return llvm::ArrayRef<StringValue>(
+        static_cast<StringValue*>(data.arr_string.arr), data.arr_string.size);
+  }
+
+  /*
+   * Type-Safe Setters
+   */
+  void SetBoolean(bool value);
+  void SetDouble(double value);
+  void SetString(llvm::StringRef value);
+  void SetRaw(llvm::StringRef value);
+
+  template<typename It>
+  void SetBooleanArray(It begin, It end) {
+  }
+
+  template<typename It>
+  void SetDoubleArray(It begin, It end) {
+  }
+
+  template<typename It>
+  void SetStringArray(It begin, It end) {
+  }
+
+  Value(const Value&) = delete;
+  Value& operator=(const Value&) = delete;
+
+  friend bool operator==(const Value& lhs, const Value& rhs);
+};
+
+bool operator==(const Value& lhs, const Value& rhs);
+
+}  // namespace ntimpl
 
 #endif  // NT_VALUE_H_

src/ntcore.cpp

@@ -108,7 +108,7 @@ const char *NT_LoadPersistent(const char *filename,
   Storage& storage = Storage::GetInstance();
   std::ifstream is(filename);
   if (!is) return "could not open file";
-  storage.LoadPersistent(is, warn);
+  if (!storage.LoadPersistent(is, warn)) return "error reading file";
   return nullptr;
 }