Skip to content

Commit 594dc70

Browse files
bblanchonbblanchon
committed
Change string copy policy: only string literal are stored by pointer
1 parent 5f8e3c0 commit 594dc70

File tree

27 files changed

+454
-195
lines changed

27 files changed

+454
-195
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ HEAD
66

77
* Fix support for NUL characters in `deserializeJson()`
88
* Make `ElementProxy` and `MemberProxy` non-copyable
9+
* Change string copy policy: only string literal are stored by pointer
910

1011
> ### BREAKING CHANGES
1112
>

‎extras/tests/JsonArray/add.cpp‎

Lines changed: 91 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -17,31 +17,112 @@ TEST_CASE("JsonArray::add(T)") {
1717

1818
SECTION("int") {
1919
array.add(123);
20+
2021
REQUIRE(123 == array[0].as<int>());
2122
REQUIRE(array[0].is<int>());
2223
REQUIRE(array[0].is<double>());
24+
REQUIRE(spy.log() == AllocatorLog{
25+
Allocate(sizeofPool()),
26+
});
2327
}
2428

2529
SECTION("double") {
2630
array.add(123.45);
31+
2732
REQUIRE(123.45 == array[0].as<double>());
2833
REQUIRE(array[0].is<double>());
2934
REQUIRE_FALSE(array[0].is<bool>());
35+
REQUIRE(spy.log() == AllocatorLog{
36+
Allocate(sizeofPool()),
37+
});
3038
}
3139

3240
SECTION("bool") {
3341
array.add(true);
34-
REQUIRE(true == array[0].as<bool>());
42+
43+
REQUIRE(array[0].as<bool>() == true);
3544
REQUIRE(array[0].is<bool>());
3645
REQUIRE_FALSE(array[0].is<int>());
46+
REQUIRE(spy.log() == AllocatorLog{
47+
Allocate(sizeofPool()),
48+
});
49+
}
50+
51+
SECTION("string literal") {
52+
array.add("hello");
53+
54+
REQUIRE(array[0].as<std::string>() == "hello");
55+
REQUIRE(array[0].is<const char*>());
56+
REQUIRE(array[0].is<int>() == false);
57+
REQUIRE(spy.log() == AllocatorLog{
58+
Allocate(sizeofPool()),
59+
});
60+
}
61+
62+
SECTION("std::string") {
63+
array.add("hello"_s);
64+
65+
REQUIRE(array[0].as<std::string>() == "hello");
66+
REQUIRE(array[0].is<const char*>() == true);
67+
REQUIRE(array[0].is<int>() == false);
68+
REQUIRE(spy.log() == AllocatorLog{
69+
Allocate(sizeofPool()),
70+
Allocate(sizeofString("hello")),
71+
});
3772
}
3873

3974
SECTION("const char*") {
4075
const char* str = "hello";
4176
array.add(str);
42-
REQUIRE(str == array[0].as<std::string>());
43-
REQUIRE(array[0].is<const char*>());
44-
REQUIRE_FALSE(array[0].is<int>());
77+
78+
REQUIRE(array[0].as<std::string>() == "hello");
79+
REQUIRE(array[0].as<const char*>() != str);
80+
REQUIRE(array[0].is<const char*>() == true);
81+
REQUIRE(array[0].is<int>() == false);
82+
REQUIRE(spy.log() == AllocatorLog{
83+
Allocate(sizeofPool()),
84+
Allocate(sizeofString("hello")),
85+
});
86+
}
87+
88+
SECTION("serialized(const char*)") {
89+
array.add(serialized("{}"));
90+
91+
REQUIRE(doc.as<std::string>() == "[{}]");
92+
REQUIRE(spy.log() == AllocatorLog{
93+
Allocate(sizeofPool()),
94+
Allocate(sizeofString("{}")),
95+
});
96+
}
97+
98+
SECTION("serialized(char*)") {
99+
array.add(serialized(const_cast<char*>("{}")));
100+
101+
REQUIRE(doc.as<std::string>() == "[{}]");
102+
REQUIRE(spy.log() == AllocatorLog{
103+
Allocate(sizeofPool()),
104+
Allocate(sizeofString("{}")),
105+
});
106+
}
107+
108+
SECTION("serialized(std::string)") {
109+
array.add(serialized("{}"_s));
110+
111+
REQUIRE(doc.as<std::string>() == "[{}]");
112+
REQUIRE(spy.log() == AllocatorLog{
113+
Allocate(sizeofPool()),
114+
Allocate(sizeofString("{}")),
115+
});
116+
}
117+
118+
SECTION("serialized(std::string)") {
119+
array.add(serialized("\0XX"_s));
120+
121+
REQUIRE(doc.as<std::string>() == "[\0XX]"_s);
122+
REQUIRE(spy.log() == AllocatorLog{
123+
Allocate(sizeofPool()),
124+
Allocate(sizeofString(" XX")),
125+
});
45126
}
46127

47128
#ifdef HAS_VARIABLE_LENGTH_ARRAY
@@ -52,7 +133,12 @@ TEST_CASE("JsonArray::add(T)") {
52133

53134
array.add(vla);
54135

55-
REQUIRE("world"_s == array[0]);
136+
strcpy(vla, "hello");
137+
REQUIRE(array[0] == "world"_s);
138+
REQUIRE(spy.log() == AllocatorLog{
139+
Allocate(sizeofPool()),
140+
Allocate(sizeofString("hello")),
141+
});
56142
}
57143
#endif
58144

@@ -99,61 +185,6 @@ TEST_CASE("JsonArray::add(T)") {
99185

100186
REQUIRE(str == array[0]);
101187
}
102-
103-
SECTION("should not duplicate const char*") {
104-
array.add("world");
105-
REQUIRE(spy.log() == AllocatorLog{
106-
Allocate(sizeofPool()),
107-
});
108-
}
109-
110-
SECTION("should duplicate char*") {
111-
array.add(const_cast<char*>("world"));
112-
REQUIRE(spy.log() == AllocatorLog{
113-
Allocate(sizeofPool()),
114-
Allocate(sizeofString("world")),
115-
});
116-
}
117-
118-
SECTION("should duplicate std::string") {
119-
array.add("world"_s);
120-
REQUIRE(spy.log() == AllocatorLog{
121-
Allocate(sizeofPool()),
122-
Allocate(sizeofString("world")),
123-
});
124-
}
125-
126-
SECTION("should duplicate serialized(const char*)") {
127-
array.add(serialized("{}"));
128-
REQUIRE(spy.log() == AllocatorLog{
129-
Allocate(sizeofPool()),
130-
Allocate(sizeofString("{}")),
131-
});
132-
}
133-
134-
SECTION("should duplicate serialized(char*)") {
135-
array.add(serialized(const_cast<char*>("{}")));
136-
REQUIRE(spy.log() == AllocatorLog{
137-
Allocate(sizeofPool()),
138-
Allocate(sizeofString("{}")),
139-
});
140-
}
141-
142-
SECTION("should duplicate serialized(std::string)") {
143-
array.add(serialized("{}"_s));
144-
REQUIRE(spy.log() == AllocatorLog{
145-
Allocate(sizeofPool()),
146-
Allocate(sizeofString("{}")),
147-
});
148-
}
149-
150-
SECTION("should duplicate serialized(std::string)") {
151-
array.add(serialized("\0XX"_s));
152-
REQUIRE(spy.log() == AllocatorLog{
153-
Allocate(sizeofPool()),
154-
Allocate(sizeofString(" XX")),
155-
});
156-
}
157188
}
158189

