JsonArray::remove() and JsonObject::remove() now release the memory of strings

test/StaticMemoryPool/CMakeLists.txt

@@ -3,9 +3,11 @@
 # MIT License
 
 add_executable(StaticMemoryPoolTests 
-	alloc.cpp
+	allocVariant.cpp
+	allocString.cpp
+	clear.cpp
 	size.cpp
-	startString.cpp
+	StringBuilder.cpp
 )
 
 target_link_libraries(StaticMemoryPoolTests catch)

test/StaticMemoryPool/StringBuilder.cpp

@@ -0,0 +1,39 @@
+// ArduinoJson - arduinojson.org
+// Copyright Benoit Blanchon 2014-2018
+// MIT License
+
+#include <ArduinoJson/Memory/StaticMemoryPool.hpp>
+#include <ArduinoJson/Memory/StringBuilder.hpp>
+#include <catch.hpp>
+
+using namespace ARDUINOJSON_NAMESPACE;
+
+TEST_CASE("StringBuilder") {
+  SECTION("WorksWhenBufferIsBigEnough") {
+    StaticMemoryPool<JSON_STRING_SIZE(6)> memoryPool;
+
+    StringBuilder str(&memoryPool);
+    str.append("hello");
+
+    REQUIRE(str.complete().equals("hello"));
+  }
+
+  SECTION("ReturnsNullWhenTooSmall") {
+    StaticMemoryPool<1> memoryPool;
+
+    StringBuilder str(&memoryPool);
+    str.append("hello!!!");
+
+    REQUIRE(str.complete().isNull());
+  }
+
+  SECTION("Increases size of memory pool") {
+    StaticMemoryPool<JSON_STRING_SIZE(6)> memoryPool;
+
+    StringBuilder str(&memoryPool);
+    str.append('h');
+    str.complete();
+
+    REQUIRE(JSON_STRING_SIZE(2) == memoryPool.size());
+  }
+}

test/StaticMemoryPool/alloc.cpp

@@ -1,54 +0,0 @@
-// ArduinoJson - arduinojson.org
-// Copyright Benoit Blanchon 2014-2018
-// MIT License
-
-#include <ArduinoJson/Memory/StaticMemoryPool.hpp>
-#include <catch.hpp>
-
-using namespace ARDUINOJSON_NAMESPACE;
-
-static bool isAligned(void *ptr) {
-  const size_t mask = sizeof(void *) - 1;
-  size_t addr = reinterpret_cast<size_t>(ptr);
-  return (addr & mask) == 0;
-}
-
-TEST_CASE("StaticMemoryPool::alloc()") {
-  StaticMemoryPool<64> memoryPool;
-
-  SECTION("Returns different addresses") {
-    void *p1 = memoryPool.alloc(1);
-    void *p2 = memoryPool.alloc(1);
-    REQUIRE(p1 != p2);
-  }
-
-  SECTION("Returns non-NULL when using full capacity") {
-    void *p = memoryPool.alloc(64);
-    REQUIRE(0 != p);
-  }
-
-  SECTION("Returns NULL when full") {
-    memoryPool.alloc(64);
-    void *p = memoryPool.alloc(1);
-    REQUIRE(0 == p);
-  }
-
-  SECTION("Returns NULL when memoryPool is too small") {
-    void *p = memoryPool.alloc(65);
-    REQUIRE(0 == p);
-  }
-
-  SECTION("Returns aligned pointers") {
-    for (size_t size = 1; size <= sizeof(void *); size++) {
-      void *p = memoryPool.alloc(1);
-      REQUIRE(isAligned(p));
-    }
-  }
-
-  SECTION("Returns same address after clear()") {
-    void *p1 = memoryPool.alloc(1);
-    memoryPool.clear();
-    void *p2 = memoryPool.alloc(1);
-    REQUIRE(p1 == p2);
-  }
-}

test/StaticMemoryPool/allocString.cpp

