Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 5488d5d

Browse files
authored
Fix crash when using String::move on empty string (#10938) (#10945)
Fixes: #10938 Keep allocated memory when rhs fits Use case: Appending to a String with pre-allocated memory (e.g. from `reserve()`) No need to move 0-termination char in String::move Simplify calls to String::copy A lot of the same checks were done before calling `copy()` which should be done in the `copy()` function itself. String::copy() Should not copy more than given length Fix potential out of range in String::concat There is no prerequisite the given array has to be a 0-terminated char array. So we should only copy the length that has been given. The `setLen()` function will make sure the internal string is 0-terminated. So no need to dangerously assume there will be 1 more byte to copy Allow String::concat(const String &s) with s.buffer() == nullptr When constructing a String object, the internal buffer is a nullptr. However concatenating this to another String would return `false` while this is perfectly fine to do.
1 parent 6fb55a7 commit 5488d5d

File tree

1 file changed

+21
-33
lines changed

1 file changed

+21
-33
lines changed

‎cores/esp32/WString.cpp

Lines changed: 21 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,11 @@ bool String::changeBuffer(unsigned int maxStrLen) {
226226
/*********************************************/
227227

228228
String &String::copy(const char *cstr, unsigned int length) {
229-
if (!reserve(length)) {
229+
if (cstr == nullptr || !reserve(length)) {
230230
invalidate();
231231
return *this;
232232
}
233-
memmove(wbuffer(), cstr, length + 1);
233+
memmove(wbuffer(), cstr, length);
234234
setLen(length);
235235
return *this;
236236
}
@@ -239,15 +239,18 @@ String &String::copy(const char *cstr, unsigned int length) {
239239
void String::move(String &rhs) {
240240
if (buffer()) {
241241
if (capacity() >= rhs.len()) {
242-
memmove(wbuffer(), rhs.buffer(), rhs.length() + 1);
242+
// Use case: When 'reserve()' was called and the first
243+
// assignment/append is the return value of a function.
244+
if (rhs.len() && rhs.buffer()) {
245+
memmove(wbuffer(), rhs.buffer(), rhs.length());
246+
}
243247
setLen(rhs.len());
244248
rhs.invalidate();
245249
return;
246-
} else {
247-
if (!isSSO()) {
248-
free(wbuffer());
249-
setBuffer(nullptr);
250-
}
250+
}
251+
if (!isSSO()) {
252+
free(wbuffer());
253+
setBuffer(nullptr);
251254
}
252255
}
253256
if (rhs.isSSO()) {
@@ -259,23 +262,15 @@ void String::move(String &rhs) {
259262
}
260263
setCapacity(rhs.capacity());
261264
setLen(rhs.len());
262-
rhs.setSSO(false);
263-
rhs.setCapacity(0);
264-
rhs.setBuffer(nullptr);
265-
rhs.setLen(0);
265+
rhs.init();
266266
}
267267
#endif
268268

269269
String &String::operator=(const String &rhs) {
270270
if (this == &rhs) {
271271
return *this;
272272
}
273-
if (rhs.buffer()) {
274-
copy(rhs.buffer(), rhs.len());
275-
} else {
276-
invalidate();
277-
}
278-
return *this;
273+
return copy(rhs.buffer(), rhs.len());
279274
}
280275

281276
#ifdef __GXX_EXPERIMENTAL_CXX0X__
@@ -295,12 +290,7 @@ String &String::operator=(StringSumHelper &&rval) {
295290
#endif
296291

297292
String &String::operator=(const char *cstr) {
298-
if (cstr) {
299-
copy(cstr, strlen(cstr));
300-
} else {
301-
invalidate();
302-
}
303-
return *this;
293+
return copy(cstr, strlen(cstr));
304294
}
305295

306296
/*********************************************/
@@ -311,23 +301,21 @@ bool String::concat(const String &s) {
311301
// Special case if we're concatting ourself (s += s;) since we may end up
312302
// realloc'ing the buffer and moving s.buffer in the method called
313303
if (&s == this) {
314-
unsigned int newlen = 2 * len();
315-
if (!s.buffer()) {
316-
return false;
317-
}
318304
if (s.len() == 0) {
319305
return true;
320306
}
307+
if (!s.buffer()) {
308+
return false;
309+
}
310+
unsigned int newlen = 2 * len();
321311
if (!reserve(newlen)) {
322312
return false;
323313
}
324314
memmove(wbuffer() + len(), buffer(), len());
325315
setLen(newlen);
326-
wbuffer()[len()] = 0;
327316
return true;
328-
} else {
329-
return concat(s.buffer(), s.len());
330317
}
318+
return concat(s.buffer(), s.len());
331319
}
332320

333321
bool String::concat(const char *cstr, unsigned int length) {
@@ -343,10 +331,10 @@ bool String::concat(const char *cstr, unsigned int length) {
343331
}
344332
if (cstr >= wbuffer() && cstr < wbuffer() + len()) {
345333
// compatible with SSO in ram #6155 (case "x += x.c_str()")
346-
memmove(wbuffer() + len(), cstr, length + 1);
334+
memmove(wbuffer() + len(), cstr, length);
347335
} else {
348336
// compatible with source in flash #6367
349-
memcpy_P(wbuffer() + len(), cstr, length + 1);
337+
memcpy_P(wbuffer() + len(), cstr, length);
350338
}
351339
setLen(newlen);
352340
return true;

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /