WString: unify numeric conversion and fix assignments (#8526)

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)

Add toString() that handles conversions, re-use it through out the class

Fix #8430

cores/esp8266/WString.cpp

@@ -31,6 +31,109 @@
 #define __STRHELPER(x) #x
 #define STR(x) __STRHELPER(x) // stringifier
 
+/*********************************************/
+/*  Conversion helpers                       */
+/*********************************************/
+
+static String toString(unsigned char value, unsigned char base) {
+    String out;
+
+    char buf[1 + 8 * sizeof(unsigned char)];
+    out = utoa(value, buf, base);
+
+    return out;
+}
+
+static String toString(int value, unsigned char base) {
+    String out;
+
+    char buf[2 + 8 * sizeof(int)];
+    if (base == 10) {
+        out.concat(buf, sprintf(buf, "%d", value));
+    } else {
+        out = itoa(value, buf, base);
+    }
+
+    return out;
+}
+
+static String toString(unsigned int value, unsigned char base) {
+    String out;
+
+    char buf[1 + 8 * sizeof(unsigned int)];
+    out = utoa(value, buf, base);
+
+    return out;
+}
+
+static String toString(long value, unsigned char base) {
+    String out;
+
+    char buf[2 + 8 * sizeof(long)];
+    if (base == 10) {
+        out.concat(buf, sprintf(buf, "%ld", value));
+    } else {
+        out = ltoa(value, buf, base);
+    }
+
+    return out;
+}
+
+static String toString(unsigned long value, unsigned char base) {
+    String out;
+
+    char buf[1 + 8 * sizeof(unsigned long)];
+    out = ultoa(value, buf, base);
+
+    return out;
+}
+
+// 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)];
+    if (base == 10) {
+        out.concat(buf, sprintf(buf, "%lld", value));
+    } else {
+        out = lltoa(value, buf, sizeof(buf), base);
+    }
+
+    return out;
+}
+
+static String toString(unsigned long long value, unsigned char base) {
+    String out;
+
+    char buf[1 + 8 * sizeof(unsigned long long)];
+    if (base == 10) {
+        out.concat(buf, sprintf(buf, "%llu", value));
+    } else {
+        out = ulltoa(value, buf, sizeof(buf), base);
+    }
+
+    return out;
+}
+
+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                             */
 /*********************************************/
@@ -56,86 +159,41 @@ String::String(String &&rval) noexcept {
     move(rval);
 }
 
-String::String(unsigned char value, unsigned char base) {
+String::String(unsigned char value, unsigned char base) :
-    init();
+    String(toString(value, base))
-    char buf[1 + 8 * sizeof(unsigned char)];
+{}
-    utoa(value, buf, base);
-    *this = buf;
-}
 
-String::String(int value, unsigned char base) {
+String::String(int value, unsigned char base) :
-    init();
+    String(toString(value, base))
-    char buf[2 + 8 * sizeof(int)];
+{}
-    if (base == 10) {
-        sprintf(buf, "%d", value);
-    } else {
-        itoa(value, buf, base);
-    }
-    *this = buf;
-}
 
-String::String(unsigned int value, unsigned char base) {
+String::String(unsigned int value, unsigned char base) :
-    init();
+    String(toString(value, base))
-    char buf[1 + 8 * sizeof(unsigned int)];
+{}
-    utoa(value, buf, base);
-    *this = buf;
-}
 
-String::String(long value, unsigned char base) {
+String::String(long value, unsigned char base) :
-    init();
+    String(toString(value, base))
-    char buf[2 + 8 * sizeof(long)];
+{}
-    if (base == 10) {
-        sprintf(buf, "%ld", value);
-    } else {
-        ltoa(value, buf, base);
-    }
-    *this = buf;
-}
 
-String::String(unsigned long value, unsigned char base) {
+String::String(unsigned long value, unsigned char base) :
-    init();
+    String(toString(value, base))
-    char buf[1 + 8 * sizeof(unsigned long)];
+{}
-    ultoa(value, buf, base);
-    *this = buf;
-}
 
-String::String(long long value) {
+String::String(long long value, unsigned char base) :
-    init();
+    String(toString(value, base))
-    char buf[2 + 8 * sizeof(long long)];
+{}
-    sprintf(buf, "%lld", value);
-    *this = buf;
-}
 