@@ -0,0 +1,131 @@
+// ArduinoJson - arduinojson.org
+// Copyright Benoit Blanchon 2014-2018
+// MIT License
+
+#include <ArduinoJson/Memory/StaticMemoryPool.hpp>
+#include <catch.hpp>
+
+using namespace ARDUINOJSON_NAMESPACE;
+
+TEST_CASE("StaticMemoryPool::allocFrozenString()") {
+  const size_t poolCapacity = 64;
+  const size_t longestString = poolCapacity - sizeof(StringSlot);
+  StaticMemoryPool<poolCapacity> pool;
+
+  SECTION("Returns different addresses") {
+    StringSlot *a = pool.allocFrozenString(1);
+    StringSlot *b = pool.allocFrozenString(1);
+    REQUIRE(a != b);
+    REQUIRE(a->value != b->value);
+  }
+
+  SECTION("Returns a StringSlot of the right size") {
+    StringSlot *s = pool.allocFrozenString(12);
+    REQUIRE(s->size == 12);
+  }
+
+  SECTION("Returns NULL when full") {
+    pool.allocFrozenString(longestString);
+    void *p = pool.allocFrozenString(1);
+    REQUIRE(0 == p);
+  }
+
+  SECTION("Returns NULL when pool is too small") {
+    void *p = pool.allocFrozenString(longestString + 1);
+    REQUIRE(0 == p);
+  }
+
+  SECTION("Returns aligned pointers") {
+    REQUIRE(isAligned(pool.allocFrozenString(1)));
+    REQUIRE(isAligned(pool.allocFrozenString(1)));
+  }
+
+  SECTION("Returns same address after clear()") {
+    StringSlot *a = pool.allocFrozenString(1);
+    pool.clear();
+    StringSlot *b = pool.allocFrozenString(1);
+
+    REQUIRE(a == b);
+    REQUIRE(a->value == b->value);
+  }
+
+  SECTION("Returns same address after freeString()") {
+    StringSlot *a = pool.allocFrozenString(1);
+    pool.freeString(a);
+    StringSlot *b = pool.allocFrozenString(1);
+
+    REQUIRE(a == b);
+    REQUIRE(a->value == b->value);
+  }
+
+  SECTION("Can use full capacity when fresh") {
+    StringSlot *a = pool.allocFrozenString(longestString);
+
+    REQUIRE(a != 0);
+  }
+
+  SECTION("Can use full capacity after clear") {
+    pool.allocFrozenString(longestString);
+    pool.clear();
+
+    StringSlot *a = pool.allocFrozenString(longestString);
+
+    REQUIRE(a != 0);
+  }
+}
+
+TEST_CASE("StaticMemoryPool::freeString()") {
+  const size_t poolCapacity = 512;
+  const size_t longestString = poolCapacity - sizeof(StringSlot);
+  StaticMemoryPool<poolCapacity> pool;
+
+  static const size_t testStringSize =
+      (poolCapacity - sizeof(StringSlot) * 4 - sizeof(VariantSlot) * 4) / 4;
+
+  SECTION("Restores full capacity") {
+    StringSlot *strings[4];
+    VariantSlot *variants[4];
+
+    for (int i = 0; i < 4; i++) {
+      strings[i] = pool.allocFrozenString(testStringSize);
+      REQUIRE(strings[i] != 0);
+      variants[i] = pool.allocVariant();
+      REQUIRE(variants[i] != 0);
+    }
+
+    // In random order
+    pool.freeString(strings[2]);
+    pool.freeVariant(variants[3]);
+    pool.freeVariant(variants[0]);
+    pool.freeString(strings[0]);
+    pool.freeVariant(variants[1]);
+    pool.freeString(strings[1]);
+    pool.freeVariant(variants[2]);
+    pool.freeString(strings[3]);
+
+    StringSlot *b = pool.allocFrozenString(longestString);
+
+    REQUIRE(b != 0);
+    REQUIRE(b->size == longestString);
+  }
+
+  SECTION("Move strings") {
+    StringSlot *a = pool.allocFrozenString(6);
+    strcpy(a->value, "hello");
+
+    StringSlot *b = pool.allocFrozenString(7);
+    strcpy(b->value, "world!");
+    pool.freeString(a);
+
+    REQUIRE(b->size == 7);
+    REQUIRE(b->value == std::string("world!"));
+    REQUIRE(a->value == b->value);
+  }
+
+  SECTION("Accepts non-frozen string") {
+    StringSlot *a = pool.allocExpandableString();
+    pool.freeString(a);
+
+    REQUIRE(pool.size() == 0);
+  }
+}

test/StaticMemoryPool/allocVariant.cpp

