Skip to content

Commit 6d28d99

Browse files
mcsprmcspr
committed
consistent null checks
expect empty cstr for simple methods, avoid doing it w/ length present expect {w,}buffer to be present, things are catastrophic anyways also, redundant wbuffer check for free()
1 parent 1b3b3f8 commit 6d28d99

File tree

2 files changed

+56
-57
lines changed

2 files changed

+56
-57
lines changed

‎cores/esp8266/WString.cpp‎

Lines changed: 51 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,6 @@
3131
/* Actual flash string helpers */
3232
/*********************************************/
3333

34-
/* XCHAL_INSTROM0_VADDR (0x40200000) for flash contents
35-
* XCHAL_INSTRAM0_VADDR (0x40000000) for instram, aka 1 << 30 */
36-
static inline bool __attribute__((always_inline)) __pgm_expected(const void *p)
37-
{
38-
return ((uintptr_t)p & (uintptr_t)((1 << 31) | (1 << 30))) > 0;
39-
}
40-
4134
struct __StringImpl {
4235
// XXX since we are always building w/ C++ headers, these cannot be resolved through the type alone
4336
// don't depend on glibc quirks alone, just try to reduce number of overloads by explicitly requesting them
@@ -58,6 +51,14 @@ struct __StringImpl {
5851
return func;
5952
}
6053

54+
// XCHAL_INSTROM0_VADDR (0x40200000) for flash contents
55+
// XCHAL_INSTRAM0_VADDR (0x40000000) for instram, aka 1 << 30
56+
static inline bool __attribute__((always_inline, const))
57+
__pgm_expected(const void *p)
58+
{
59+
return ((uintptr_t)p & (uintptr_t)((1 << 31) | (1 << 30))) > 0;
60+
}
61+
6162
// jump to specific func depending on the src address
6263
// (similar logic is done in libc, e.g. memmove_P)
6364

@@ -240,13 +241,13 @@ String::String(double value, unsigned char decimalPlaces) :
240241
/*********************************************/
241242

242243
void String::invalidate(void) {
243-
if (!isSSO() && wbuffer())
244+
if (!isSSO())
244245
free(wbuffer());
245246
init();
246247
}
247248

248249
bool String::reserve(unsigned int size) {
249-
if (buffer() && capacity() >= size)
250+
if (capacity() >= size)
250251
return true;
251252
if (changeBuffer(size)) {
252253
if (len() == 0)
@@ -371,8 +372,7 @@ String &String::operator =(const char *cstr) {
371372
}
372373

373374
String &String::operator =(char c) {
374-
char buffer[2] { c, '\0' };
375-
*this = buffer;
375+
copy(&c, 1);
376376
return *this;
377377
}
378378

@@ -402,8 +402,6 @@ bool String::concat(const String &s) {
402402

403403
bool String::concat(const char *str, unsigned int length) {
404404
unsigned int newlen = len() + length;
405-
if (!str)
406-
return false;
407405
if (length == 0)
408406
return true;
409407
if (!reserve(newlen))
@@ -464,16 +462,16 @@ bool String::concat(double num) {
464462
/* Insert */
465463
/*********************************************/
466464

467-
String &String::insert(size_t position, const char *other, size_t other_length) {
468-
if (position > length())
465+
String &String::insert(size_t position, const char *other, unsigned int other_length) {
466+
auto this_length = len();
467+
if (position > this_length)
469468
return *this;
470469

471-
auto len = length();
472-
auto total = len + other_length;
470+
auto total = this_length + other_length;
473471
if (!reserve(total))
474472
return *this;
475473

476-
auto left = len - position;
474+
auto left = this_length - position;
477475
setLen(total);
478476

479477
auto *start = wbuffer() + position;
@@ -490,7 +488,9 @@ String &String::insert(size_t position, char other) {
490488
}
491489

492490
String &String::insert(size_t position, const char *other) {
493-
return insert(position, other, other ? strlen_P(other) : 0);
491+
if (!other)
492+
return *this;
493+
return insert(position, other, strlen_P(other));
494494
}
495495

496496
String &String::insert(size_t position, const String &other) {
@@ -538,12 +538,10 @@ String operator +(char lhs, const String &rhs) {
538538

539539
String operator +(const char *lhs, const String &rhs) {
540540
String res;
541-
542541
const auto lhs_len = lhs ? strlen_P(lhs) : 0;
543542
res.reserve(lhs_len + rhs.length());
544543
res.concat(lhs, lhs_len);
545544
res.concat(rhs);
546-
547545
return res;
548546
}
549547

@@ -570,17 +568,16 @@ int String::compareTo(const String &s) const {
570568
}
571569

572570
int String::compareTo(const char *cstr) const {
573-
return compareToImpl(__StringImpl::select_strncmp(cstr), cstr, cstr ? strlen_P(cstr) : 0);
571+
if (!cstr)
572+
return 1;
573+
return compareToImpl(__StringImpl::select_strncmp(cstr), cstr, strlen_P(cstr));
574574
}
575575

576576
int String::compareTo(const char *str, unsigned int length) const {
577577
return compareToImpl(__StringImpl::select_strncmp(str), str, length);
578578
}
579579

580580
bool String::equalsImpl(internal_strncmp_t impl, const char *str, unsigned int length) const {
581-
if (!str)
582-
return false;
583-
584581
const auto same_length = len() == length;
585582
if (same_length && length == 0)
586583
return true;
@@ -632,7 +629,7 @@ bool String::operator>=(const char *rhs) const {
632629
bool String::equalsIgnoreCaseImpl(internal_strncasecmp_t impl, const char *str, unsigned int length) const {
633630
if (len() != length)
634631
return false;
635-
if (!str || !length)
632+
if (!length)
636633
return true;
637634
return impl(buffer(), str, length) == 0;
638635
}
@@ -683,8 +680,6 @@ unsigned char String::equalsConstantTime(const String& s) const {
683680
}
684681

685682
bool String::startsWithImpl(internal_strncmp_t impl, const char *str, unsigned int length, unsigned int offset) const {
686-
if (!buffer() || !str)
687-
return false;
688683
if (len() < length)
689684
return false;
690685
if (offset > (unsigned)(len() - length))
@@ -701,6 +696,8 @@ bool String::startsWith(const String &prefix) const {
701696
}
702697

703698
bool String::startsWith(const char *prefix) const {
699+
if (!prefix)
700+
return false;
704701
return startsWith(prefix, 0);
705702
}
706703

@@ -709,12 +706,12 @@ bool String::startsWith(const String &prefix, unsigned int offset) const {
709706
}
710707

711708
bool String::startsWith(const char *prefix, unsigned int offset) const {
712-
return startsWithImpl(__StringImpl::select_strncmp(prefix), prefix, prefix ? strlen_P(prefix) : 0, offset);
709+
if (!prefix)
710+
return false;
711+
return startsWithImpl(__StringImpl::select_strncmp(prefix), prefix, strlen_P(prefix), offset);
713712
}
714713

715714
bool String::endsWithImpl(internal_strncmp_t impl, const char *str, unsigned int length) const {
716-
if (!buffer() || !str)
717-
return false;
718715
if (len() < length)
719716
return false;
720717
return impl(&buffer()[len() - length], str, length) == 0;
@@ -729,7 +726,9 @@ bool String::endsWith(const String &suffix) const {
729726
}
730727

731728
bool String::endsWith(const char *suffix) const {
732-
return endsWithImpl(__StringImpl::select_strncmp(suffix), suffix, suffix ? strlen_P(suffix) : 0);
729+
if (!suffix)
730+
return false;
731+
return endsWithImpl(__StringImpl::select_strncmp(suffix), suffix, strlen_P(suffix));
733732
}
734733

735734
/*********************************************/
@@ -743,7 +742,7 @@ void String::setCharAt(unsigned int loc, char c) {
743742

744743
char &String::operator[](unsigned int index) {
745744
static char dummy_writable_char;
746-
if (index >= len() || !buffer()) {
745+
if (index >= len()) {
747746
dummy_writable_char = 0;
748747
return dummy_writable_char;
749748
}
@@ -795,11 +794,13 @@ int String::indexOfImpl(internal_strstr_t impl, const char *str, unsigned int le
795794
}
796795

797796
int String::indexOf(const char *cstr, unsigned int fromIndex) const {
798-
return indexOfImpl(__StringImpl::select_strstr(cstr), cstr, cstr ? strlen_P(cstr) : 0, fromIndex);
797+
if (!cstr)
798+
return -1;
799+
return indexOfImpl(__StringImpl::select_strstr(cstr), cstr, strlen_P(cstr), fromIndex);
799800
}
800801

801-
int String::indexOf(const String &s2, unsigned int fromIndex) const {
802-
return indexOfImpl(__StringImpl::overloaded_strstr(strstr), s2.buffer(), s2.len(), fromIndex);
802+
int String::indexOf(const String &str, unsigned int fromIndex) const {
803+
return indexOfImpl(__StringImpl::overloaded_strstr(strstr), str.buffer(), str.len(), fromIndex);
803804
}
804805

805806
int String::lastIndexOf(char ch) const {
@@ -842,13 +843,16 @@ int String::lastIndexOf(const String &str, unsigned int fromIndex) const {
842843
return lastIndexOf(str.buffer(), str.len(), fromIndex);
843844
}
844845

845-
int String::lastIndexOf(const char *str) const {
846-
const auto length = str ? strlen_P(str) : 0;
847-
return lastIndexOf(str, length, len() - length);
846+
int String::lastIndexOf(const char *cstr) const {
847+
if (!cstr)
848+
return -1;
849+
return lastIndexOf(cstr, strlen_P(cstr), 0);
848850
}
849851

850-
int String::lastIndexOf(const char *str, unsigned int fromIndex) const {
851-
return lastIndexOf(str, str ? strlen_P(str) : 0, fromIndex);
852+
int String::lastIndexOf(const char *cstr, unsigned int fromIndex) const {
853+
if (!cstr)
854+
return -1;
855+
return lastIndexOf(cstr, strlen_P(cstr), fromIndex);
852856
}
853857

854858
String String::substring(unsigned int left, unsigned int right) const {
@@ -871,8 +875,6 @@ String String::substring(unsigned int left, unsigned int right) const {
871875
/*********************************************/
872876

873877
void String::replace(char find, char replace) {
874-
if (!buffer())
875-
return;
876878
for (char *p = wbuffer(); *p; p++) {
877879
if (*p == find)
878880
*p = replace;
@@ -898,7 +900,8 @@ void String::replaceImpl(internal_strstr_t impl, const char *find, unsigned int
898900
memmove_P(writeTo, readFrom, n);
899901

900902
writeTo += n;
901-
memmove_P(writeTo, replace, replace_len);
903+
if (replace_len)
904+
memmove_P(writeTo, replace, replace_len);
902905

903906
writeTo += replace_len;
904907
readFrom = foundAt + find_len;
@@ -973,23 +976,19 @@ void String::remove(unsigned int index, unsigned int count) {
973976
}
974977

975978
void String::toLowerCase(void) {
976-
if (!buffer())
977-
return;
978979
for (char *p = wbuffer(); *p; p++) {
979980
*p = tolower(*p);
980981
}
981982
}
982983

983984
void String::toUpperCase(void) {
984-
if (!buffer())
985-
return;
986985
for (char *p = wbuffer(); *p; p++) {
987986
*p = toupper(*p);
988987
}
989988
}
990989

991990
void String::trim(void) {
992-
if (!buffer() || len() == 0)
991+
if (len() == 0)
993992
return;
994993
char *begin = wbuffer();
995994
while (isspace(*begin))
@@ -1009,19 +1008,19 @@ void String::trim(void) {
10091008
/*********************************************/
10101009

10111010
long String::toInt(void) const {
1012-
if (buffer())
1011+
if (len())
10131012
return atol(buffer());
10141013
return 0;
10151014
}
10161015

10171016
float String::toFloat(void) const {
1018-
if (buffer())
1017+
if (len())
10191018
return atof(buffer());
10201019
return 0.0F;
10211020
}
10221021

10231022
double String::toDouble(void) const {
1024-
if (buffer())
1023+
if (len())
10251024
return atof(buffer());
10261025
return 0.0;
10271026
}

‎cores/esp8266/WString.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -334,8 +334,8 @@ class String {
334334
int lastIndexOf(char ch, unsigned int fromIndex) const;
335335
int lastIndexOf(const String &str) const;
336336
int lastIndexOf(const String &str, unsigned int fromIndex) const;
337-
int lastIndexOf(const char *str) const;
338-
int lastIndexOf(const char *str, unsigned int fromIndex) const;
337+
int lastIndexOf(const char *cstr) const;
338+
int lastIndexOf(const char *cstr, unsigned int fromIndex) const;
339339
int lastIndexOf(const char *str, unsigned int length, unsigned int fromIndex) const;
340340

341341
int lastIndexOf(const __FlashStringHelper *str) const {
@@ -461,12 +461,12 @@ class String {
461461
void replace(const char *find, unsigned int find_len, const char *replace, unsigned int replace_len);
462462

463463
String &insert(size_t position, char);
464-
String &insert(size_t position, const char *);
464+
String &insert(size_t position, const char * str);
465465
String &insert(size_t position, const __FlashStringHelper *str) {
466466
return insert(position, reinterpret_cast<const char *>(str));
467467
}
468-
String &insert(size_t position, const char *, size_t length);
469-
String &insert(size_t position, const String &);
468+
String &insert(size_t position, const char * str, unsigned int length);
469+
String &insert(size_t position, const String &str);
470470

471471
// rvalue helper
472472
void move(String &rhs) noexcept;

0 commit comments

Comments
 (0)