Skip to content

Commit 926b6a0

Browse files
bblanchonbblanchon
committed
Don't store string literals by pointer anymore
Fixes #2189
1 parent 509807d commit 926b6a0

File tree

30 files changed

+172
-328
lines changed

30 files changed

+172
-328
lines changed

‎CHANGELOG.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,25 @@
11
ArduinoJson: change log
22
=======================
33

4+
HEAD
5+
----
6+
7+
* Don't store string literals by pointer anymore (issue #2189)
8+
Version 7.3 introduced a new way to detect string literals, but it failed in some edges cases.
9+
I could not find a way to fix it, so I prefer to remove the optimization rather than keeping it broken.
10+
11+
> ### BREAKING CHANGES
12+
>
13+
> Since version 7.3, you could pass a boolean to `JsonString`'s constructor to force the string to be stored by pointer.
14+
> This optimization has been removed, you'll get a deprecation warning if you use it.
15+
> To fix the issue, you must remove the boolean argument from the constructor, or better yet, remove the `JsonString` altogether.
16+
>
17+
> ```diff
18+
> char name[] = "ArduinoJson";
19+
> - doc["name"] = JsonString(name, true);
20+
> + doc["name"] = name;
21+
> ```
22+
423
v7.4.2 (2025-06-20)
524
------
625

‎extras/tests/Deprecated/BasicJsonDocument.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ TEST_CASE("BasicJsonDocument") {
5454
doc["hello"] = "world";
5555
auto copy = doc;
5656
REQUIRE(copy.as<std::string>() == "{\"hello\":\"world\"}");
57-
REQUIRE(allocatorLog == "AA");
57+
REQUIRE(allocatorLog == "AAAAAA");
5858
}
5959

6060
SECTION("capacity") {

‎extras/tests/JsonArray/add.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ TEST_CASE("JsonArray::add(T)") {
5656
REQUIRE(array[0].is<int>() == false);
5757
REQUIRE(spy.log() == AllocatorLog{
5858
Allocate(sizeofPool()),
59+
Allocate(sizeofString("hello")),
5960
});
6061
}
6162

‎extras/tests/JsonDeserializer/destination_types.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ TEST_CASE("deserializeJson(MemberProxy)") {
104104

105105
REQUIRE(err == DeserializationError::Ok);
106106
REQUIRE(doc.as<std::string>() == "{\"hello\":\"world\",\"value\":[42]}");
107-
REQUIRE(spy.log() == AllocatorLog{});
107+
REQUIRE(spy.log() == AllocatorLog{
108+
Allocate(sizeofString("value")),
109+
});
108110
}
109111
}

‎extras/tests/JsonDocument/ElementProxy.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ TEST_CASE("ElementProxy::add()") {
3131
REQUIRE(doc.as<std::string>() == "[[\"world\"]]");
3232
REQUIRE(spy.log() == AllocatorLog{
3333
Allocate(sizeofPool()),
34+
Allocate(sizeofString("world")),
3435
});
3536
}
3637

‎extras/tests/JsonDocument/MemberProxy.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ TEST_CASE("MemberProxy::add()") {
2525
REQUIRE(doc.as<std::string>() == "{\"hello\":[42]}");
2626
REQUIRE(spy.log() == AllocatorLog{
2727
Allocate(sizeofPool()),
28+
Allocate(sizeofString("hello")),
2829
});
2930
}
3031

@@ -34,6 +35,8 @@ TEST_CASE("MemberProxy::add()") {
3435
REQUIRE(doc.as<std::string>() == "{\"hello\":[\"world\"]}");
3536
REQUIRE(spy.log() == AllocatorLog{
3637
Allocate(sizeofPool()),
38+
Allocate(sizeofString("hello")),
39+
Allocate(sizeofString("world")),
3740
});
3841
}
3942

