Change StringBuilder::save() to take a VariantData*

Author: bblanchon

extras/tests/ResourceManager/StringBuilder.cpp

@@ -6,6 +6,7 @@
 #include <catch.hpp>
 
 #include "Allocators.hpp"
+#include "Literals.hpp"
 
 using namespace ArduinoJson::detail;
 
@@ -16,9 +17,10 @@ TEST_CASE("StringBuilder") {
 
   SECTION("Empty string") {
     StringBuilder str(&resources);
+    VariantData data;
 
     str.startString();
-    str.save();
+    str.save(&data);
 
     REQUIRE(resources.size() == sizeofString(""));
     REQUIRE(resources.overflowed() == false);
@@ -96,48 +98,69 @@ TEST_CASE("StringBuilder") {
   }
 }
 
-static StringNode* addStringToPool(ResourceManager& resources, const char* s) {
-  StringBuilder str(&resources);
-  str.startString();
-  str.append(s);
-  return str.save();
+static const char* saveString(StringBuilder& builder, const char* s) {
+  VariantData data;
+  builder.startString();
+  builder.append(s);
+  builder.save(&data);
+  return data.asString().c_str();
 }
 
 TEST_CASE("StringBuilder::save() deduplicates strings") {
-  ResourceManager resources;
+  SpyingAllocator spy;
+  ResourceManager resources(&spy);
+  StringBuilder builder(&resources);
 
   SECTION("Basic") {
-    auto s1 = addStringToPool(resources, "hello");
-    auto s2 = addStringToPool(resources, "world");
-    auto s3 = addStringToPool(resources, "hello");
-
-    REQUIRE(s1 == s3);
-    REQUIRE(s2 != s3);
-    REQUIRE(s1->references == 2);
-    REQUIRE(s2->references == 1);
-    REQUIRE(s3->references == 2);
-    REQUIRE(resources.size() == sizeofString("hello") + sizeofString("world"));
+    auto s1 = saveString(builder, "hello");
+    auto s2 = saveString(builder, "world");
+    auto s3 = saveString(builder, "hello");
+
+    REQUIRE(s1 == "hello"_s);
+    REQUIRE(s2 == "world"_s);
+    REQUIRE(+s1 == +s3);  // same address
+
+    REQUIRE(spy.log() ==
+            AllocatorLog{
+                Allocate(sizeofStringBuffer()),
+                Reallocate(sizeofStringBuffer(), sizeofString("hello")),
+                Allocate(sizeofStringBuffer()),
+                Reallocate(sizeofStringBuffer(), sizeofString("world")),
+                Allocate(sizeofStringBuffer()),
+            });
   }
 
   SECTION("Requires terminator") {
-    auto s1 = addStringToPool(resources, "hello world");
-    auto s2 = addStringToPool(resources, "hello");
+    auto s1 = saveString(builder, "hello world");
+    auto s2 = saveString(builder, "hello");
 
-    REQUIRE(s2 != s1);
-    REQUIRE(s1->references == 1);
-    REQUIRE(s2->references == 1);
-    REQUIRE(resources.size() ==
-            sizeofString("hello world") + sizeofString("hello"));
+    REQUIRE(s1 == "hello world"_s);
+    REQUIRE(s2 == "hello"_s);
+    REQUIRE(+s2 != +s1);  // different address
+
+    REQUIRE(spy.log() ==
+            AllocatorLog{
+                Allocate(sizeofStringBuffer()),
+                Reallocate(sizeofStringBuffer(), sizeofString("hello world")),
+                Allocate(sizeofStringBuffer()),
+                Reallocate(sizeofStringBuffer(), sizeofString("hello")),
+            });
   }
 
   SECTION("Don't overrun") {
-    auto s1 = addStringToPool(resources, "hello world");
-    auto s2 = addStringToPool(resources, "wor");
+    auto s1 = saveString(builder, "hello world");
+    auto s2 = saveString(builder, "wor");
 
+    REQUIRE(s1 == "hello world"_s);
+    REQUIRE(s2 == "wor"_s);
     REQUIRE(s2 != s1);
-    REQUIRE(s1->references == 1);
-    REQUIRE(s2->references == 1);
-    REQUIRE(resources.size() ==
-            sizeofString("hello world") + sizeofString("wor"));
+
+    REQUIRE(spy.log() ==
+            AllocatorLog{
+                Allocate(sizeofStringBuffer()),
+                Reallocate(sizeofStringBuffer(), sizeofString("hello world")),
+                Allocate(sizeofStringBuffer()),
+                Reallocate(sizeofStringBuffer(), sizeofString("wor")),
+            });
   }
 }

src/ArduinoJson/Json/JsonDeserializer.hpp

@@ -279,7 +279,7 @@ class JsonDeserializer {
           if (!keyVariant)
             return DeserializationError::NoMemory;
 
-          keyVariant->setOwnedString(stringBuilder_.save());
+          stringBuilder_.save(keyVariant);
         } else {
           member->clear(resources_);
         }
@@ -388,7 +388,7 @@ class JsonDeserializer {
     if (err)
       return err;
 
-    variant.setOwnedString(stringBuilder_.save());
+    stringBuilder_.save(&variant);
 
     return DeserializationError::Ok;
   }

src/ArduinoJson/Memory/StringBuilder.hpp

@@ -25,7 +25,8 @@ class StringBuilder {
       node_ = resources_->createString(initialCapacity);
   }
 
-  StringNode* save() {
+  void save(VariantData* variant) {
+    ARDUINOJSON_ASSERT(variant != nullptr);
     ARDUINOJSON_ASSERT(node_ != nullptr);
     node_->data[size_] = 0;
     StringNode* node = resources_->getString(adaptString(node_->data, size_));
@@ -37,7 +38,7 @@ class StringBuilder {
     } else {
       node->references++;
     }
-    return node;
+    variant->setOwnedString(node);
   }
 
   void append(const char* s) {

src/ArduinoJson/Variant/ConverterImpl.hpp

@@ -230,9 +230,9 @@ class StringBuilderPrint : public Print {
     copier_.startString();
   }
 
-  StringNode* save() {
+  void save(VariantData* data) {
     ARDUINOJSON_ASSERT(!overflowed());
-    return copier_.save();
+    copier_.save(data);
   }
 
   size_t write(uint8_t c) {
@@ -268,7 +268,7 @@ inline void convertToJson(const ::Printable& src, JsonVariant dst) {
   src.printTo(print);
   if (print.overflowed())
     return;
-  data->setOwnedString(print.save());
+  print.save(data);
 }
 
 #endif