From 1b56cb2689af04f672bdeae769538b214e4a3a8b Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 31 Mar 2022 00:40:43 +0300 Subject: [PATCH 1/8] WString: unify numeric conversion and fix assignments Restore the pre-3.0.0 behaviour when we could assign numeric values to the string object. After introducing `operator =(char)`, everything was converted to `char` instead of the expected 'stringification' of the number (built-in int, long, unsigned int, unsigned long, long long, unsigned long long, float and double) --- cores/esp8266/WString.cpp | 288 ++++++++++++++++++++++++++++---------- cores/esp8266/WString.h | 29 ++-- 2 files changed, 235 insertions(+), 82 deletions(-) diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp index c6b3c6ae0a..9750bb0bd1 100644 --- a/cores/esp8266/WString.cpp +++ b/cores/esp8266/WString.cpp @@ -32,109 +32,218 @@ #define STR(x) __STRHELPER(x) // stringifier /*********************************************/ -/* Constructors */ +/* Conversion helpers */ /*********************************************/ -String::String(const char *cstr) { - init(); - if (cstr) - copy(cstr, strlen_P(cstr)); -} +static String toString(unsigned char value, unsigned char base) { + String out; -String::String(const String &value) { - init(); - *this = value; -} + char buf[1 + 8 * sizeof(unsigned char)]; + utoa(value, buf, base); + out = buf; -String::String(const __FlashStringHelper *pstr) { - init(); - *this = pstr; // see operator = + return out; } -String::String(String &&rval) noexcept { - init(); - move(rval); +static String toString(unsigned char value) { + return toString(value, 10); } -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); } else { itoa(value, buf, base); } - *this = buf; + out = buf; + + return out; } -String::String(unsigned int value, unsigned char base) { - init(); +static String toString(int value) { + return toString(value, 10); +} + +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 = buf; + + return out; } -String::String(long value, unsigned char base) { - init(); +static String toString(unsigned int value) { + return toString(value, 10); +} + +static String toString(long value, unsigned char base) { + String out; + char buf[2 + 8 * sizeof(long)]; if (base == 10) { sprintf(buf, "%ld", value); } else { ltoa(value, buf, base); } - *this = buf; + out = buf; + + return out; } -String::String(unsigned long value, unsigned char base) { - init(); +static String toString(long value) { + return toString(value, 10); +} + +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 = buf; + + return out; } -String::String(long long value) { - init(); +static String toString(unsigned long value) { + return toString(value, 10); +} + +static String toString(long long value, unsigned char base) { + String out; + + char buf[2 + 8 * sizeof(long long)]; + out = lltoa(value, buf, sizeof(buf), base); + + return out; +} + +static String toString(long long value) { + String out; + char buf[2 + 8 * sizeof(long long)]; sprintf(buf, "%lld", value); - *this = buf; + out = buf; + + 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)]; + out = ulltoa(value, buf, sizeof(buf), base); + + return out; +} + +static String toString(unsigned long long value) { + String out; + char buf[1 + 8 * sizeof(unsigned long long)]; sprintf(buf, "%llu", value); - *this = buf; + out = buf; + + 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(float value) { + return toString(value, 2); +} + +static String toString(double value, unsigned char decimalPlaces) { + String out; + + char buf[33]; + out = dtostrf(value, (decimalPlaces + 2), decimalPlaces, buf); + + return out; +} + +static String toString(double value) { + return toString(value, 2); +} + +/*********************************************/ +/* 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) { + *this = toString(value, base); +} + +String::String(int value, unsigned char base) { + *this = toString(value, base); +} + +String::String(unsigned int value, unsigned char base) { + *this = toString(value, base); +} + +String::String(long value, unsigned char base) { + *this = toString(value, base); +} + +String::String(unsigned long value, unsigned char base) { + *this = toString(value, base); +} + +String::String(long long value, unsigned char base) { + *this = toString(value, base); +} + +String::String(long long value) { + *this = toString(value); +} + +String::String(unsigned long long value, unsigned char base) { + *this = toString(value, base); +} + +String::String(unsigned long long value) { + *this = toString(value); +} + +String::String(float value, unsigned char decimalPlaces) { + *this = toString(value, decimalPlaces); +} + +String::String(double value, unsigned char decimalPlaces) { + *this = toString(value, decimalPlaces); } /*********************************************/ @@ -279,6 +388,50 @@ String &String::operator =(char c) { return *this; } +String &String::operator =(unsigned char value) { + *this = toString(value); + return *this; +} + +String &String::operator =(int value) { + *this = toString(value); + return *this; +} + +String &String::operator =(unsigned int value) { + *this = toString(value); + return *this; +} + +String &String::operator =(long value) { + *this = toString(value); + return *this; +} + +String &String::operator =(unsigned long value) { + *this = toString(value); + return *this; +} + +String &String::operator =(long long value) { + *this = toString(value); + return *this; +} + +String &String::operator =(unsigned long long value) { + *this = toString(value); + return *this; +} + +String &String::operator =(float value) { + *this = toString(value); + return *this; +} + +String &String::operator =(double value) { + *this = toString(value); + return *this; +} /*********************************************/ /* concat */ @@ -329,52 +482,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(toString(num)); } bool String::concat(int num) { - char buf[2 + 3 * sizeof(int)]; - return concat(buf, sprintf(buf, "%d", num)); + return concat(toString(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(toString(num)); } bool String::concat(long num) { - char buf[2 + 3 * sizeof(long)]; - return concat(buf, sprintf(buf, "%ld", num)); + return concat(toString(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(toString(num)); } bool String::concat(long long num) { - char buf[2 + 3 * sizeof(long long)]; - return concat(buf, sprintf(buf, "%lld", num)); + return concat(toString(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(toString(num)); } bool String::concat(float num) { - char buf[20]; - char *string = dtostrf(num, 4, 2, buf); - return concat(string, strlen(string)); + return concat(toString(num)); } bool String::concat(double num) { - char buf[20]; - char *string = dtostrf(num, 4, 2, buf); - return concat(string, strlen(string)); + return concat(toString(num)); } bool String::concat(const __FlashStringHelper *str) { diff --git a/cores/esp8266/WString.h b/cores/esp8266/WString.h index d028c0abfb..b8ca90e10b 100644 --- a/cores/esp8266/WString.h +++ b/cores/esp8266/WString.h @@ -94,22 +94,33 @@ 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 =(char c); + String &operator =(unsigned char c); + String &operator =(int num); + String &operator =(unsigned int num); + String &operator =(long num); + String &operator =(unsigned long num); + String &operator =(long long num); + String &operator =(unsigned long long num); + String &operator =(float num); + String &operator =(double num); + + // 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); @@ -120,8 +131,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 +140,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; } @@ -292,6 +303,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); From a192705533147aa6a2f07597a645b5c14b83154e Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 31 Mar 2022 01:29:09 +0300 Subject: [PATCH 2/8] cannot assign to uninitialized obj, try to placement-new replace the ctor --- cores/esp8266/WString.cpp | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp index 9750bb0bd1..2f3d49f5d8 100644 --- a/cores/esp8266/WString.cpp +++ b/cores/esp8266/WString.cpp @@ -202,48 +202,50 @@ String::String(String &&rval) noexcept { move(rval); } +// placement new is safe enough, but be wary when `this` has something in the hierarchy + String::String(unsigned char value, unsigned char base) { - *this = toString(value, base); + new (this) String(toString(value, base)); } String::String(int value, unsigned char base) { - *this = toString(value, base); + new (this) String(toString(value, base)); } String::String(unsigned int value, unsigned char base) { - *this = toString(value, base); + new (this) String(toString(value, base)); } String::String(long value, unsigned char base) { - *this = toString(value, base); + new (this) String(toString(value, base)); } String::String(unsigned long value, unsigned char base) { - *this = toString(value, base); + new (this) String(toString(value, base)); } String::String(long long value, unsigned char base) { - *this = toString(value, base); + new (this) String(toString(value, base)); } String::String(long long value) { - *this = toString(value); + new (this) String(toString(value)); } String::String(unsigned long long value, unsigned char base) { - *this = toString(value, base); + new (this) String(toString(value, base)); } String::String(unsigned long long value) { - *this = toString(value); + new (this) String(toString(value)); } String::String(float value, unsigned char decimalPlaces) { - *this = toString(value, decimalPlaces); + new (this) String(toString(value, decimalPlaces)); } String::String(double value, unsigned char decimalPlaces) { - *this = toString(value, decimalPlaces); + new (this) String(toString(value, decimalPlaces)); } /*********************************************/ From 7e6780031db7532902123a76e11c75ab0f2e2b76 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 31 Mar 2022 01:29:36 +0300 Subject: [PATCH 3/8] mention the abundance of init() --- cores/esp8266/WString.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cores/esp8266/WString.h b/cores/esp8266/WString.h index b8ca90e10b..f267cf1424 100644 --- a/cores/esp8266/WString.h +++ b/cores/esp8266/WString.h @@ -286,6 +286,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; From 16dff0e9d327fa13471886ebfca80f807de18277 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 31 Mar 2022 01:29:50 +0300 Subject: [PATCH 4/8] implement host test --- tests/host/core/test_string.cpp | 89 +++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) 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"); + } +} From 0ca77028bf268ebaaef63f805553d4edf7e1dcdb Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 31 Mar 2022 01:46:34 +0300 Subject: [PATCH 5/8] oops --- cores/esp8266/WString.cpp | 68 +++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp index 2f3d49f5d8..aed0d17390 100644 --- a/cores/esp8266/WString.cpp +++ b/cores/esp8266/WString.cpp @@ -202,51 +202,49 @@ String::String(String &&rval) noexcept { move(rval); } -// placement new is safe enough, but be wary when `this` has something in the hierarchy +String::String(unsigned char value, unsigned char base) : + String(toString(value, base)) +{} -String::String(unsigned char value, unsigned char base) { - new (this) String(toString(value, base)); -} - -String::String(int value, unsigned char base) { - new (this) String(toString(value, base)); -} +String::String(int value, unsigned char base) : + String(toString(value, base)) +{} -String::String(unsigned int value, unsigned char base) { - new (this) String(toString(value, base)); -} +String::String(unsigned int value, unsigned char base) : + String(toString(value, base)) +{} -String::String(long value, unsigned char base) { - new (this) String(toString(value, base)); -} +String::String(long value, unsigned char base) : + String(toString(value, base)) +{} -String::String(unsigned long value, unsigned char base) { - new (this) String(toString(value, base)); -} +String::String(unsigned long value, unsigned char base) : + String(toString(value, base)) +{} -String::String(long long value, unsigned char base) { - new (this) String(toString(value, base)); -} +String::String(long long value, unsigned char base) : + String(toString(value, base)) +{} -String::String(long long value) { - new (this) String(toString(value)); -} +String::String(long long value) : + String(toString(value)) +{} -String::String(unsigned long long value, unsigned char base) { - new (this) String(toString(value, base)); -} +String::String(unsigned long long value, unsigned char base) : + String(toString(value, base)) +{} -String::String(unsigned long long value) { - new (this) String(toString(value)); -} +String::String(unsigned long long value) : + String(toString(value)) +{} -String::String(float value, unsigned char decimalPlaces) { - new (this) String(toString(value, decimalPlaces)); -} +String::String(float value, unsigned char decimalPlaces) : + String(toString(value, decimalPlaces)) +{} -String::String(double value, unsigned char decimalPlaces) { - new (this) String(toString(value, decimalPlaces)); -} +String::String(double value, unsigned char decimalPlaces) : + String(toString(value, decimalPlaces)) +{} /*********************************************/ /* Memory Management */ From 4206518be226a95d34b2038a0dd78e28a648fe5e Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 31 Mar 2022 02:37:46 +0300 Subject: [PATCH 6/8] trying to reduce the instances of where we specify the default base / decimalPlaces branch via if (base == 10) { sprintf(...) } else { ... } instead of separate funcs reuse the constructor for numeric types where it's possible --- cores/esp8266/WString.cpp | 133 +++++++------------------------------- cores/esp8266/WString.h | 114 +++++++++++++++++++++++++------- 2 files changed, 114 insertions(+), 133 deletions(-) diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp index aed0d17390..44bcfc566a 100644 --- a/cores/esp8266/WString.cpp +++ b/cores/esp8266/WString.cpp @@ -45,10 +45,6 @@ static String toString(unsigned char value, unsigned char base) { return out; } -static String toString(unsigned char value) { - return toString(value, 10); -} - static String toString(int value, unsigned char base) { String out; @@ -63,10 +59,6 @@ static String toString(int value, unsigned char base) { return out; } -static String toString(int value) { - return toString(value, 10); -} - static String toString(unsigned int value, unsigned char base) { String out; @@ -77,10 +69,6 @@ static String toString(unsigned int value, unsigned char base) { return out; } -static String toString(unsigned int value) { - return toString(value, 10); -} - static String toString(long value, unsigned char base) { String out; @@ -95,10 +83,6 @@ static String toString(long value, unsigned char base) { return out; } -static String toString(long value) { - return toString(value, 10); -} - static String toString(unsigned long value, unsigned char base) { String out; @@ -109,25 +93,18 @@ static String toString(unsigned long value, unsigned char base) { return out; } -static String toString(unsigned long value) { - return toString(value, 10); -} +// 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)]; - out = lltoa(value, buf, sizeof(buf), base); - - return out; -} - -static String toString(long long value) { - String out; - - char buf[2 + 8 * sizeof(long long)]; - sprintf(buf, "%lld", value); - out = buf; + if (base == 10) { + sprintf(buf, "%lld", value); + out = buf; + } else { + out = lltoa(value, buf, sizeof(buf), base); + } return out; } @@ -136,17 +113,12 @@ static String toString(unsigned long long value, unsigned char base) { String out; char buf[1 + 8 * sizeof(unsigned long long)]; - out = ulltoa(value, buf, sizeof(buf), base); - - return out; -} - -static String toString(unsigned long long value) { - String out; - - char buf[1 + 8 * sizeof(unsigned long long)]; - sprintf(buf, "%llu", value); - out = buf; + if (base == 10) { + sprintf(buf, "%llu", value); + out = buf; + } else { + out = ulltoa(value, buf, sizeof(buf), base); + } return out; } @@ -160,10 +132,6 @@ static String toString(float value, unsigned char decimalPlaces) { return out; } -static String toString(float value) { - return toString(value, 2); -} - static String toString(double value, unsigned char decimalPlaces) { String out; @@ -173,10 +141,6 @@ static String toString(double value, unsigned char decimalPlaces) { return out; } -static String toString(double value) { - return toString(value, 2); -} - /*********************************************/ /* Constructors */ /*********************************************/ @@ -226,18 +190,10 @@ String::String(long long value, unsigned char base) : String(toString(value, base)) {} -String::String(long long value) : - String(toString(value)) -{} - String::String(unsigned long long value, unsigned char base) : String(toString(value, base)) {} -String::String(unsigned long long value) : - String(toString(value)) -{} - String::String(float value, unsigned char decimalPlaces) : String(toString(value, decimalPlaces)) {} @@ -388,51 +344,6 @@ String &String::operator =(char c) { return *this; } -String &String::operator =(unsigned char value) { - *this = toString(value); - return *this; -} - -String &String::operator =(int value) { - *this = toString(value); - return *this; -} - -String &String::operator =(unsigned int value) { - *this = toString(value); - return *this; -} - -String &String::operator =(long value) { - *this = toString(value); - return *this; -} - -String &String::operator =(unsigned long value) { - *this = toString(value); - return *this; -} - -String &String::operator =(long long value) { - *this = toString(value); - return *this; -} - -String &String::operator =(unsigned long long value) { - *this = toString(value); - return *this; -} - -String &String::operator =(float value) { - *this = toString(value); - return *this; -} - -String &String::operator =(double value) { - *this = toString(value); - return *this; -} - /*********************************************/ /* concat */ /*********************************************/ @@ -482,39 +393,39 @@ bool String::concat(char c) { } bool String::concat(unsigned char num) { - return concat(toString(num)); + return concat(String(num)); } bool String::concat(int num) { - return concat(toString(num)); + return concat(String(num)); } bool String::concat(unsigned int num) { - return concat(toString(num)); + return concat(String(num)); } bool String::concat(long num) { - return concat(toString(num)); + return concat(String(num)); } bool String::concat(unsigned long num) { - return concat(toString(num)); + return concat(String(num)); } bool String::concat(long long num) { - return concat(toString(num)); + return concat(String(num)); } bool String::concat(unsigned long long num) { - return concat(toString(num)); + return concat(String(num)); } bool String::concat(float num) { - return concat(toString(num)); + return concat(String(num)); } bool String::concat(double num) { - return concat(toString(num)); + return concat(String(num)); } bool String::concat(const __FlashStringHelper *str) { diff --git a/cores/esp8266/WString.h b/cores/esp8266/WString.h index f267cf1424..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(); } @@ -99,17 +134,52 @@ class String { String &operator =(String &&rval) noexcept; String &operator =(const char *cstr); String &operator =(const __FlashStringHelper *str); - String &operator =(char c); - String &operator =(unsigned char c); - String &operator =(int num); - String &operator =(unsigned int num); - String &operator =(long num); - String &operator =(unsigned long num); - String &operator =(long long num); - String &operator =(unsigned long long num); - String &operator =(float num); - String &operator =(double num); + + 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) @@ -120,8 +190,8 @@ class String { 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); From 44c8a24a616221d4b39f2c08e245de0837622ae4 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 31 Mar 2022 19:50:56 +0300 Subject: [PATCH 7/8] use pointer from the func, don't default to the &buf[0] nonstd arduino funcs expect this usage pattern --- cores/esp8266/WString.cpp | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp index 44bcfc566a..d4bdc4b416 100644 --- a/cores/esp8266/WString.cpp +++ b/cores/esp8266/WString.cpp @@ -39,8 +39,7 @@ static String toString(unsigned char value, unsigned char base) { String out; char buf[1 + 8 * sizeof(unsigned char)]; - utoa(value, buf, base); - out = buf; + out = utoa(value, buf, base); return out; } @@ -51,10 +50,10 @@ static String toString(int value, unsigned char base) { char buf[2 + 8 * sizeof(int)]; if (base == 10) { sprintf(buf, "%d", value); + out = buf; } else { - itoa(value, buf, base); + out = itoa(value, buf, base); } - out = buf; return out; } @@ -63,8 +62,7 @@ static String toString(unsigned int value, unsigned char base) { String out; char buf[1 + 8 * sizeof(unsigned int)]; - utoa(value, buf, base); - out = buf; + out = utoa(value, buf, base); return out; } @@ -75,10 +73,10 @@ static String toString(long value, unsigned char base) { char buf[2 + 8 * sizeof(long)]; if (base == 10) { sprintf(buf, "%ld", value); + out = buf; } else { - ltoa(value, buf, base); + out = ltoa(value, buf, base); } - out = buf; return out; } @@ -87,8 +85,7 @@ static String toString(unsigned long value, unsigned char base) { String out; char buf[1 + 8 * sizeof(unsigned long)]; - ultoa(value, buf, base); - out = buf; + out = ultoa(value, buf, base); return out; } From d9639490de96fbe328eb4456ad4c408fd42679ed Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 31 Mar 2022 20:10:19 +0300 Subject: [PATCH 8/8] hint about the length of the buffer also -1 line --- cores/esp8266/WString.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp index d4bdc4b416..f77ded194e 100644 --- a/cores/esp8266/WString.cpp +++ b/cores/esp8266/WString.cpp @@ -49,8 +49,7 @@ static String toString(int value, unsigned char base) { char buf[2 + 8 * sizeof(int)]; if (base == 10) { - sprintf(buf, "%d", value); - out = buf; + out.concat(buf, sprintf(buf, "%d", value)); } else { out = itoa(value, buf, base); } @@ -72,8 +71,7 @@ static String toString(long value, unsigned char base) { char buf[2 + 8 * sizeof(long)]; if (base == 10) { - sprintf(buf, "%ld", value); - out = buf; + out.concat(buf, sprintf(buf, "%ld", value)); } else { out = ltoa(value, buf, base); } @@ -97,8 +95,7 @@ static String toString(long long value, unsigned char base) { char buf[2 + 8 * sizeof(long long)]; if (base == 10) { - sprintf(buf, "%lld", value); - out = buf; + out.concat(buf, sprintf(buf, "%lld", value)); } else { out = lltoa(value, buf, sizeof(buf), base); } @@ -111,8 +108,7 @@ static String toString(unsigned long long value, unsigned char base) { char buf[1 + 8 * sizeof(unsigned long long)]; if (base == 10) { - sprintf(buf, "%llu", value); - out = buf; + out.concat(buf, sprintf(buf, "%llu", value)); } else { out = ulltoa(value, buf, sizeof(buf), base); }