From 05b68fc7cc9d31bc83d9966be7884c650e0d5395 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Fri, 28 Feb 2025 09:59:46 +0100 Subject: [PATCH] Change `StringBuilder::save()` to take a `VariantData*` --- .../tests/ResourceManager/StringBuilder.cpp | 81 ++++++++++++------- src/ArduinoJson/Json/JsonDeserializer.hpp | 4 +- src/ArduinoJson/Memory/StringBuilder.hpp | 5 +- src/ArduinoJson/Variant/ConverterImpl.hpp | 6 +- 4 files changed, 60 insertions(+), 36 deletions(-) diff --git a/extras/tests/ResourceManager/StringBuilder.cpp b/extras/tests/ResourceManager/StringBuilder.cpp index 0c1b1c16..9a122619 100644 --- a/extras/tests/ResourceManager/StringBuilder.cpp +++ b/extras/tests/ResourceManager/StringBuilder.cpp @@ -6,6 +6,7 @@ #include #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"); + auto s1 = saveString(builder, "hello"); + auto s2 = saveString(builder, "world"); + auto s3 = saveString(builder, "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")); + 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")), + }); } } diff --git a/src/ArduinoJson/Json/JsonDeserializer.hpp b/src/ArduinoJson/Json/JsonDeserializer.hpp index 38b49359..288a22ca 100644 --- a/src/ArduinoJson/Json/JsonDeserializer.hpp +++ b/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; } diff --git a/src/ArduinoJson/Memory/StringBuilder.hpp b/src/ArduinoJson/Memory/StringBuilder.hpp index f503d8a1..05c9a920 100644 --- a/src/ArduinoJson/Memory/StringBuilder.hpp +++ b/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) { diff --git a/src/ArduinoJson/Variant/ConverterImpl.hpp b/src/ArduinoJson/Variant/ConverterImpl.hpp index dddc6fa1..6c5fcda8 100644 --- a/src/ArduinoJson/Variant/ConverterImpl.hpp +++ b/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