@@ -0,0 +1,38 @@
+// ArduinoJson - arduinojson.org
+// Copyright Benoit Blanchon 2014-2018
+// MIT License
+
+#include <ArduinoJson/Memory/StaticMemoryPool.hpp>
+#include <catch.hpp>
+
+using namespace ARDUINOJSON_NAMESPACE;
+
+TEST_CASE("StaticMemoryPool::allocVariant()") {
+  StaticMemoryPool<128> memoryPool;
+
+  SECTION("Returns different pointer") {
+    VariantSlot* s1 = memoryPool.allocVariant();
+    REQUIRE(s1 != 0);
+    VariantSlot* s2 = memoryPool.allocVariant();
+    REQUIRE(s2 != 0);
+
+    REQUIRE(s1 != s2);
+  }
+
+  SECTION("Returns same pointer after freeSlot()") {
+    VariantSlot* s1 = memoryPool.allocVariant();
+    memoryPool.freeVariant(s1);
+    VariantSlot* s2 = memoryPool.allocVariant();
+
+    REQUIRE(s1 == s2);
+  }
+
+  SECTION("Returns aligned pointers") {
+    // make room for two
+    // pass an uneven capacity
+    StaticMemoryPool<2 * sizeof(VariantSlot) + 1> pool;
+
+    REQUIRE(isAligned(pool.allocVariant()));
+    REQUIRE(isAligned(pool.allocVariant()));
+  }
+}

test/StaticMemoryPool/clear.cpp

@@ -0,0 +1,81 @@
+// ArduinoJson - arduinojson.org
+// Copyright Benoit Blanchon 2014-2018
+// MIT License
+
+#include <ArduinoJson/Memory/StaticMemoryPool.hpp>
+#include <catch.hpp>
+
+using namespace ARDUINOJSON_NAMESPACE;
+
+static const size_t poolCapacity = 512;
+
+TEST_CASE("StaticMemoryPool::clear()") {
+  StaticMemoryPool<poolCapacity> memoryPool;
+
+  SECTION("Discards allocated variants") {
+    memoryPool.allocVariant();
+
+    memoryPool.clear();
+    REQUIRE(memoryPool.size() == 0);
+  }
+
+  SECTION("Discards allocated strings") {
+    memoryPool.allocFrozenString(10);
+
+    memoryPool.clear();
+
+    REQUIRE(memoryPool.size() == 0);
+  }
+
+  SECTION("Purges variant cache") {
+    VariantSlot* a = memoryPool.allocVariant();
+    REQUIRE(a != 0);
+    VariantSlot* b = memoryPool.allocVariant();
+    REQUIRE(b != 0);
+
+    // place slot a in the pool of free slots
+    memoryPool.freeVariant(a);
+    memoryPool.clear();
+
+    REQUIRE(memoryPool.size() == 0);
+  }
+
+  SECTION("Purges string cache") {
+    StringSlot* a = memoryPool.allocFrozenString(10);
+    REQUIRE(a != 0);
+    StringSlot* b = memoryPool.allocFrozenString(10);
+    REQUIRE(b != 0);
+
+    // place slot a in the pool of free slots
+    memoryPool.freeString(a);
+    memoryPool.clear();
+
+    REQUIRE(memoryPool.size() == 0);
+  }
+
+  SECTION("Purges list of string") {
+    StringSlot* a = memoryPool.allocFrozenString(6);
+    REQUIRE(a != 0);
+    strcpy(a->value, "hello");
+
+    StringSlot* b = memoryPool.allocFrozenString(6);
+    REQUIRE(b != 0);
+    strcpy(b->value, "world");
+
+    memoryPool.clear();  // ACT!
+
+    StringSlot* c = memoryPool.allocFrozenString(2);
+    REQUIRE(c != 0);
+    strcpy(c->value, "H");
+
+    StringSlot* d = memoryPool.allocFrozenString(2);
+    REQUIRE(d != 0);
+    strcpy(d->value, "W");
+
+    // if the memory pool keeps pointer to the old strings
+    // it will try to compact the strings
+    memoryPool.freeString(c);
+
+    REQUIRE(d->value == std::string("W"));
+  }
+}

test/StaticMemoryPool/size.cpp

