From 806fa907ab5ebba773fc4ba0277208aeb74fc640 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Tue, 2 May 2023 09:36:40 +0200 Subject: [PATCH] Always store `serialized("string")` by copy (#1915) --- CHANGELOG.md | 1 + extras/tests/JsonArray/add.cpp | 4 ++-- extras/tests/JsonDocument/shrinkToFit.cpp | 11 ----------- extras/tests/JsonVariant/copy.cpp | 11 ++++++----- extras/tests/JsonVariant/set.cpp | 2 +- src/ArduinoJson/Variant/ConverterImpl.hpp | 13 +------------ src/ArduinoJson/Variant/VariantContent.hpp | 1 - src/ArduinoJson/Variant/VariantData.hpp | 7 ------- src/ArduinoJson/Variant/VariantFunctions.hpp | 12 ------------ src/ArduinoJson/Variant/VariantImpl.hpp | 3 --- 10 files changed, 11 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8e10dd8..bf73d288 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,3 +13,4 @@ HEAD * Remove `JsonDocument::capacity()` * Store the strings in the heap * Reference-count shared strings +* Always store `serialized("string")` by copy (#1915) diff --git a/extras/tests/JsonArray/add.cpp b/extras/tests/JsonArray/add.cpp index 22a63ee8..68ee2e42 100644 --- a/extras/tests/JsonArray/add.cpp +++ b/extras/tests/JsonArray/add.cpp @@ -115,9 +115,9 @@ TEST_CASE("JsonArray::add()") { REQUIRE(expectedSize == doc.memoryUsage()); } - SECTION("should not duplicate serialized(const char*)") { + SECTION("should duplicate serialized(const char*)") { array.add(serialized("{}")); - const size_t expectedSize = sizeofArray(1); + const size_t expectedSize = sizeofArray(1) + sizeofString(2); REQUIRE(expectedSize == doc.memoryUsage()); } diff --git a/extras/tests/JsonDocument/shrinkToFit.cpp b/extras/tests/JsonDocument/shrinkToFit.cpp index e10dacab..1117c9c4 100644 --- a/extras/tests/JsonDocument/shrinkToFit.cpp +++ b/extras/tests/JsonDocument/shrinkToFit.cpp @@ -97,17 +97,6 @@ TEST_CASE("JsonDocument::shrinkToFit()") { << AllocatorLog::Reallocate(4096, 0)); } - SECTION("linked raw") { - doc.set(serialized("[{},123]")); - - doc.shrinkToFit(); - - REQUIRE(doc.as() == "[{},123]"); - REQUIRE(spyingAllocator.log() == AllocatorLog() - << AllocatorLog::Allocate(4096) - << AllocatorLog::Reallocate(4096, 0)); - } - SECTION("owned raw") { doc.set(serialized(std::string("[{},12]"))); diff --git a/extras/tests/JsonVariant/copy.cpp b/extras/tests/JsonVariant/copy.cpp index 636fe1b6..0cd253c4 100644 --- a/extras/tests/JsonVariant/copy.cpp +++ b/extras/tests/JsonVariant/copy.cpp @@ -89,15 +89,16 @@ TEST_CASE("JsonVariant::set(JsonVariant)") { AllocatorLog() << AllocatorLog::Allocate(sizeofString((7)))); } - SECTION("stores Serialized by reference") { - var1.set(serialized("hello!!", 8)); + SECTION("stores Serialized by copy") { + var1.set(serialized("hello!!", 7)); spyingAllocator.clearLog(); var2.set(var1); - REQUIRE(doc1.memoryUsage() == 0); - REQUIRE(doc2.memoryUsage() == 0); - REQUIRE(spyingAllocator.log() == AllocatorLog()); + REQUIRE(doc1.memoryUsage() == sizeofString(7)); + REQUIRE(doc2.memoryUsage() == sizeofString(7)); + REQUIRE(spyingAllocator.log() == + AllocatorLog() << AllocatorLog::Allocate(sizeofString((7)))); } SECTION("stores Serialized by copy") { diff --git a/extras/tests/JsonVariant/set.cpp b/extras/tests/JsonVariant/set.cpp index 17bed43e..4484760e 100644 --- a/extras/tests/JsonVariant/set.cpp +++ b/extras/tests/JsonVariant/set.cpp @@ -205,6 +205,6 @@ TEST_CASE("JsonVariant::set() releases the previous value") { SECTION("Serialized") { v.set(serialized("[]")); - REQUIRE(doc.memoryUsage() == sizeofObject(1)); + REQUIRE(doc.memoryUsage() == sizeofObject(1) + sizeofString(2)); } } diff --git a/src/ArduinoJson/Variant/ConverterImpl.hpp b/src/ArduinoJson/Variant/ConverterImpl.hpp index c94387e3..fad2fd95 100644 --- a/src/ArduinoJson/Variant/ConverterImpl.hpp +++ b/src/ArduinoJson/Variant/ConverterImpl.hpp @@ -155,22 +155,11 @@ convertToJson(const T& src, JsonVariant dst) { variantSetString(data, adaptString(src), pool); } -template <> -struct Converter> - : private detail::VariantAttorney { - static void toJson(SerializedValue src, JsonVariant dst) { - variantSetLinkedRaw(getData(dst), src, getPool(dst)); - } -}; - // SerializedValue // SerializedValue // SerializedValue template -struct Converter< - SerializedValue, - typename detail::enable_if::value>::type> - : private detail::VariantAttorney { +struct Converter> : private detail::VariantAttorney { static void toJson(SerializedValue src, JsonVariant dst) { variantSetOwnedRaw(getData(dst), src, getPool(dst)); } diff --git a/src/ArduinoJson/Variant/VariantContent.hpp b/src/ArduinoJson/Variant/VariantContent.hpp index 22239019..e6d925a1 100644 --- a/src/ArduinoJson/Variant/VariantContent.hpp +++ b/src/ArduinoJson/Variant/VariantContent.hpp @@ -17,7 +17,6 @@ enum { OWNED_VALUE_BIT = 0x01, VALUE_IS_NULL = 0, - VALUE_IS_LINKED_RAW = 0x02, VALUE_IS_OWNED_RAW = 0x03, VALUE_IS_LINKED_STRING = 0x04, VALUE_IS_OWNED_STRING = 0x05, diff --git a/src/ArduinoJson/Variant/VariantData.hpp b/src/ArduinoJson/Variant/VariantData.hpp index 7685c60e..c2af860b 100644 --- a/src/ArduinoJson/Variant/VariantData.hpp +++ b/src/ArduinoJson/Variant/VariantData.hpp @@ -46,7 +46,6 @@ class VariantData { content_.asString.size); case VALUE_IS_OWNED_RAW: - case VALUE_IS_LINKED_RAW: return visitor.visitRawJson(content_.asString.data, content_.asString.size); @@ -158,12 +157,6 @@ class VariantData { content_.asFloat = value; } - void setLinkedRaw(const char* data, size_t n) { - setType(VALUE_IS_LINKED_RAW); - content_.asString.data = data; - content_.asString.size = n; - } - void setOwnedRaw(const char* data, size_t n) { setType(VALUE_IS_OWNED_RAW); content_.asString.data = data; diff --git a/src/ArduinoJson/Variant/VariantFunctions.hpp b/src/ArduinoJson/Variant/VariantFunctions.hpp index 8e1fafed..5c4a0cdc 100644 --- a/src/ArduinoJson/Variant/VariantFunctions.hpp +++ b/src/ArduinoJson/Variant/VariantFunctions.hpp @@ -135,18 +135,6 @@ inline void variantSetOwnedRaw(VariantData* var, SerializedValue value, var->setNull(); } -inline void variantSetLinkedRaw(VariantData* var, - SerializedValue value, - MemoryPool* pool) { - if (!var) - return; - variantRelease(var, pool); - if (value.data()) - var->setLinkedRaw(value.data(), value.size()); - else - var->setNull(); -} - inline size_t variantSize(const VariantData* var) { return var != 0 ? var->size() : 0; } diff --git a/src/ArduinoJson/Variant/VariantImpl.hpp b/src/ArduinoJson/Variant/VariantImpl.hpp index 0d55dff8..9bdc71ad 100644 --- a/src/ArduinoJson/Variant/VariantImpl.hpp +++ b/src/ArduinoJson/Variant/VariantImpl.hpp @@ -85,9 +85,6 @@ inline JsonString VariantData::asString() const { inline JsonString VariantData::asRaw() const { switch (type()) { - case VALUE_IS_LINKED_RAW: - return JsonString(content_.asString.data, content_.asString.size, - JsonString::Linked); case VALUE_IS_OWNED_RAW: return JsonString(content_.asString.data, content_.asString.size, JsonString::Copied);