-String::String(unsigned long long value) {
+String::String(unsigned long long value, unsigned char base) :
-    init();
+    String(toString(value, base))
-    char buf[1 + 8 * sizeof(unsigned long long)];
+{}
-    sprintf(buf, "%llu", value);
-    *this = buf;
-}
 
-String::String(long long value, unsigned char base) {
+String::String(float value, unsigned char decimalPlaces) :
-    init();
+    String(toString(value, decimalPlaces))
-    char buf[2 + 8 * sizeof(long long)];
+{}
-    *this = lltoa(value, buf, sizeof(buf), base);
-}
 
-String::String(unsigned long long value, unsigned char base) {
+String::String(double value, unsigned char decimalPlaces) :
-    init();
+    String(toString(value, decimalPlaces))
-    char buf[1 + 8 * sizeof(unsigned long long)];
+{}
-    *this = ulltoa(value, buf, sizeof(buf), base);
-}
-
-String::String(float value, unsigned char decimalPlaces) {
-    init();
-    char buf[33];
-    *this = dtostrf(value, (decimalPlaces + 2), decimalPlaces, buf);
-}
-
-String::String(double value, unsigned char decimalPlaces) {
-    init();
-    char buf[33];
-    *this = dtostrf(value, (decimalPlaces + 2), decimalPlaces, buf);
-}
 
 /*********************************************/
 /*  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(String(num));
-    return concat(buf, sprintf(buf, "%d", num));
 }
 
 bool String::concat(int num) {
-    char buf[2 + 3 * sizeof(int)];
+    return concat(String(num));
-    return concat(buf, sprintf(buf, "%d", num));
 }
 
 bool String::concat(unsigned int num) {
-    char buf[1 + 3 * sizeof(unsigned int)];
+    return concat(String(num));
-    utoa(num, buf, 10);
-    return concat(buf, strlen(buf));
 }
 
 bool String::concat(long num) {
-    char buf[2 + 3 * sizeof(long)];
+    return concat(String(num));
-    return concat(buf, sprintf(buf, "%ld", num));
 }
 
 bool String::concat(unsigned long num) {
-    char buf[1 + 3 * sizeof(unsigned long)];
+    return concat(String(num));
-    ultoa(num, buf, 10);
-    return concat(buf, strlen(buf));
 }
 
 bool String::concat(long long num) {
-    char buf[2 + 3 * sizeof(long long)];
+    return concat(String(num));
-    return concat(buf, sprintf(buf, "%lld", num));
 }
 
 bool String::concat(unsigned long long num) {
-    char buf[1 + 3 * sizeof(unsigned long long)];
+    return concat(String(num));
-    return concat(buf, sprintf(buf, "%llu", num));
 }
 
 bool String::concat(float num) {
-    char buf[20];
+    return concat(String(num));
-    char *string = dtostrf(num, 4, 2, buf);
-    return concat(string, strlen(string));
 }
 
 bool String::concat(double num) {
-    char buf[20];
+    return concat(String(num));
-    char *string = dtostrf(num, 4, 2, buf);
-    return concat(string, strlen(string));
 }
 
 bool String::concat(const __FlashStringHelper *str) {

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);
+        String(unsigned char, unsigned char base);
-        explicit String(unsigned int, unsigned char base = 10);
+        explicit String(unsigned char value) :
-        explicit String(long, unsigned char base = 10);
+            String(value, 10)
-        explicit String(unsigned long, unsigned char base = 10);
+        {}
-        explicit String(long long /* base 10 */);
+
-        explicit String(long long, unsigned char base);
+        String(int, unsigned char base);
-        explicit String(unsigned long long /* base 10 */);
+        explicit String(int value) :
-        explicit String(unsigned long long, unsigned char base);
+            String(value, 10)
-        explicit String(float, unsigned char decimalPlaces = 2);
+        {}
-        explicit String(double, unsigned char decimalPlaces = 2);
+
+        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
+        // assign string types as well as built-in numeric types
-        // invalid, or if the memory allocation fails, the string will be
-        // marked as invalid ("if (s)" will be false).
         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);
 

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<unsigned char>(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");
+    }
+}