@@ -44,6 +47,7 @@ TEST_CASE("MemberProxy::add()") {
4447
REQUIRE(doc.as<std::string>() == "{\"hello\":[\"world\"]}");
4548
REQUIRE(spy.log() == AllocatorLog{
4649
Allocate(sizeofPool()),
50+
Allocate(sizeofString("hello")),
4751
Allocate(sizeofString("world")),
4852
});
4953
}
@@ -55,6 +59,7 @@ TEST_CASE("MemberProxy::add()") {
5559
REQUIRE(doc.as<std::string>() == "{\"hello\":[\"world\"]}");
5660
REQUIRE(spy.log() == AllocatorLog{
5761
Allocate(sizeofPool()),
62+
Allocate(sizeofString("hello")),
5863
Allocate(sizeofString("world")),
5964

6065
});
@@ -71,6 +76,7 @@ TEST_CASE("MemberProxy::add()") {
7176
REQUIRE(doc.as<std::string>() == "{\"hello\":[\"world\"]}");
7277
REQUIRE(spy.log() == AllocatorLog{
7378
Allocate(sizeofPool()),
79+
Allocate(sizeofString("hello")),
7480
Allocate(sizeofString("world")),
7581
});
7682
}

‎extras/tests/JsonDocument/add.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ TEST_CASE("JsonDocument::add(T)") {
3232
REQUIRE(doc.as<std::string>() == "[\"hello\"]");
3333
REQUIRE(spy.log() == AllocatorLog{
3434
Allocate(sizeofPool()),
35+
Allocate(sizeofString("hello")),
3536
});
3637
}
3738

‎extras/tests/JsonDocument/constructor.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ TEST_CASE("JsonDocument constructor") {
6464
REQUIRE(doc2.as<std::string>() == "{\"hello\":\"world\"}");
6565
REQUIRE(spyingAllocator.log() == AllocatorLog{
6666
Allocate(sizeofPool()),
67+
Allocate(sizeofString("hello")),
68+
Allocate(sizeofString("world")),
6769
});
6870
}
6971

@@ -87,6 +89,7 @@ TEST_CASE("JsonDocument constructor") {
8789
REQUIRE(doc2.as<std::string>() == "[\"hello\"]");
8890
REQUIRE(spyingAllocator.log() == AllocatorLog{
8991
Allocate(sizeofPool()),
92+
Allocate(sizeofString("hello")),
9093
});
9194
}
9295

‎extras/tests/JsonDocument/set.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ TEST_CASE("JsonDocument::set()") {
3737
doc.set("example");
3838

3939
REQUIRE(doc.as<const char*>() == "example"_s);
40-
REQUIRE(spy.log() == AllocatorLog{});
40+
REQUIRE(spy.log() == AllocatorLog{
41+
Allocate(sizeofString("example")),
42+
});
4143
}
4244

4345
SECTION("const char*") {

‎extras/tests/JsonDocument/shrinkToFit.cpp‎

Lines changed: 5 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -69,17 +69,8 @@ TEST_CASE("JsonDocument::shrinkToFit()") {
6969
REQUIRE(spyingAllocator.log() == AllocatorLog{});
7070
}
7171

72-
SECTION("linked string") {
73-
doc.set("hello");
74-
75-
doc.shrinkToFit();
76-
77-
REQUIRE(doc.as<std::string>() == "hello");
78-
REQUIRE(spyingAllocator.log() == AllocatorLog{});
79-
}
80-
81-
SECTION("owned string") {
82-
doc.set("abcdefg"_s);
72+
SECTION("string") {
73+
doc.set("abcdefg");
8374
REQUIRE(doc.as<std::string>() == "abcdefg");
8475

8576
doc.shrinkToFit();
@@ -101,20 +92,7 @@ TEST_CASE("JsonDocument::shrinkToFit()") {
10192
});
10293
}
10394

104-
SECTION("linked key") {
105-
doc["key"] = 42;
106-
107-
doc.shrinkToFit();
108-
109-
REQUIRE(doc.as<std::string>() == "{\"key\":42}");
110-
REQUIRE(spyingAllocator.log() ==
111-
AllocatorLog{
112-
Allocate(sizeofPool()),
113-
Reallocate(sizeofPool(), sizeofObject(1)),
114-
});
115-
}
116-
117-
SECTION("owned key") {
95+
SECTION("object key") {
11896
doc["abcdefg"_s] = 42;
11997

12098
doc.shrinkToFit();
@@ -128,20 +106,7 @@ TEST_CASE("JsonDocument::shrinkToFit()") {
128106
});
129107
}
130108

131-
SECTION("linked string in array") {
132-
doc.add("hello");
133-
134-
doc.shrinkToFit();
135-
136-
REQUIRE(doc.as<std::string>() == "[\"hello\"]");
137-
REQUIRE(spyingAllocator.log() ==
138-
AllocatorLog{
139-
Allocate(sizeofPool()),
140-
Reallocate(sizeofPool(), sizeofArray(1)),
141-
});
142-
}
143-
144-
SECTION("owned string in array") {
109+
SECTION("string in array") {
145110
doc.add("abcdefg"_s);
146111

147112
doc.shrinkToFit();
@@ -155,20 +120,7 @@ TEST_CASE("JsonDocument::shrinkToFit()") {
155120
});
156121
}
157122

158-
SECTION("linked string in object") {
159-
doc["key"] = "hello";
160-
161-
doc.shrinkToFit();
162-
163-
REQUIRE(doc.as<std::string>() == "{\"key\":\"hello\"}");
164-
REQUIRE(spyingAllocator.log() ==
165-
AllocatorLog{
166-
Allocate(sizeofPool()),
167-
Reallocate(sizeofPool(), sizeofObject(1)),
168-
});
169-
}
170-
171-
SECTION("owned string in object") {
123+
SECTION("string in object") {
172124
doc["key"] = "abcdefg"_s;
173125

174126
doc.shrinkToFit();

0 commit comments

Comments
 (0)