From f95dd7d204fc592f8b3c424206f415cd37b2769c Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Wed, 26 Feb 2025 14:22:09 +0100 Subject: [PATCH] Fix overflowed() --- extras/tests/JsonVariant/set.cpp | 14 ++++++ extras/tests/ResourceManager/CMakeLists.txt | 1 + .../ResourceManager/saveStaticString.cpp | 47 +++++++++++++++++++ extras/tests/ResourceManager/saveString.cpp | 22 --------- src/ArduinoJson/Memory/ResourceManager.hpp | 4 +- 5 files changed, 65 insertions(+), 23 deletions(-) create mode 100644 extras/tests/ResourceManager/saveStaticString.cpp diff --git a/extras/tests/JsonVariant/set.cpp b/extras/tests/JsonVariant/set.cpp index d548f33d..6565416d 100644 --- a/extras/tests/JsonVariant/set.cpp +++ b/extras/tests/JsonVariant/set.cpp @@ -257,6 +257,20 @@ TEST_CASE("JsonVariant::set() with not enough memory") { JsonVariant v = doc.to(); + SECTION("string literal") { + bool result = v.set("hello world"); + + REQUIRE(result == false); + REQUIRE(v.isNull()); + } + + SECTION("static JsonString") { + bool result = v.set(JsonString("hello world", true)); + + REQUIRE(result == false); + REQUIRE(v.isNull()); + } + SECTION("std::string") { bool result = v.set("hello world!!"_s); diff --git a/extras/tests/ResourceManager/CMakeLists.txt b/extras/tests/ResourceManager/CMakeLists.txt index 27ff0e90..d55494eb 100644 --- a/extras/tests/ResourceManager/CMakeLists.txt +++ b/extras/tests/ResourceManager/CMakeLists.txt @@ -5,6 +5,7 @@ add_executable(ResourceManagerTests allocVariant.cpp clear.cpp + saveStaticString.cpp saveString.cpp shrinkToFit.cpp size.cpp diff --git a/extras/tests/ResourceManager/saveStaticString.cpp b/extras/tests/ResourceManager/saveStaticString.cpp new file mode 100644 index 00000000..588f2072 --- /dev/null +++ b/extras/tests/ResourceManager/saveStaticString.cpp @@ -0,0 +1,47 @@ +// ArduinoJson - https://arduinojson.org +// Copyright © 2014-2025, Benoit BLANCHON +// MIT License + +#include +#include +#include + +#include "Allocators.hpp" + +using namespace ArduinoJson::detail; + +TEST_CASE("ResourceManager::saveStaticString() deduplicates strings") { + SpyingAllocator spy; + ResourceManager resources(&spy); + + auto str1 = "hello"; + auto str2 = "world"; + + auto id1 = resources.saveStaticString(str1); + auto id2 = resources.saveStaticString(str2); + REQUIRE(id1 != id2); + + auto id3 = resources.saveStaticString(str1); + REQUIRE(id1 == id3); + + resources.shrinkToFit(); + REQUIRE(spy.log() == + AllocatorLog{ + Allocate(sizeofStaticStringPool()), + Reallocate(sizeofStaticStringPool(), sizeofStaticStringPool(2)), + }); + REQUIRE(resources.overflowed() == false); +} + +TEST_CASE("ResourceManager::saveStaticString() when allocation fails") { + SpyingAllocator spy(FailingAllocator::instance()); + ResourceManager resources(&spy); + + auto slotId = resources.saveStaticString("hello"); + + REQUIRE(slotId == NULL_SLOT); + REQUIRE(resources.overflowed() == true); + REQUIRE(spy.log() == AllocatorLog{ + AllocateFail(sizeofStaticStringPool()), + }); +} diff --git a/extras/tests/ResourceManager/saveString.cpp b/extras/tests/ResourceManager/saveString.cpp index 88e61c47..3c7a228b 100644 --- a/extras/tests/ResourceManager/saveString.cpp +++ b/extras/tests/ResourceManager/saveString.cpp @@ -19,28 +19,6 @@ static StringNode* saveString(ResourceManager& resources, const char* s, return resources.saveString(adaptString(s, n)); } -TEST_CASE("ResourceManager::saveStaticString()") { - SpyingAllocator spy; - ResourceManager resources(&spy); - - auto str1 = "hello"; - auto str2 = "world"; - - auto id1 = resources.saveStaticString(str1); - auto id2 = resources.saveStaticString(str2); - REQUIRE(id1 != id2); - - auto id3 = resources.saveStaticString(str1); - REQUIRE(id1 == id3); - - resources.shrinkToFit(); - REQUIRE(spy.log() == - AllocatorLog{ - Allocate(sizeofStaticStringPool()), - Reallocate(sizeofStaticStringPool(), sizeofStaticStringPool(2)), - }); -} - TEST_CASE("ResourceManager::saveString()") { ResourceManager resources; diff --git a/src/ArduinoJson/Memory/ResourceManager.hpp b/src/ArduinoJson/Memory/ResourceManager.hpp index 18aa198c..12a379c8 100644 --- a/src/ArduinoJson/Memory/ResourceManager.hpp +++ b/src/ArduinoJson/Memory/ResourceManager.hpp @@ -121,6 +121,8 @@ class ResourceManager { auto slot = staticStringsPools_.allocSlot(allocator_); if (slot) *slot = s; + else + overflowed_ = true; return slot.id(); } @@ -130,8 +132,8 @@ class ResourceManager { } void clear() { - variantPools_.clear(allocator_); overflowed_ = false; + variantPools_.clear(allocator_); stringPool_.clear(allocator_); staticStringsPools_.clear(allocator_); }