From dddc4912c4cfe70cc4f24f4c7968acdbac6004d5 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Thu, 28 Aug 2025 10:04:11 +0200 Subject: [PATCH] Don't store string literals by pointer anymore Fixes #2189 --- CHANGELOG.md | 19 ++++ extras/tests/Deprecated/BasicJsonDocument.cpp | 2 +- extras/tests/JsonArray/add.cpp | 1 + .../JsonDeserializer/destination_types.cpp | 4 +- extras/tests/JsonDocument/ElementProxy.cpp | 1 + extras/tests/JsonDocument/MemberProxy.cpp | 6 ++ extras/tests/JsonDocument/add.cpp | 1 + extras/tests/JsonDocument/constructor.cpp | 3 + extras/tests/JsonDocument/set.cpp | 4 +- extras/tests/JsonDocument/shrinkToFit.cpp | 58 +---------- extras/tests/JsonDocument/subscript.cpp | 1 + extras/tests/JsonObject/set.cpp | 32 +----- extras/tests/JsonObject/subscript.cpp | 64 +----------- extras/tests/JsonVariant/as.cpp | 3 - extras/tests/JsonVariant/copy.cpp | 6 +- extras/tests/JsonVariant/set.cpp | 22 ++--- extras/tests/Misc/JsonString.cpp | 2 - extras/tests/Misc/StringAdapters.cpp | 23 +---- .../string_length_size_4.cpp | 99 +++++++++++-------- .../MsgPackDeserializer/destination_types.cpp | 4 +- .../MsgPackSerializer/serializeVariant.cpp | 5 +- extras/tests/ResourceManager/StringBuffer.cpp | 4 +- src/ArduinoJson/Memory/StringBuffer.hpp | 2 +- src/ArduinoJson/Memory/StringBuilder.hpp | 2 +- .../Strings/Adapters/FlashString.hpp | 4 - .../Strings/Adapters/RamString.hpp | 22 +---- src/ArduinoJson/Strings/JsonString.hpp | 24 +++-- src/ArduinoJson/Variant/VariantContent.hpp | 20 ++-- src/ArduinoJson/Variant/VariantData.hpp | 58 ++++------- src/ArduinoJson/Variant/VariantImpl.hpp | 9 +- 30 files changed, 174 insertions(+), 331 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0a7a6f6..9510a95c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,25 @@ ArduinoJson: change log ======================= +HEAD +---- + +* Don't store string literals by pointer anymore (issue #2189) + Version 7.3 introduced a new way to detect string literals, but it fails in some edge cases. + I could not find a way to fix it, so I chose to remove the optimization rather than keep it broken. + +> ### BREAKING CHANGES +> +> Since version 7.3, you could pass a boolean to `JsonString`'s constructor to force the string to be stored by pointer. +> This optimization has been removed, and you'll get a deprecation warning if you use it. +> To fix the issue, you must remove the boolean argument from the constructor, or better yet, remove `JsonString` altogether. +> +> ```diff +> char name[] = "ArduinoJson"; +> - doc["name"] = JsonString(name, true); +> + doc["name"] = name; +> ``` + v7.4.2 (2025-06-20) ------ diff --git a/extras/tests/Deprecated/BasicJsonDocument.cpp b/extras/tests/Deprecated/BasicJsonDocument.cpp index a3509779..d6861b82 100644 --- a/extras/tests/Deprecated/BasicJsonDocument.cpp +++ b/extras/tests/Deprecated/BasicJsonDocument.cpp @@ -54,7 +54,7 @@ TEST_CASE("BasicJsonDocument") { doc["hello"] = "world"; auto copy = doc; REQUIRE(copy.as() == "{\"hello\":\"world\"}"); - REQUIRE(allocatorLog == "AA"); + REQUIRE(allocatorLog == "AAAAAA"); } SECTION("capacity") { diff --git a/extras/tests/JsonArray/add.cpp b/extras/tests/JsonArray/add.cpp index 0983e3bd..78ff9011 100644 --- a/extras/tests/JsonArray/add.cpp +++ b/extras/tests/JsonArray/add.cpp @@ -56,6 +56,7 @@ TEST_CASE("JsonArray::add(T)") { REQUIRE(array[0].is() == false); REQUIRE(spy.log() == AllocatorLog{ Allocate(sizeofPool()), + Allocate(sizeofString("hello")), }); } diff --git a/extras/tests/JsonDeserializer/destination_types.cpp b/extras/tests/JsonDeserializer/destination_types.cpp index 0f96b23e..72584a06 100644 --- a/extras/tests/JsonDeserializer/destination_types.cpp +++ b/extras/tests/JsonDeserializer/destination_types.cpp @@ -104,6 +104,8 @@ TEST_CASE("deserializeJson(MemberProxy)") { REQUIRE(err == DeserializationError::Ok); REQUIRE(doc.as() == "{\"hello\":\"world\",\"value\":[42]}"); - REQUIRE(spy.log() == AllocatorLog{}); + REQUIRE(spy.log() == AllocatorLog{ + Allocate(sizeofString("value")), + }); } } diff --git a/extras/tests/JsonDocument/ElementProxy.cpp b/extras/tests/JsonDocument/ElementProxy.cpp index 9cf88551..3c1fb50c 100644 --- a/extras/tests/JsonDocument/ElementProxy.cpp +++ b/extras/tests/JsonDocument/ElementProxy.cpp @@ -31,6 +31,7 @@ TEST_CASE("ElementProxy::add()") { REQUIRE(doc.as() == "[[\"world\"]]"); REQUIRE(spy.log() == AllocatorLog{ Allocate(sizeofPool()), + Allocate(sizeofString("world")), }); } diff --git a/extras/tests/JsonDocument/MemberProxy.cpp b/extras/tests/JsonDocument/MemberProxy.cpp index c42edcc1..da9c24d7 100644 --- a/extras/tests/JsonDocument/MemberProxy.cpp +++ b/extras/tests/JsonDocument/MemberProxy.cpp @@ -25,6 +25,7 @@ TEST_CASE("MemberProxy::add()") { REQUIRE(doc.as() == "{\"hello\":[42]}"); REQUIRE(spy.log() == AllocatorLog{ Allocate(sizeofPool()), + Allocate(sizeofString("hello")), }); } @@ -34,6 +35,8 @@ TEST_CASE("MemberProxy::add()") { REQUIRE(doc.as() == "{\"hello\":[\"world\"]}"); REQUIRE(spy.log() == AllocatorLog{ Allocate(sizeofPool()), + Allocate(sizeofString("hello")), + Allocate(sizeofString("world")), }); } @@ -44,6 +47,7 @@ TEST_CASE("MemberProxy::add()") { REQUIRE(doc.as() == "{\"hello\":[\"world\"]}"); REQUIRE(spy.log() == AllocatorLog{ Allocate(sizeofPool()), + Allocate(sizeofString("hello")), Allocate(sizeofString("world")), }); } @@ -55,6 +59,7 @@ TEST_CASE("MemberProxy::add()") { REQUIRE(doc.as() == "{\"hello\":[\"world\"]}"); REQUIRE(spy.log() == AllocatorLog{ Allocate(sizeofPool()), + Allocate(sizeofString("hello")), Allocate(sizeofString("world")), }); @@ -71,6 +76,7 @@ TEST_CASE("MemberProxy::add()") { REQUIRE(doc.as() == "{\"hello\":[\"world\"]}"); REQUIRE(spy.log() == AllocatorLog{ Allocate(sizeofPool()), + Allocate(sizeofString("hello")), Allocate(sizeofString("world")), }); } diff --git a/extras/tests/JsonDocument/add.cpp b/extras/tests/JsonDocument/add.cpp index da898e66..3e3ccb61 100644 --- a/extras/tests/JsonDocument/add.cpp +++ b/extras/tests/JsonDocument/add.cpp @@ -32,6 +32,7 @@ TEST_CASE("JsonDocument::add(T)") { REQUIRE(doc.as() == "[\"hello\"]"); REQUIRE(spy.log() == AllocatorLog{ Allocate(sizeofPool()), + Allocate(sizeofString("hello")), }); } diff --git a/extras/tests/JsonDocument/constructor.cpp b/extras/tests/JsonDocument/constructor.cpp index 371a2561..ee7c3f8e 100644 --- a/extras/tests/JsonDocument/constructor.cpp +++ b/extras/tests/JsonDocument/constructor.cpp @@ -64,6 +64,8 @@ TEST_CASE("JsonDocument constructor") { REQUIRE(doc2.as() == "{\"hello\":\"world\"}"); REQUIRE(spyingAllocator.log() == AllocatorLog{ Allocate(sizeofPool()), + Allocate(sizeofString("hello")), + Allocate(sizeofString("world")), }); } @@ -87,6 +89,7 @@ TEST_CASE("JsonDocument constructor") { REQUIRE(doc2.as() == "[\"hello\"]"); REQUIRE(spyingAllocator.log() == AllocatorLog{ Allocate(sizeofPool()), + Allocate(sizeofString("hello")), }); } diff --git a/extras/tests/JsonDocument/set.cpp b/extras/tests/JsonDocument/set.cpp index 88c33bbc..c11b6a4d 100644 --- a/extras/tests/JsonDocument/set.cpp +++ b/extras/tests/JsonDocument/set.cpp @@ -37,7 +37,9 @@ TEST_CASE("JsonDocument::set()") { doc.set("example"); REQUIRE(doc.as() == "example"_s); - REQUIRE(spy.log() == AllocatorLog{}); + REQUIRE(spy.log() == AllocatorLog{ + Allocate(sizeofString("example")), + }); } SECTION("const char*") { diff --git a/extras/tests/JsonDocument/shrinkToFit.cpp b/extras/tests/JsonDocument/shrinkToFit.cpp index 148b552d..eb04c446 100644 --- a/extras/tests/JsonDocument/shrinkToFit.cpp +++ b/extras/tests/JsonDocument/shrinkToFit.cpp @@ -69,17 +69,8 @@ TEST_CASE("JsonDocument::shrinkToFit()") { REQUIRE(spyingAllocator.log() == AllocatorLog{}); } - SECTION("linked string") { - doc.set("hello"); - - doc.shrinkToFit(); - - REQUIRE(doc.as() == "hello"); - REQUIRE(spyingAllocator.log() == AllocatorLog{}); - } - - SECTION("owned string") { - doc.set("abcdefg"_s); + SECTION("string") { + doc.set("abcdefg"); REQUIRE(doc.as() == "abcdefg"); doc.shrinkToFit(); @@ -101,20 +92,7 @@ TEST_CASE("JsonDocument::shrinkToFit()") { }); } - SECTION("linked key") { - doc["key"] = 42; - - doc.shrinkToFit(); - - REQUIRE(doc.as() == "{\"key\":42}"); - REQUIRE(spyingAllocator.log() == - AllocatorLog{ - Allocate(sizeofPool()), - Reallocate(sizeofPool(), sizeofObject(1)), - }); - } - - SECTION("owned key") { + SECTION("object key") { doc["abcdefg"_s] = 42; doc.shrinkToFit(); @@ -128,20 +106,7 @@ TEST_CASE("JsonDocument::shrinkToFit()") { }); } - SECTION("linked string in array") { - doc.add("hello"); - - doc.shrinkToFit(); - - REQUIRE(doc.as() == "[\"hello\"]"); - REQUIRE(spyingAllocator.log() == - AllocatorLog{ - Allocate(sizeofPool()), - Reallocate(sizeofPool(), sizeofArray(1)), - }); - } - - SECTION("owned string in array") { + SECTION("string in array") { doc.add("abcdefg"_s); doc.shrinkToFit(); @@ -155,20 +120,7 @@ TEST_CASE("JsonDocument::shrinkToFit()") { }); } - SECTION("linked string in object") { - doc["key"] = "hello"; - - doc.shrinkToFit(); - - REQUIRE(doc.as() == "{\"key\":\"hello\"}"); - REQUIRE(spyingAllocator.log() == - AllocatorLog{ - Allocate(sizeofPool()), - Reallocate(sizeofPool(), sizeofObject(1)), - }); - } - - SECTION("owned string in object") { + SECTION("string in object") { doc["key"] = "abcdefg"_s; doc.shrinkToFit(); diff --git a/extras/tests/JsonDocument/subscript.cpp b/extras/tests/JsonDocument/subscript.cpp index 17bdcc2e..aec3bc92 100644 --- a/extras/tests/JsonDocument/subscript.cpp +++ b/extras/tests/JsonDocument/subscript.cpp @@ -114,6 +114,7 @@ TEST_CASE("JsonDocument::operator[] key storage") { REQUIRE(doc.as() == "{\"hello\":0}"); REQUIRE(spy.log() == AllocatorLog{ Allocate(sizeofPool()), + Allocate(sizeofString("hello")), }); } diff --git a/extras/tests/JsonObject/set.cpp b/extras/tests/JsonObject/set.cpp index e5c3044d..56898bf3 100644 --- a/extras/tests/JsonObject/set.cpp +++ b/extras/tests/JsonObject/set.cpp @@ -16,44 +16,18 @@ TEST_CASE("JsonObject::set()") { JsonObject obj1 = doc1.to(); JsonObject obj2 = doc2.to(); - SECTION("doesn't copy static string in key or value") { + SECTION("copy key and string value") { obj1["hello"] = "world"; spy.clearLog(); bool success = obj2.set(obj1); - REQUIRE(success == true); - REQUIRE(obj2["hello"] == "world"_s); - REQUIRE(spy.log() == AllocatorLog{ - Allocate(sizeofPool()), - }); - } - - SECTION("copy local string value") { - obj1["hello"] = "world"_s; - spy.clearLog(); - - bool success = obj2.set(obj1); - - REQUIRE(success == true); - REQUIRE(obj2["hello"] == "world"_s); - REQUIRE(spy.log() == AllocatorLog{ - Allocate(sizeofPool()), - Allocate(sizeofString("world")), - }); - } - - SECTION("copy local key") { - obj1["hello"_s] = "world"; - spy.clearLog(); - - bool success = obj2.set(obj1); - REQUIRE(success == true); REQUIRE(obj2["hello"] == "world"_s); REQUIRE(spy.log() == AllocatorLog{ Allocate(sizeofPool()), Allocate(sizeofString("hello")), + Allocate(sizeofString("world")), }); } @@ -110,7 +84,7 @@ TEST_CASE("JsonObject::set()") { } SECTION("copy fails in the middle of an array") { - TimebombAllocator timebomb(1); + TimebombAllocator timebomb(2); JsonDocument doc3(&timebomb); JsonObject obj3 = doc3.to(); diff --git a/extras/tests/JsonObject/subscript.cpp b/extras/tests/JsonObject/subscript.cpp index bdf900f9..85b37a43 100644 --- a/extras/tests/JsonObject/subscript.cpp +++ b/extras/tests/JsonObject/subscript.cpp @@ -101,30 +101,8 @@ TEST_CASE("JsonObject::operator[]") { obj[key] = 42; REQUIRE(42 == obj[key]); } - - SECTION("should not duplicate const char*") { + SECTION("should duplicate key and value strings") { obj["hello"] = "world"; - REQUIRE(spy.log() == AllocatorLog{Allocate(sizeofPool())}); - } - - SECTION("should duplicate char* value") { - obj["hello"] = const_cast("world"); - REQUIRE(spy.log() == AllocatorLog{ - Allocate(sizeofPool()), - Allocate(sizeofString("world")), - }); - } - - SECTION("should duplicate char* key") { - obj[const_cast("hello")] = "world"; - REQUIRE(spy.log() == AllocatorLog{ - Allocate(sizeofPool()), - Allocate(sizeofString("hello")), - }); - } - - SECTION("should duplicate char* key&value") { - obj[const_cast("hello")] = const_cast("world"); REQUIRE(spy.log() == AllocatorLog{ Allocate(sizeofPool()), Allocate(sizeofString("hello")), @@ -132,46 +110,6 @@ TEST_CASE("JsonObject::operator[]") { }); } - SECTION("should duplicate std::string value") { - obj["hello"] = "world"_s; - REQUIRE(spy.log() == AllocatorLog{ - Allocate(sizeofPool()), - Allocate(sizeofString("world")), - }); - } - - SECTION("should duplicate std::string key") { - obj["hello"_s] = "world"; - REQUIRE(spy.log() == AllocatorLog{ - Allocate(sizeofPool()), - Allocate(sizeofString("hello")), - }); - } - - SECTION("should duplicate std::string key&value") { - obj["hello"_s] = "world"_s; - REQUIRE(spy.log() == AllocatorLog{ - Allocate(sizeofPool()), - Allocate(sizeofString("hello")), - Allocate(sizeofString("world")), - }); - } - - SECTION("should duplicate a non-static JsonString key") { - obj[JsonString("hello", false)] = "world"; - REQUIRE(spy.log() == AllocatorLog{ - Allocate(sizeofPool()), - Allocate(sizeofString("hello")), - }); - } - - SECTION("should not duplicate a static JsonString key") { - obj[JsonString("hello", true)] = "world"; - REQUIRE(spy.log() == AllocatorLog{ - Allocate(sizeofPool()), - }); - } - SECTION("should ignore null key") { // object must have a value to make a call to strcmp() obj["dummy"] = 42; diff --git a/extras/tests/JsonVariant/as.cpp b/extras/tests/JsonVariant/as.cpp index b1235725..69009f04 100644 --- a/extras/tests/JsonVariant/as.cpp +++ b/extras/tests/JsonVariant/as.cpp @@ -185,7 +185,6 @@ TEST_CASE("JsonVariant::as()") { REQUIRE(variant.as() == 42L); REQUIRE(variant.as() == 42); REQUIRE(variant.as() == "42"); - REQUIRE(variant.as().isStatic() == true); } SECTION("set(\"hello\")") { @@ -208,7 +207,6 @@ TEST_CASE("JsonVariant::as()") { REQUIRE(variant.as() == "4.2"_s); REQUIRE(variant.as() == "4.2"_s); REQUIRE(variant.as() == "4.2"); - REQUIRE(variant.as().isStatic() == false); } SECTION("set(std::string(\"123.45\"))") { @@ -220,7 +218,6 @@ TEST_CASE("JsonVariant::as()") { REQUIRE(variant.as() == "123.45"_s); REQUIRE(variant.as() == "123.45"_s); REQUIRE(variant.as() == "123.45"); - REQUIRE(variant.as().isStatic() == false); } SECTION("set(\"true\")") { diff --git a/extras/tests/JsonVariant/copy.cpp b/extras/tests/JsonVariant/copy.cpp index b5da71f5..9ea4a2bd 100644 --- a/extras/tests/JsonVariant/copy.cpp +++ b/extras/tests/JsonVariant/copy.cpp @@ -38,13 +38,15 @@ TEST_CASE("JsonVariant::set(JsonVariant)") { REQUIRE(var1.as() == "{\"value\":[42]}"); } - SECTION("stores const char* by reference") { + SECTION("stores string literals by copy") { var1.set("hello!!"); spyingAllocator.clearLog(); var2.set(var1); - REQUIRE(spyingAllocator.log() == AllocatorLog{}); + REQUIRE(spyingAllocator.log() == AllocatorLog{ + Allocate(sizeofString("hello!!")), + }); } SECTION("stores char* by copy") { diff --git a/extras/tests/JsonVariant/set.cpp b/extras/tests/JsonVariant/set.cpp index 7d56818b..5bb212aa 100644 --- a/extras/tests/JsonVariant/set.cpp +++ b/extras/tests/JsonVariant/set.cpp @@ -9,6 +9,7 @@ #include "Literals.hpp" using ArduinoJson::detail::sizeofObject; +using ArduinoJson::detail::sizeofString; enum ErrorCode { ERROR_01 = 1, ERROR_10 = 10 }; @@ -21,9 +22,10 @@ TEST_CASE("JsonVariant::set() when there is enough memory") { bool result = variant.set("hello\0world"); REQUIRE(result == true); - CHECK(variant == - "hello"_s); // linked string cannot contain '\0' at the moment - CHECK(spy.log() == AllocatorLog{}); + REQUIRE(variant == "hello\0world"_s); // stores by copy + REQUIRE(spy.log() == AllocatorLog{ + Allocate(sizeofString(11)), + }); } SECTION("const char*") { @@ -140,19 +142,7 @@ TEST_CASE("JsonVariant::set() when there is enough memory") { }); } - SECTION("static JsonString") { - char str[16]; - - strcpy(str, "hello"); - bool result = variant.set(JsonString(str, true)); - strcpy(str, "world"); - - REQUIRE(result == true); - REQUIRE(variant == "world"); // stores by pointer - REQUIRE(spy.log() == AllocatorLog{}); - } - - SECTION("non-static JsonString") { + SECTION("JsonString") { char str[16]; strcpy(str, "hello"); diff --git a/extras/tests/Misc/JsonString.cpp b/extras/tests/Misc/JsonString.cpp index a66f7710..4c6f42c9 100644 --- a/extras/tests/Misc/JsonString.cpp +++ b/extras/tests/Misc/JsonString.cpp @@ -13,7 +13,6 @@ TEST_CASE("JsonString") { CHECK(s.isNull() == true); CHECK(s.c_str() == 0); - CHECK(s.isStatic() == true); CHECK(s == JsonString()); CHECK(s != ""); } @@ -96,7 +95,6 @@ TEST_CASE("JsonString") { JsonString s("hello world", 5); CHECK(s.size() == 5); - CHECK(s.isStatic() == false); CHECK(s == "hello"); CHECK(s != "hello world"); } diff --git a/extras/tests/Misc/StringAdapters.cpp b/extras/tests/Misc/StringAdapters.cpp index 99178975..94426e0a 100644 --- a/extras/tests/Misc/StringAdapters.cpp +++ b/extras/tests/Misc/StringAdapters.cpp @@ -22,7 +22,6 @@ TEST_CASE("adaptString()") { CHECK(s.isNull() == false); CHECK(s.size() == 11); - CHECK(s.isStatic() == true); } SECTION("null const char*") { @@ -38,7 +37,6 @@ TEST_CASE("adaptString()") { CHECK(s.isNull() == false); CHECK(s.size() == 5); - CHECK(s.isStatic() == false); CHECK(s.data() == p); } @@ -46,7 +44,6 @@ TEST_CASE("adaptString()") { auto s = adaptString(static_cast(0), 10); CHECK(s.isNull() == true); - CHECK(s.isStatic() == false); } SECTION("non-null const char* + size") { @@ -54,7 +51,6 @@ TEST_CASE("adaptString()") { CHECK(s.isNull() == false); CHECK(s.size() == 5); - CHECK(s.isStatic() == false); } SECTION("null Flash string") { @@ -62,7 +58,6 @@ TEST_CASE("adaptString()") { CHECK(s.isNull() == true); CHECK(s.size() == 0); - CHECK(s.isStatic() == false); } SECTION("non-null Flash string") { @@ -70,7 +65,6 @@ TEST_CASE("adaptString()") { CHECK(s.isNull() == false); CHECK(s.size() == 5); - CHECK(s.isStatic() == false); } SECTION("std::string") { @@ -79,7 +73,6 @@ TEST_CASE("adaptString()") { CHECK(s.isNull() == false); CHECK(s.size() == 5); - CHECK(s.isStatic() == false); } SECTION("Arduino String") { @@ -88,7 +81,6 @@ TEST_CASE("adaptString()") { CHECK(s.isNull() == false); CHECK(s.size() == 5); - CHECK(s.isStatic() == false); } SECTION("custom_string") { @@ -97,25 +89,14 @@ TEST_CASE("adaptString()") { CHECK(s.isNull() == false); CHECK(s.size() == 5); - CHECK(s.isStatic() == false); } - SECTION("JsonString linked") { - JsonString orig("hello", true); + SECTION("JsonString") { + JsonString orig("hello"); auto s = adaptString(orig); CHECK(s.isNull() == false); CHECK(s.size() == 5); - CHECK(s.isStatic() == true); - } - - SECTION("JsonString copied") { - JsonString orig("hello", false); - auto s = adaptString(orig); - - CHECK(s.isNull() == false); - CHECK(s.size() == 5); - CHECK(s.isStatic() == false); } } diff --git a/extras/tests/MixedConfiguration/string_length_size_4.cpp b/extras/tests/MixedConfiguration/string_length_size_4.cpp index 4059c3d4..992bff34 100644 --- a/extras/tests/MixedConfiguration/string_length_size_4.cpp +++ b/extras/tests/MixedConfiguration/string_length_size_4.cpp @@ -89,58 +89,71 @@ TEST_CASE("ARDUINOJSON_STRING_LENGTH_SIZE == 4") { REQUIRE(err != DeserializationError::Ok); } + + SECTION("bin 32") { + auto str = std::string(65536, '?'); + auto input = "\xc6\x00\x01\x00\x00"_s + str; + + auto err = deserializeMsgPack(doc, input); + + REQUIRE(err == DeserializationError::Ok); + REQUIRE(doc.is()); + auto binary = doc.as(); + REQUIRE(binary.size() == 65536); + REQUIRE(binary.data() != nullptr); + REQUIRE(std::string(reinterpret_cast(binary.data()), + binary.size()) == str); + } + + SECTION("ext 32 deserialization") { + auto str = std::string(65536, '?'); + auto input = "\xc9\x00\x01\x00\x00\x2a"_s + str; + + auto err = deserializeMsgPack(doc, input); + + REQUIRE(err == DeserializationError::Ok); + REQUIRE(doc.is()); + auto value = doc.as(); + REQUIRE(value.type() == 42); + REQUIRE(value.size() == 65536); + REQUIRE(value.data() != nullptr); + REQUIRE(std::string(reinterpret_cast(value.data()), + value.size()) == str); + } } - SECTION("bin 32 deserialization") { - auto str = std::string(65536, '?'); - auto input = "\xc6\x00\x01\x00\x00"_s + str; + SECTION("serializeMsgPack()") { + SECTION("bin 32 serialization") { + auto str = std::string(65536, '?'); + doc.set(MsgPackBinary(str.data(), str.size())); - auto err = deserializeMsgPack(doc, input); + std::string output; + auto result = serializeMsgPack(doc, output); - REQUIRE(err == DeserializationError::Ok); - REQUIRE(doc.is()); - auto binary = doc.as(); - REQUIRE(binary.size() == 65536); - REQUIRE(binary.data() != nullptr); - REQUIRE(std::string(reinterpret_cast(binary.data()), - binary.size()) == str); - } + REQUIRE(result == 5 + str.size()); + REQUIRE(output == "\xc6\x00\x01\x00\x00"_s + str); + } - SECTION("bin 32 serialization") { - auto str = std::string(65536, '?'); - doc.set(MsgPackBinary(str.data(), str.size())); + SECTION("ext 32 serialization") { + auto str = std::string(65536, '?'); + doc.set(MsgPackExtension(42, str.data(), str.size())); - std::string output; - auto result = serializeMsgPack(doc, output); + std::string output; + auto result = serializeMsgPack(doc, output); - REQUIRE(result == 5 + str.size()); - REQUIRE(output == "\xc6\x00\x01\x00\x00"_s + str); - } + REQUIRE(result == 6 + str.size()); + REQUIRE(output == "\xc9\x00\x01\x00\x00\x2a"_s + str); + } - SECTION("ext 32 deserialization") { - auto str = std::string(65536, '?'); - auto input = "\xc9\x00\x01\x00\x00\x2a"_s + str; + SECTION("str 32 serialization") { + auto str = std::string(65536, '?'); + doc.set(str); - auto err = deserializeMsgPack(doc, input); + std::string output; + auto result = serializeMsgPack(doc, output); - REQUIRE(err == DeserializationError::Ok); - REQUIRE(doc.is()); - auto value = doc.as(); - REQUIRE(value.type() == 42); - REQUIRE(value.size() == 65536); - REQUIRE(value.data() != nullptr); - REQUIRE(std::string(reinterpret_cast(value.data()), - value.size()) == str); - } - - SECTION("ext 32 serialization") { - auto str = std::string(65536, '?'); - doc.set(MsgPackExtension(42, str.data(), str.size())); - - std::string output; - auto result = serializeMsgPack(doc, output); - - REQUIRE(result == 6 + str.size()); - REQUIRE(output == "\xc9\x00\x01\x00\x00\x2a"_s + str); + REQUIRE(result == 5 + str.size()); + REQUIRE(output == "\xDB\x00\x01\x00\x00"_s + str); + } } } diff --git a/extras/tests/MsgPackDeserializer/destination_types.cpp b/extras/tests/MsgPackDeserializer/destination_types.cpp index 6b437ec7..859bb5ac 100644 --- a/extras/tests/MsgPackDeserializer/destination_types.cpp +++ b/extras/tests/MsgPackDeserializer/destination_types.cpp @@ -104,6 +104,8 @@ TEST_CASE("deserializeMsgPack(MemberProxy)") { REQUIRE(err == DeserializationError::Ok); REQUIRE(doc.as() == "{\"hello\":\"world\",\"value\":[42]}"); - REQUIRE(spy.log() == AllocatorLog{}); + REQUIRE(spy.log() == AllocatorLog{ + Allocate(sizeofString("value")), + }); } } diff --git a/extras/tests/MsgPackSerializer/serializeVariant.cpp b/extras/tests/MsgPackSerializer/serializeVariant.cpp index 56faccd4..a7ecf342 100644 --- a/extras/tests/MsgPackSerializer/serializeVariant.cpp +++ b/extras/tests/MsgPackSerializer/serializeVariant.cpp @@ -137,11 +137,12 @@ TEST_CASE("serialize MsgPack value") { checkVariant(longest.c_str(), "\xDA\xFF\xFF"_s + longest); } +#if ARDUINOJSON_STRING_LENGTH_SIZE > 2 SECTION("str 32") { std::string shortest(65536, '?'); - checkVariant(JsonString(shortest.c_str(), true), // force store by pointer - "\xDB\x00\x01\x00\x00"_s + shortest); + checkVariant(shortest.c_str(), "\xDB\x00\x01\x00\x00"_s + shortest); } +#endif SECTION("serialized(const char*)") { checkVariant(serialized("\xDA\xFF\xFF"), "\xDA\xFF\xFF"); diff --git a/extras/tests/ResourceManager/StringBuffer.cpp b/extras/tests/ResourceManager/StringBuffer.cpp index 1fb2b01b..167120f4 100644 --- a/extras/tests/ResourceManager/StringBuffer.cpp +++ b/extras/tests/ResourceManager/StringBuffer.cpp @@ -30,7 +30,7 @@ TEST_CASE("StringBuffer") { memcpy(ptr, "a\0b", 3); sb.save(&variant); - REQUIRE(variant.type() == VariantType::OwnedString); + REQUIRE(variant.type() == VariantType::LongString); auto str = variant.asString(); REQUIRE(str.size() == 3); @@ -44,7 +44,7 @@ TEST_CASE("StringBuffer") { strcpy(ptr, "alfa"); sb.save(&variant); - REQUIRE(variant.type() == VariantType::OwnedString); + REQUIRE(variant.type() == VariantType::LongString); REQUIRE(variant.asString() == "alfa"); } } diff --git a/src/ArduinoJson/Memory/StringBuffer.hpp b/src/ArduinoJson/Memory/StringBuffer.hpp index 4d5caa75..1a925597 100644 --- a/src/ArduinoJson/Memory/StringBuffer.hpp +++ b/src/ArduinoJson/Memory/StringBuffer.hpp @@ -43,7 +43,7 @@ class StringBuffer { if (isTinyString(s, size_)) data->setTinyString(adaptString(s, size_)); else - data->setOwnedString(commitStringNode()); + data->setLongString(commitStringNode()); } void saveRaw(VariantData* data) { diff --git a/src/ArduinoJson/Memory/StringBuilder.hpp b/src/ArduinoJson/Memory/StringBuilder.hpp index db6c3544..07b117f5 100644 --- a/src/ArduinoJson/Memory/StringBuilder.hpp +++ b/src/ArduinoJson/Memory/StringBuilder.hpp @@ -45,7 +45,7 @@ class StringBuilder { } else { node->references++; } - variant->setOwnedString(node); + variant->setLongString(node); } void append(const char* s) { diff --git a/src/ArduinoJson/Strings/Adapters/FlashString.hpp b/src/ArduinoJson/Strings/Adapters/FlashString.hpp index 3658e8ee..44d9dd34 100644 --- a/src/ArduinoJson/Strings/Adapters/FlashString.hpp +++ b/src/ArduinoJson/Strings/Adapters/FlashString.hpp @@ -63,10 +63,6 @@ class FlashString { ::memcpy_P(p, s.str_, n); } - bool isStatic() const { - return false; - } - private: const char* str_; size_t size_; diff --git a/src/ArduinoJson/Strings/Adapters/RamString.hpp b/src/ArduinoJson/Strings/Adapters/RamString.hpp index c269ffb7..25870538 100644 --- a/src/ArduinoJson/Strings/Adapters/RamString.hpp +++ b/src/ArduinoJson/Strings/Adapters/RamString.hpp @@ -20,14 +20,8 @@ struct IsChar class RamString { public: static const size_t typeSortKey = 2; -#if ARDUINOJSON_SIZEOF_POINTER <= 2 - static constexpr size_t sizeMask = size_t(-1) >> 1; -#else - static constexpr size_t sizeMask = size_t(-1); -#endif - RamString(const char* str, size_t sz, bool isStatic = false) - : str_(str), size_(sz & sizeMask), static_(isStatic) { + RamString(const char* str, size_t sz) : str_(str), size_(sz) { ARDUINOJSON_ASSERT(size_ == sz); } @@ -49,21 +43,9 @@ class RamString { return str_; } - bool isStatic() const { - return static_; - } - protected: const char* str_; - -#if ARDUINOJSON_SIZEOF_POINTER <= 2 - // Use a bitfield only on 8-bit microcontrollers - size_t size_ : sizeof(size_t) * 8 - 1; - bool static_ : 1; -#else size_t size_; - bool static_; -#endif }; template @@ -91,7 +73,7 @@ struct StringAdapter { using AdaptedString = RamString; static AdaptedString adapt(const char (&p)[N]) { - return RamString(p, N - 1, true); + return RamString(p, N - 1); } }; diff --git a/src/ArduinoJson/Strings/JsonString.hpp b/src/ArduinoJson/Strings/JsonString.hpp index 98bae43f..98a70d3f 100644 --- a/src/ArduinoJson/Strings/JsonString.hpp +++ b/src/ArduinoJson/Strings/JsonString.hpp @@ -19,17 +19,25 @@ class JsonString { friend struct detail::StringAdapter; public: - JsonString() : str_(nullptr, 0, true) {} + JsonString() : str_(nullptr, 0) {} - JsonString(const char* data, bool isStatic = false) - : str_(data, data ? ::strlen(data) : 0, isStatic) {} + JsonString(const char* data) : str_(data, data ? ::strlen(data) : 0) {} + + ARDUINOJSON_DEPRECATED( + "ArduinoJson doesn't differentiate between static and dynamic strings " + "anymore. Remove the second argument to fix this warning.") + JsonString(const char* data, bool) : JsonString(data) {} template ::value && !detail::is_same::value, int> = 0> - JsonString(const char* data, TSize sz, bool isStatic = false) - : str_(data, size_t(sz), isStatic) {} + JsonString(const char* data, TSize sz) : str_(data, size_t(sz)) {} + + ARDUINOJSON_DEPRECATED( + "ArduinoJson doesn't differentiate between static and dynamic strings " + "anymore. Remove the third argument to fix this warning.") + JsonString(const char* data, size_t sz, bool) : JsonString(data, sz) {} // Returns a pointer to the characters. const char* c_str() const { @@ -41,10 +49,10 @@ class JsonString { return str_.isNull(); } - // Returns true if the string is stored by address. - // Returns false if the string is stored by copy. + // Deprecated: always returns false. + ARDUINOJSON_DEPRECATED("The isStatic() was removed in v7.5") bool isStatic() const { - return str_.isStatic(); + return false; } // Returns length of the string. diff --git a/src/ArduinoJson/Variant/VariantContent.hpp b/src/ArduinoJson/Variant/VariantContent.hpp index d6ee6c78..186cdc9d 100644 --- a/src/ArduinoJson/Variant/VariantContent.hpp +++ b/src/ArduinoJson/Variant/VariantContent.hpp @@ -23,15 +23,14 @@ enum class VariantTypeBits : uint8_t { }; enum class VariantType : uint8_t { - Null = 0, // 0000 0000 - TinyString = 0x02, // 0000 0010 - RawString = 0x03, // 0000 0011 - LinkedString = 0x04, // 0000 0100 - OwnedString = 0x05, // 0000 0101 - Boolean = 0x06, // 0000 0110 - Uint32 = 0x0A, // 0000 1010 - Int32 = 0x0C, // 0000 1100 - Float = 0x0E, // 0000 1110 + Null = 0, // 0000 0000 + TinyString = 0x02, // 0000 0010 + RawString = 0x03, // 0000 0011 + LongString = 0x05, // 0000 0101 + Boolean = 0x06, // 0000 0110 + Uint32 = 0x0A, // 0000 1010 + Int32 = 0x0C, // 0000 1100 + Float = 0x0E, // 0000 1110 #if ARDUINOJSON_USE_LONG_LONG Uint64 = 0x1A, // 0001 1010 Int64 = 0x1C, // 0001 1100 @@ -62,8 +61,7 @@ union VariantContent { ArrayData asArray; ObjectData asObject; CollectionData asCollection; - const char* asLinkedString; - struct StringNode* asOwnedString; + struct StringNode* asStringNode; char asTinyString[tinyStringMaxLength + 1]; }; diff --git a/src/ArduinoJson/Variant/VariantData.hpp b/src/ArduinoJson/Variant/VariantData.hpp index 6b0d474b..160e2cff 100644 --- a/src/ArduinoJson/Variant/VariantData.hpp +++ b/src/ArduinoJson/Variant/VariantData.hpp @@ -76,16 +76,13 @@ class VariantData { case VariantType::TinyString: return visit.visit(JsonString(content_.asTinyString)); - case VariantType::LinkedString: - return visit.visit(JsonString(content_.asLinkedString, true)); - - case VariantType::OwnedString: - return visit.visit(JsonString(content_.asOwnedString->data, - content_.asOwnedString->length)); + case VariantType::LongString: + return visit.visit(JsonString(content_.asStringNode->data, + content_.asStringNode->length)); case VariantType::RawString: - return visit.visit(RawString(content_.asOwnedString->data, - content_.asOwnedString->length)); + return visit.visit(RawString(content_.asStringNode->data, + content_.asStringNode->length)); case VariantType::Int32: return visit.visit(static_cast(content_.asInt32)); @@ -215,11 +212,8 @@ class VariantData { case VariantType::TinyString: str = content_.asTinyString; break; - case VariantType::LinkedString: - str = content_.asLinkedString; - break; - case VariantType::OwnedString: - str = content_.asOwnedString->data; + case VariantType::LongString: + str = content_.asStringNode->data; break; case VariantType::Float: return static_cast(content_.asFloat); @@ -260,11 +254,8 @@ class VariantData { case VariantType::TinyString: str = content_.asTinyString; break; - case VariantType::LinkedString: - str = content_.asLinkedString; - break; - case VariantType::OwnedString: - str = content_.asOwnedString->data; + case VariantType::LongString: + str = content_.asStringNode->data; break; case VariantType::Float: return convertNumber(content_.asFloat); @@ -291,8 +282,8 @@ class VariantData { JsonString asRawString() const { switch (type_) { case VariantType::RawString: - return JsonString(content_.asOwnedString->data, - content_.asOwnedString->length); + return JsonString(content_.asStringNode->data, + content_.asStringNode->length); default: return JsonString(); } @@ -302,11 +293,9 @@ class VariantData { switch (type_) { case VariantType::TinyString: return JsonString(content_.asTinyString); - case VariantType::LinkedString: - return JsonString(content_.asLinkedString, true); - case VariantType::OwnedString: - return JsonString(content_.asOwnedString->data, - content_.asOwnedString->length); + case VariantType::LongString: + return JsonString(content_.asStringNode->data, + content_.asStringNode->length); default: return JsonString(); } @@ -415,9 +404,7 @@ class VariantData { } bool isString() const { - return type_ == VariantType::LinkedString || - type_ == VariantType::OwnedString || - type_ == VariantType::TinyString; + return type_ == VariantType::LongString || type_ == VariantType::TinyString; } size_t nesting(const ResourceManager* resources) const { @@ -492,7 +479,7 @@ class VariantData { ARDUINOJSON_ASSERT(type_ == VariantType::Null); // must call clear() first ARDUINOJSON_ASSERT(s); type_ = VariantType::RawString; - content_.asOwnedString = s; + content_.asStringNode = s; } template @@ -519,13 +506,6 @@ class VariantData { var->setString(value, resources); } - void setLinkedString(const char* s) { - ARDUINOJSON_ASSERT(type_ == VariantType::Null); // must call clear() first - ARDUINOJSON_ASSERT(s); - type_ = VariantType::LinkedString; - content_.asLinkedString = s; - } - template void setTinyString(const TAdaptedString& s) { ARDUINOJSON_ASSERT(type_ == VariantType::Null); // must call clear() first @@ -543,11 +523,11 @@ class VariantData { content_.asTinyString[n] = 0; } - void setOwnedString(StringNode* s) { + void setLongString(StringNode* s) { ARDUINOJSON_ASSERT(type_ == VariantType::Null); // must call clear() first ARDUINOJSON_ASSERT(s); - type_ = VariantType::OwnedString; - content_.asOwnedString = s; + type_ = VariantType::LongString; + content_.asStringNode = s; } size_t size(const ResourceManager* resources) const { diff --git a/src/ArduinoJson/Variant/VariantImpl.hpp b/src/ArduinoJson/Variant/VariantImpl.hpp index 98289ed0..926965e4 100644 --- a/src/ArduinoJson/Variant/VariantImpl.hpp +++ b/src/ArduinoJson/Variant/VariantImpl.hpp @@ -26,11 +26,6 @@ inline bool VariantData::setString(TAdaptedString value, if (value.isNull()) return false; - if (value.isStatic()) { - setLinkedString(value.data()); - return true; - } - if (isTinyString(value, value.size())) { setTinyString(value); return true; @@ -38,7 +33,7 @@ inline bool VariantData::setString(TAdaptedString value, auto dup = resources->saveString(value); if (dup) { - setOwnedString(dup); + setLongString(dup); return true; } @@ -47,7 +42,7 @@ inline bool VariantData::setString(TAdaptedString value, inline void VariantData::clear(ResourceManager* resources) { if (type_ & VariantTypeBits::OwnedStringBit) - resources->dereferenceString(content_.asOwnedString->data); + resources->dereferenceString(content_.asStringNode->data); #if ARDUINOJSON_USE_EXTENSIONS if (type_ & VariantTypeBits::ExtensionBit)