159190
TEST_CASE("JsonArray::add<T>()") {

‎extras/tests/JsonArray/subscript.cpp‎

Lines changed: 43 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,55 @@ TEST_CASE("JsonArray::operator[]") {
5959
REQUIRE(false == array[0].is<int>());
6060
}
6161

62+
SECTION("string literal") {
63+
array[0] = "hello";
64+
65+
REQUIRE(array[0].as<std::string>() == "hello");
66+
REQUIRE(true == array[0].is<const char*>());
67+
REQUIRE(false == array[0].is<int>());
68+
REQUIRE(spy.log() == AllocatorLog{
69+
Allocate(sizeofPool()),
70+
Allocate(sizeofString("world")),
71+
});
72+
}
73+
6274
SECTION("const char*") {
6375
const char* str = "hello";
64-
6576
array[0] = str;
66-
REQUIRE(str == array[0].as<const char*>());
77+
78+
REQUIRE(array[0].as<std::string>() == "hello");
79+
REQUIRE(true == array[0].is<const char*>());
80+
REQUIRE(false == array[0].is<int>());
81+
REQUIRE(spy.log() == AllocatorLog{
82+
Allocate(sizeofPool()),
83+
Allocate(sizeofString("world")),
84+
});
85+
}
86+
87+
SECTION("std::string") {
88+
array[0] = "hello"_s;
89+
90+
REQUIRE(array[0].as<std::string>() == "hello");
6791
REQUIRE(true == array[0].is<const char*>());
6892
REQUIRE(false == array[0].is<int>());
93+
REQUIRE(spy.log() == AllocatorLog{
94+
Allocate(sizeofPool()),
95+
Allocate(sizeofString("world")),
96+
});
97+
}
98+
99+
#ifdef HAS_VARIABLE_LENGTH_ARRAY
100+
SECTION("VLA") {
101+
size_t i = 16;
102+
char vla[i];
103+
strcpy(vla, "world");
104+
105+
array.add("hello");
106+
array[0] = vla;
107+
108+
REQUIRE(array[0] == "world"_s);
69109
}
110+
#endif
70111

71112
SECTION("nested array") {
72113
JsonDocument doc2;
@@ -114,58 +155,11 @@ TEST_CASE("JsonArray::operator[]") {
114155
REQUIRE(str == array[0]);
115156
}
116157

117-
SECTION("should not duplicate const char*") {
118-
array[0] = "world";
119-
REQUIRE(spy.log() == AllocatorLog{
120-
Allocate(sizeofPool()),
121-
});
122-
}
123-
124-
SECTION("should duplicate char*") {
125-
array[0] = const_cast<char*>("world");
126-
REQUIRE(spy.log() == AllocatorLog{
127-
Allocate(sizeofPool()),
128-
Allocate(sizeofString("world")),
129-
});
130-
}
131-
132-
SECTION("should duplicate std::string") {
133-
array[0] = "world"_s;
134-
REQUIRE(spy.log() == AllocatorLog{
135-
Allocate(sizeofPool()),
136-
Allocate(sizeofString("world")),
137-
});
138-
}
139-
140158
SECTION("array[0].to<JsonObject>()") {
141159
JsonObject obj = array[0].to<JsonObject>();
142160
REQUIRE(obj.isNull() == false);
143161
}
144162

145-
#ifdef HAS_VARIABLE_LENGTH_ARRAY
146-
SECTION("set(VLA)") {
147-
size_t i = 16;
148-
char vla[i];
149-
strcpy(vla, "world");
150-
151-
array.add("hello");
152-
array[0].set(vla);
153-
154-
REQUIRE("world"_s == array[0]);
155-
}
156-
157-
SECTION("operator=(VLA)") {
158-
size_t i = 16;
159-
char vla[i];
160-
strcpy(vla, "world");
161-
162-
array.add("hello");
163-
array[0] = vla;
164-
165-
REQUIRE("world"_s == array[0]);
166-
}
167-
#endif
168-
169163
SECTION("Use a JsonVariant as index") {
170164
array[0] = 1;
171165
array[1] = 2;

‎extras/tests/JsonDocument/ElementProxy.cpp‎

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,44 +5,71 @@
55
#include <ArduinoJson.h>
66
#include <catch.hpp>
77

8+
#include "Allocators.hpp"
89
#include "Literals.hpp"
910

1011
using ElementProxy = ArduinoJson::detail::ElementProxy<JsonDocument&>;
1112

1213
TEST_CASE("ElementProxy::add()") {
13-
JsonDocument doc;
14+
SpyingAllocator spy;
15+
JsonDocument doc(&spy);
1416
doc.add<JsonVariant>();
1517
const ElementProxy& ep = doc[0];
1618

17-
SECTION("add(int)") {
19+
SECTION("integer") {
1820
ep.add(42);
1921

2022
REQUIRE(doc.as<std::string>() == "[[42]]");
23+
REQUIRE(spy.log() == AllocatorLog{
24+
Allocate(sizeofPool()),
25+
});
2126
}
2227

23-
SECTION("add(const char*)") {
28+
SECTION("string literal") {
2429
ep.add("world");
2530

2631
REQUIRE(doc.as<std::string>() == "[[\"world\"]]");
32+
REQUIRE(spy.log() == AllocatorLog{
33+
Allocate(sizeofPool()),
34+
});
35+
}
36+
37+
SECTION("const char*") {
38+
const char* s = "world";
39+
ep.add(s);
40+
41+
REQUIRE(doc.as<std::string>() == "[[\"world\"]]");
42+
REQUIRE(spy.log() == AllocatorLog{
43+
Allocate(sizeofPool()),
44+
Allocate(sizeofString("world")),
45+
});
2746
}
2847

29-
SECTION("add(char[])") {
48+
SECTION("char[]") {
3049
char s[] = "world";
3150
ep.add(s);
3251
strcpy(s, "!!!!!");
3352

3453
REQUIRE(doc.as<std::string>() == "[[\"world\"]]");
54+
REQUIRE(spy.log() == AllocatorLog{
55+
Allocate(sizeofPool()),
56+
Allocate(sizeofString("world")),
57+
});
3558
}
3659

3760
#ifdef HAS_VARIABLE_LENGTH_ARRAY
38-
SECTION("set(vla)") {
61+
SECTION("VLA") {
3962
size_t i = 8;
4063
char vla[i];
4164
strcpy(vla, "world");
4265

4366
ep.add(vla);
4467

4568
REQUIRE(doc.as<std::string>() == "[[\"world\"]]");
69+
REQUIRE(spy.log() == AllocatorLog{
70+
Allocate(sizeofPool()),
71+
Allocate(sizeofString("world")),
72+
});
4673
}
4774
#endif
4875
}

0 commit comments

Comments
 (0)