diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp index c6b3c6ae0a..f77ded194e 100644 --- a/cores/esp8266/WString.cpp +++ b/cores/esp8266/WString.cpp @@ -32,111 +32,169 @@ #define STR(x) __STRHELPER(x) // stringifier /*********************************************/ -/* Constructors */ +/* Conversion helpers */ /*********************************************/ -String::String(const char *cstr) { - init(); - if (cstr) - copy(cstr, strlen_P(cstr)); -} - -String::String(const String &value) { - init(); - *this = value; -} +static String toString(unsigned char value, unsigned char base) { + String out; -String::String(const __FlashStringHelper *pstr) { - init(); - *this = pstr; // see operator = -} + char buf[1 + 8 * sizeof(unsigned char)]; + out = utoa(value, buf, base); -String::String(String &&rval) noexcept { - init(); - move(rval); + return out; } -String::String(unsigned char value, unsigned char base) { - init(); - char buf[1 + 8 * sizeof(unsigned char)]; - utoa(value, buf, base); - *this = buf; -} +static String toString(int value, unsigned char base) { + String out; -String::String(int value, unsigned char base) { - init(); char buf[2 + 8 * sizeof(int)]; if (base == 10) { - sprintf(buf, "%d", value); + out.concat(buf, sprintf(buf, "%d", value)); } else { - itoa(value, buf, base); + out = itoa(value, buf, base); } - *this = buf; + + return out; } -String::String(unsigned int value, unsigned char base) { - init(); +static String toString(unsigned int value, unsigned char base) { + String out; + char buf[1 + 8 * sizeof(unsigned int)]; - utoa(value, buf, base); - *this = buf; + out = utoa(value, buf, base); + + return out; } -String::String(long value, unsigned char base) { - init(); +static String toString(long value, unsigned char base) { + String out; + char buf[2 + 8 * sizeof(long)]; if (base == 10) { - sprintf(buf, "%ld", value); + out.concat(buf, sprintf(buf, "%ld", value)); } else { - ltoa(value, buf, base); + out = ltoa(value, buf, base); } - *this = buf; + + return out; } -String::String(unsigned long value, unsigned char base) { - init(); +static String toString(unsigned long value, unsigned char base) { + String out; + char buf[1 + 8 * sizeof(unsigned long)]; - ultoa(value, buf, base); - *this = buf; + out = ultoa(value, buf, base); + + return out; } -String::String(long long value) { - init(); +// TODO: {u,}lltoa don't guarantee that the buffer is usable directly, one should always use the returned pointer + +static String toString(long long value, unsigned char base) { + String out; + char buf[2 + 8 * sizeof(long long)]; - sprintf(buf, "%lld", value); - *this = buf; + if (base == 10) { + out.concat(buf, sprintf(buf, "%lld", value)); + } else { + out = lltoa(value, buf, sizeof(buf), base); + } + + return out; } -String::String(unsigned long long value) { - init(); +static String toString(unsigned long long value, unsigned char base) { + String out; + char buf[1 + 8 * sizeof(unsigned long long)]; - sprintf(buf, "%llu", value); - *this = buf; + if (base == 10) { + out.concat(buf, sprintf(buf, "%llu", value)); + } else { + out = ulltoa(value, buf, sizeof(buf), base); + } + + return out; } -String::String(long long value, unsigned char base) { +static String toString(float value, unsigned char decimalPlaces) { + String out; + + char buf[33]; + out = dtostrf(value, (decimalPlaces + 2), decimalPlaces, buf); + + return out; +} + +static String toString(double value, unsigned char decimalPlaces) { + String out; + + char buf[33]; + out = dtostrf(value, (decimalPlaces + 2), decimalPlaces, buf); + + return out; +} + +/*********************************************/ +/* Constructors */ +/*********************************************/ + +String::String(const char *cstr) { init(); - char buf[2 + 8 * sizeof(long long)]; - *this = lltoa(value, buf, sizeof(buf), base); + if (cstr) + copy(cstr, strlen_P(cstr)); } -String::String(unsigned long long value, unsigned char base) { +String::String(const String &value) { init(); - char buf[1 + 8 * sizeof(unsigned long long)]; - *this = ulltoa(value, buf, sizeof(buf), base); + *this = value; } -String::String(float value, unsigned char decimalPlaces) { +String::String(const __FlashStringHelper *pstr) { init(); - char buf[33]; - *this = dtostrf(value, (decimalPlaces + 2), decimalPlaces, buf); + *this = pstr; // see operator = } -String::String(double value, unsigned char decimalPlaces) { +String::String(String &&rval) noexcept { init(); - char buf[33]; - *this = dtostrf(value, (decimalPlaces + 2), decimalPlaces, buf); + move(rval); } +String::String(unsigned char value, unsigned char base) : + String(toString(value, base)) +{} + +String::String(int value, unsigned char base) : + String(toString(value, base)) +{} + +String::String(unsigned int value, unsigned char base) : + String(toString(value, base)) +{} + +String::String(long value, unsigned char base) : + String(toString(value, base)) +{} + +String::String(unsigned long value, unsigned char base) : + String(toString(value, base)) +{} + +String::String(long long value, unsigned char base) : + String(toString(value, base)) +{} + +String::String(unsigned long long value, unsigned char base) : + String(toString(value, base)) +{} + +String::String(float value, unsigned char decimalPlaces) : + String(toString(value, decimalPlaces)) +{} + +String::String(double value, unsigned char decimalPlaces) : + String(toString(value, decimalPlaces)) +{} + /*********************************************/ /* Memory Management */ /*********************************************/ @@ -279,7 +337,6 @@ String &String::operator =(char c) { return *this; } - /*********************************************/ /* concat */ /*********************************************/ @@ -329,52 +386,39 @@ bool String::concat(char c) { } bool String::concat(unsigned char num) { - char buf[1 + 3 * sizeof(unsigned char)]; - return concat(buf, sprintf(buf, "%d", num)); + return concat(String(num)); } bool String::concat(int num) { - char buf[2 + 3 * sizeof(int)]; - return concat(buf, sprintf(buf, "%d", num)); + return concat(String(num)); } bool String::concat(unsigned int num) { - char buf[1 + 3 * sizeof(unsigned int)]; - utoa(num, buf, 10); - return concat(buf, strlen(buf)); + return concat(String(num)); } bool String::concat(long num) { - char buf[2 + 3 * sizeof(long)]; - return concat(buf, sprintf(buf, "%ld", num)); + return concat(String(num)); } bool String::concat(unsigned long num) { - char buf[1 + 3 * sizeof(unsigned long)]; - ultoa(num, buf, 10); - return concat(buf, strlen(buf)); + return concat(String(num)); } bool String::concat(long long num) { - char buf[2 + 3 * sizeof(long long)]; - return concat(buf, sprintf(buf, "%lld", num)); + return concat(String(num)); } bool String::concat(unsigned long long num) { - char buf[1 + 3 * sizeof(unsigned long long)]; - return concat(buf, sprintf(buf, "%llu", num)); + return concat(String(num)); } bool String::concat(float num) { - char buf[20]; - char *string = dtostrf(num, 4, 2, buf); - return concat(string, strlen(string)); + return concat(String(num)); } bool String::concat(double num) { - char buf[20]; - char *string = dtostrf(num, 4, 2, buf); - return concat(string, strlen(string)); + return concat(String(num)); } bool String::concat(const __FlashStringHelper *str) { diff --git a/cores/esp8266/WString.h b/cores/esp8266/WString.h index d028c0abfb..780c7fdf60 100644 --- a/cores/esp8266/WString.h +++ b/cores/esp8266/WString.h @@ -64,17 +64,52 @@ class String { sso.len = 1; sso.isHeap = 0; } - explicit String(unsigned char, unsigned char base = 10); - explicit String(int, unsigned char base = 10); - explicit String(unsigned int, unsigned char base = 10); - explicit String(long, unsigned char base = 10); - explicit String(unsigned long, unsigned char base = 10); - explicit String(long long /* base 10 */); - explicit String(long long, unsigned char base); - explicit String(unsigned long long /* base 10 */); - explicit String(unsigned long long, unsigned char base); - explicit String(float, unsigned char decimalPlaces = 2); - explicit String(double, unsigned char decimalPlaces = 2); + + String(unsigned char, unsigned char base); + explicit String(unsigned char value) : + String(value, 10) + {} + + String(int, unsigned char base); + explicit String(int value) : + String(value, 10) + {} + + String(unsigned int, unsigned char base); + explicit String(unsigned int value) : + String(value, 10) + {} + + String(long, unsigned char base); + explicit String(long value) : + String(value, 10) + {} + + String(unsigned long, unsigned char base); + explicit String(unsigned long value) : + String(value, 10) + {} + + String(long long, unsigned char base); + explicit String(long long value) : + String(value, 10) + {} + + String(unsigned long long, unsigned char base); + explicit String(unsigned long long value) : + String(value, 10) + {} + + String(float, unsigned char decimalPlaces); + explicit String(float value) : + String(value, 2) + {} + + String(double, unsigned char decimalPlaces); + explicit String(double value) : + String(value, 2) + {} + ~String() { invalidate(); } @@ -94,23 +129,69 @@ class String { return length() == 0; } - // creates a copy of the assigned value. if the value is null or - // invalid, or if the memory allocation fails, the string will be - // marked as invalid ("if (s)" will be false). + // assign string types as well as built-in numeric types String &operator =(const String &rhs); + String &operator =(String &&rval) noexcept; String &operator =(const char *cstr); String &operator =(const __FlashStringHelper *str); - String &operator =(String &&rval) noexcept; String &operator =(char c); - // concatenate (works w/ built-in types) + String &operator =(unsigned char value) { + *this = String(value); + return *this; + } + + String &operator =(int value) { + *this = String(value); + return *this; + } + + String &operator =(unsigned int value) { + *this = String(value); + return *this; + } + + String &operator =(long value) { + *this = String(value); + return *this; + } + + String &operator =(unsigned long value) { + *this = String(value); + return *this; + } + + String &operator =(long long value) { + *this = String(value); + return *this; + } + + String &operator =(unsigned long long value) { + *this = String(value); + return *this; + } + + String &operator =(float value) { + *this = String(value); + return *this; + } + + String &operator =(double value) { + *this = String(value); + return *this; + } + + // concatenate (works w/ built-in types, same as assignment) // returns true on success, false on failure (in which case, the string // is left unchanged). if the argument is null or invalid, the // concatenation is considered unsuccessful. bool concat(const String &str); bool concat(const char *cstr); + bool concat(const char *cstr, unsigned int length); + bool concat(const __FlashStringHelper *str); bool concat(char c); + bool concat(unsigned char c); bool concat(int num); bool concat(unsigned int num); @@ -120,8 +201,6 @@ class String { bool concat(unsigned long long num); bool concat(float num); bool concat(double num); - bool concat(const __FlashStringHelper *str); - bool concat(const char *cstr, unsigned int length); // if there's not enough memory for the concatenated value, the string // will be left unchanged (but this isn't signalled in any way) @@ -131,6 +210,8 @@ class String { return *this; } + // checks whether the internal buffer pointer is set. + // (should not be the case for us, since we always reset the pointer to the SSO buffer instead of setting it to nullptr) explicit operator bool() const { return buffer() != nullptr; } @@ -275,6 +356,8 @@ class String { friend String operator +(const __FlashStringHelper *lhs, String &&rhs); protected: + // TODO: replace init() with a union constructor, so it's called implicitly + void init(void) __attribute__((always_inline)) { sso.buff[0] = 0; sso.len = 0; @@ -292,6 +375,8 @@ class String { // Unfortunately, GCC seems not to re-evaluate the cost of inlining after the store-merging optimizer stage, // `always_inline` attribute is necessary in order to keep inlining. } + + // resets the string storage to the initial state void invalidate(void); bool changeBuffer(unsigned int maxStrLen); diff --git a/tests/host/core/test_string.cpp b/tests/host/core/test_string.cpp index 25de913108..1a235ea92a 100644 --- a/tests/host/core/test_string.cpp +++ b/tests/host/core/test_string.cpp @@ -611,3 +611,92 @@ TEST_CASE("String concat OOB #8198", "[core][String]") REQUIRE(!strcmp(s.c_str(), "abcdxxxxxxxxxxxxxxxx")); free(p); } + +TEST_CASE("String operator =(value) #8430", "[core][String]") +{ + // just like String(char), replace the string with a single char + { + String str { "123456789" }; + str = '\n'; + REQUIRE(str.length() == 1); + REQUIRE(str[0] == '\n'); + } + + // just like String(..., 10) where ... is a numeric type + // (base10 implicitly, since we don't expect an operator call with a 2nd argument) + { + String str { "99u3pokaposdas" }; + str = static_cast(123); + REQUIRE(str.length() == 3); + REQUIRE(str == "123"); + } + + { + String str { "adaj019j310923" }; + + unsigned int a { 8712373 }; + str = a; + REQUIRE(str.length() == 7); + REQUIRE(str == "8712373"); + + unsigned long b { 4231235 }; + str = b; + REQUIRE(str.length() == 7); + REQUIRE(str == "4231235"); + } + + { + String str { "123123124" }; + + int a { 123456 }; + str = a; + REQUIRE(str.length() == 6); + REQUIRE(str == "123456"); + + long b { 7654321 }; + str = b; + REQUIRE(str.length() == 7); + REQUIRE(str == "7654321"); + } + + { + String str { "adaj019j310923" }; + + long long a { 1234567890123456 }; + str = a; + REQUIRE(str.length() == 16); + REQUIRE(str == "1234567890123456"); + } + + { + String str { "lkojqwlekmas" }; + + unsigned long long a { 851238718912 }; + str = a; + REQUIRE(str.length() == 12); + REQUIRE(str == "851238718912"); + } + + // floating-point are specifically base10 + // expected to work like String(..., 2) + // + // may not be the best idea though, due to the dtostrf implementation + // and it's rounding logic may change at any point + { + String str { "qaje09`sjdsas" }; + + float a { 5.123 }; + str = a; + REQUIRE(str.length() == 4); + REQUIRE(str == "5.12"); + } + + { + String str { "9u1omasldmas" }; + + double a { 123.45 }; + str = a; + REQUIRE(str.length() == 6); + REQUIRE(str == "123.45"); + } +}