@@ -8,37 +8,72 @@
 using namespace ARDUINOJSON_NAMESPACE;
 
 TEST_CASE("StaticMemoryPool::size()") {
-  StaticMemoryPool<64> memoryPool;
-
   SECTION("Capacity equals template parameter") {
-    REQUIRE(64 == memoryPool.capacity());
+    const size_t capacity = 64;
+    StaticMemoryPool<capacity> memoryPool;
+    REQUIRE(capacity == memoryPool.capacity());
   }
 
   SECTION("Initial size is 0") {
+    StaticMemoryPool<32> memoryPool;
     REQUIRE(0 == memoryPool.size());
   }
 
-  SECTION("Increases after alloc()") {
-    memoryPool.alloc(1);
-    REQUIRE(1U <= memoryPool.size());
-    memoryPool.alloc(1);
-    REQUIRE(2U <= memoryPool.size());
+  SECTION("Increases after allocFrozenString()") {
+    StaticMemoryPool<128> memoryPool;
+    memoryPool.allocFrozenString(0);
+    REQUIRE(memoryPool.size() == JSON_STRING_SIZE(0));
+    memoryPool.allocFrozenString(0);
+    REQUIRE(memoryPool.size() == 2 * JSON_STRING_SIZE(0));
   }
 
-  SECTION("Doesn't grow when memoryPool is full") {
-    memoryPool.alloc(64);
-    memoryPool.alloc(1);
-    REQUIRE(64 == memoryPool.size());
+  SECTION("Decreases after freeVariant()") {
+    StaticMemoryPool<128> memoryPool;
+    VariantSlot* a = memoryPool.allocVariant();
+    VariantSlot* b = memoryPool.allocVariant();
+
+    memoryPool.freeVariant(b);
+    REQUIRE(memoryPool.size() == sizeof(VariantSlot));
+    memoryPool.freeVariant(a);
+    REQUIRE(memoryPool.size() == 0);
   }
 
-  SECTION("Does't grow when memoryPool is too small for alloc") {
-    memoryPool.alloc(65);
-    REQUIRE(0 == memoryPool.size());
+  SECTION("Decreases after calling freeString() in order") {
+    StaticMemoryPool<128> memoryPool;
+    StringSlot* a = memoryPool.allocFrozenString(5);
+    REQUIRE(a != 0);
+    StringSlot* b = memoryPool.allocFrozenString(6);
+    REQUIRE(b != 0);
+
+    memoryPool.freeString(b);
+    REQUIRE(memoryPool.size() == JSON_STRING_SIZE(5));
+    memoryPool.freeString(a);
+    REQUIRE(memoryPool.size() == 0);
   }
 
-  SECTION("Goes back to zero after clear()") {
-    memoryPool.alloc(1);
-    memoryPool.clear();
-    REQUIRE(0 == memoryPool.size());
+  SECTION("Decreases after calling freeString() in reverse order") {
+    StaticMemoryPool<128> memoryPool;
+    StringSlot* a = memoryPool.allocFrozenString(5);
+    REQUIRE(a != 0);
+    StringSlot* b = memoryPool.allocFrozenString(6);
+    REQUIRE(b != 0);
+
+    memoryPool.freeString(a);
+    REQUIRE(memoryPool.size() == JSON_STRING_SIZE(6));
+    memoryPool.freeString(b);
+    REQUIRE(memoryPool.size() == 0);
+  }
+
+  SECTION("Doesn't grow when memory pool is full") {
+    const size_t variantCount = 4;
+    const size_t capacity = variantCount * sizeof(VariantSlot);
+    StaticMemoryPool<capacity> memoryPool;
+
+    for (size_t i = 0; i < variantCount; i++) memoryPool.allocVariant();
+    REQUIRE(capacity == memoryPool.size());
+
+    memoryPool.allocVariant();
+
+    REQUIRE(capacity == memoryPool.size());
   }
 }

test/StaticMemoryPool/startString.cpp

@@ -1,46 +0,0 @@
-// ArduinoJson - arduinojson.org
-// Copyright Benoit Blanchon 2014-2018
-// MIT License
-
-#include <ArduinoJson.h>
-#include <catch.hpp>
-
-using namespace ARDUINOJSON_NAMESPACE;
-
-TEST_CASE("StaticMemoryPool::startString()") {
-  SECTION("WorksWhenBufferIsBigEnough") {
-    StaticMemoryPool<6> memoryPool;
-
-    StringBuilder str = memoryPool.startString();
-    str.append('h');
-    str.append('e');
-    str.append('l');
-    str.append('l');
-    str.append('o');
-
-    REQUIRE(str.complete().equals("hello"));
-  }
-
-  SECTION("ReturnsNullWhenTooSmall") {
-    StaticMemoryPool<5> memoryPool;
-
-    StringBuilder str = memoryPool.startString();
-    str.append('h');
-    str.append('e');
-    str.append('l');
-    str.append('l');
-    str.append('o');
-
-    REQUIRE(str.complete().isNull());
-  }
-
-  SECTION("SizeIncreases") {
-    StaticMemoryPool<5> memoryPool;
-
-    StringBuilder str = memoryPool.startString();
-    str.append('h');
-    str.complete();
-
-    REQUIRE(2 == memoryPool.size());
-  }
-}