From 4e9b55908753fd7e7a84ebe5a3a8f3f4c55bb928 Mon Sep 17 00:00:00 2001
From: Maxim Prokhorov <prokhorov.max@outlook.com>
Date: Thu, 25 Mar 2021 02:25:18 +0300
Subject: [PATCH] WString: return bool instead of unsigned char

Clean up the intent, resulting assembly stays the same.
---
 cores/esp8266/WString.cpp | 110 +++++++++++++++++++-------------------
 cores/esp8266/WString.h   |  81 +++++++++++++---------------
 2 files changed, 92 insertions(+), 99 deletions(-)

diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp
index 93b9c43a59..2fea3bfaf6 100644
--- a/cores/esp8266/WString.cpp
+++ b/cores/esp8266/WString.cpp
@@ -147,18 +147,18 @@ void String::invalidate(void) {
     init();
 }
 
-unsigned char String::reserve(unsigned int size) {
+bool String::reserve(unsigned int size) {
     if (buffer() && capacity() >= size)
-        return 1;
+        return true;
     if (changeBuffer(size)) {
         if (len() == 0)
             wbuffer()[0] = 0;
-        return 1;
+        return true;
     }
-    return 0;
+    return false;
 }
 
-unsigned char String::changeBuffer(unsigned int maxStrLen) {
+bool String::changeBuffer(unsigned int maxStrLen) {
     // Can we use SSO here to avoid allocation?
     if (maxStrLen < sizeof(sso.buff) - 1) {
         if (isSSO() || !buffer()) {
@@ -175,7 +175,7 @@ unsigned char String::changeBuffer(unsigned int maxStrLen) {
             memcpy(wbuffer(), temp, maxStrLen);
             free((void *)temp);
         }
-        return 1;
+        return true;
     }
     // Fallthrough to normal allocator
     size_t newSize = (maxStrLen + 16) & (~0xf);
@@ -189,7 +189,7 @@ unsigned char String::changeBuffer(unsigned int maxStrLen) {
 #endif
     // Make sure we can fit newsize in the buffer
     if (newSize > CAPACITY_MAX) {
-        return 0;
+        return false;
     }
     uint16_t oldLen = len();
     char *newbuffer = (char *)realloc(isSSO() ? nullptr : wbuffer(), newSize);
@@ -206,9 +206,9 @@ unsigned char String::changeBuffer(unsigned int maxStrLen) {
         setCapacity(newSize - 1);
         setLen(oldLen); // Needed in case of SSO where len() never existed
         setBuffer(newbuffer);
-        return 1;
+        return true;
     }
-    return 0;
+    return false;
 }
 
 /*********************************************/
@@ -277,111 +277,111 @@ String &String::operator =(const __FlashStringHelper *pstr) {
 /*  concat                                   */
 /*********************************************/
 
-unsigned char String::concat(const String &s) {
+bool String::concat(const String &s) {
     // Special case if we're concatting ourself (s += s;) since we may end up
     // realloc'ing the buffer and moving s.buffer in the method called
     if (&s == this) {
         unsigned int newlen = 2 * len();
         if (!s.buffer())
-            return 0;
+            return false;
         if (s.len() == 0)
-            return 1;
+            return true;
         if (!reserve(newlen))
-            return 0;
+            return false;
         memmove_P(wbuffer() + len(), buffer(), len());
         setLen(newlen);
         wbuffer()[newlen] = 0;
-        return 1;
+        return true;
     } else {
         return concat(s.buffer(), s.len());
     }
 }
 
-unsigned char String::concat(const char *cstr, unsigned int length) {
+bool String::concat(const char *cstr, unsigned int length) {
     unsigned int newlen = len() + length;
     if (!cstr)
-        return 0;
+        return false;
     if (length == 0)
-        return 1;
+        return true;
     if (!reserve(newlen))
-        return 0;
+        return false;
     memmove_P(wbuffer() + len(), cstr, length + 1);
     setLen(newlen);
     wbuffer()[newlen] = 0;
-    return 1;
+    return true;
 }
 
-unsigned char String::concat(const char *cstr) {
+bool String::concat(const char *cstr) {
     if (!cstr)
-        return 0;
+        return false;
     return concat(cstr, strlen(cstr));
 }
 
-unsigned char String::concat(char c) {
+bool String::concat(char c) {
     return concat(&c, 1);
 }
 
-unsigned char String::concat(unsigned char num) {
+bool String::concat(unsigned char num) {
     char buf[1 + 3 * sizeof(unsigned char)];
     return concat(buf, sprintf(buf, "%d", num));
 }
 
-unsigned char String::concat(int num) {
+bool String::concat(int num) {
     char buf[2 + 3 * sizeof(int)];
     return concat(buf, sprintf(buf, "%d", num));
 }
 
-unsigned char String::concat(unsigned int num) {
+bool String::concat(unsigned int num) {
     char buf[1 + 3 * sizeof(unsigned int)];
     utoa(num, buf, 10);
     return concat(buf, strlen(buf));
 }
 
-unsigned char String::concat(long num) {
+bool String::concat(long num) {
     char buf[2 + 3 * sizeof(long)];
     return concat(buf, sprintf(buf, "%ld", num));
 }
 
-unsigned char String::concat(unsigned long num) {
+bool String::concat(unsigned long num) {
     char buf[1 + 3 * sizeof(unsigned long)];
     ultoa(num, buf, 10);
     return concat(buf, strlen(buf));
 }
 
-unsigned char String::concat(long long num) {
+bool String::concat(long long num) {
     char buf[2 + 3 * sizeof(long long)];
     return concat(buf, sprintf(buf, "%lld", num));
 }
 
-unsigned char String::concat(unsigned long long num) {
+bool String::concat(unsigned long long num) {
     char buf[1 + 3 * sizeof(unsigned long long)];
     return concat(buf, sprintf(buf, "%llu", num));
 }
 
-unsigned char String::concat(float num) {
+bool String::concat(float num) {
     char buf[20];
     char *string = dtostrf(num, 4, 2, buf);
     return concat(string, strlen(string));
 }
 
-unsigned char String::concat(double num) {
+bool String::concat(double num) {
     char buf[20];
     char *string = dtostrf(num, 4, 2, buf);
     return concat(string, strlen(string));
 }
 
-unsigned char String::concat(const __FlashStringHelper *str) {
+bool String::concat(const __FlashStringHelper *str) {
     if (!str)
-        return 0;
+        return false;
     int length = strlen_P((PGM_P)str);
     if (length == 0)
-        return 1;
+        return true;
     unsigned int newlen = len() + length;
     if (!reserve(newlen))
-        return 0;
+        return false;
     memcpy_P(wbuffer() + len(), (PGM_P)str, length + 1);
     setLen(newlen);
-    return 1;
+    return true;
 }
 
 /*********************************************/
@@ -488,11 +488,11 @@ int String::compareTo(const String &s) const {
     return strcmp(buffer(), s.buffer());
 }
 
-unsigned char String::equals(const String &s2) const {
+bool String::equals(const String &s2) const {
     return (len() == s2.len() && compareTo(s2) == 0);
 }
 
-unsigned char String::equals(const char *cstr) const {
+bool String::equals(const char *cstr) const {
     if (len() == 0)
         return (cstr == NULL || *cstr == 0);
     if (cstr == NULL)
@@ -500,36 +500,36 @@ unsigned char String::equals(const char *cstr) const {
     return strcmp(buffer(), cstr) == 0;
 }
 
-unsigned char String::operator<(const String &rhs) const {
+bool String::operator<(const String &rhs) const {
     return compareTo(rhs) < 0;
 }
 
-unsigned char String::operator>(const String &rhs) const {
+bool String::operator>(const String &rhs) const {
     return compareTo(rhs) > 0;
 }
 
-unsigned char String::operator<=(const String &rhs) const {
+bool String::operator<=(const String &rhs) const {
     return compareTo(rhs) <= 0;
 }
 
-unsigned char String::operator>=(const String &rhs) const {
+bool String::operator>=(const String &rhs) const {
     return compareTo(rhs) >= 0;
 }
 
-unsigned char String::equalsIgnoreCase(const String &s2) const {
+bool String::equalsIgnoreCase(const String &s2) const {
     if (this == &s2)
-        return 1;
+        return true;
     if (len() != s2.len())
-        return 0;
+        return false;
     if (len() == 0)
-        return 1;
+        return true;
     const char *p1 = buffer();
     const char *p2 = s2.buffer();
     while (*p1) {
         if (tolower(*p1++) != tolower(*p2++))
-            return 0;
+            return false;
     }
-    return 1;
+    return true;
 }
 
 unsigned char String::equalsConstantTime(const String &s2) const {
@@ -559,21 +559,21 @@ unsigned char String::equalsConstantTime(const String &s2) const {
     return (equalcond & diffcond); //bitwise AND
 }
 
-unsigned char String::startsWith(const String &s2) const {
+bool String::startsWith(const String &s2) const {
     if (len() < s2.len())
-        return 0;
+        return false;
     return startsWith(s2, 0);
 }
 
-unsigned char String::startsWith(const String &s2, unsigned int offset) const {
+bool String::startsWith(const String &s2, unsigned int offset) const {
     if (offset > (unsigned)(len() - s2.len()) || !buffer() || !s2.buffer())
-        return 0;
+        return false;
     return strncmp(&buffer()[offset], s2.buffer(), s2.len()) == 0;
 }
 
-unsigned char String::endsWith(const String &s2) const {
+bool String::endsWith(const String &s2) const {
     if (len() < s2.len() || !buffer() || !s2.buffer())
-        return 0;
+        return false;
     return strcmp(&buffer()[len() - s2.len()], s2.buffer()) == 0;
 }
 
@@ -597,7 +597,7 @@ char &String::operator[](unsigned int index) {
 
 char String::operator[](unsigned int index) const {
     if (index >= len() || !buffer())
-        return 0;
+        return '\0';
     return buffer()[index];
 }
 
diff --git a/cores/esp8266/WString.h b/cores/esp8266/WString.h
index 458431e27b..37bb3becb6 100644
--- a/cores/esp8266/WString.h
+++ b/cores/esp8266/WString.h
@@ -45,13 +45,6 @@ class StringSumHelper;
 
 // The string class
 class String {
-        // use a function pointer to allow for "if (s)" without the
-        // complications of an operator bool(). for more information, see:
-        // http://www.artima.com/cppsource/safebool.html
-        typedef void (String::*StringIfHelperType)() const;
-        void StringIfHelper() const {
-        }
-
     public:
         // constructors
         // creates a copy of the initial value.
@@ -90,7 +83,7 @@ class String {
         // return true on success, false on failure (in which case, the string
         // is left unchanged).  reserve(0), if successful, will validate an
         // invalid string (i.e., "if (s)" will be true afterwards)
-        unsigned char reserve(unsigned int size);
+        bool reserve(unsigned int size);
         unsigned int length(void) const {
             return buffer() ? len() : 0;
         }
@@ -119,20 +112,20 @@ class String {
         // 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.
-        unsigned char concat(const String &str);
-        unsigned char concat(const char *cstr);
-        unsigned char concat(char c);
-        unsigned char concat(unsigned char c);
-        unsigned char concat(int num);
-        unsigned char concat(unsigned int num);
-        unsigned char concat(long num);
-        unsigned char concat(unsigned long num);
-        unsigned char concat(long long num);
-        unsigned char concat(unsigned long long num);
-        unsigned char concat(float num);
-        unsigned char concat(double num);
-        unsigned char concat(const __FlashStringHelper *str);
-        unsigned char concat(const char *cstr, unsigned int length);
+        bool concat(const String &str);
+        bool concat(const char *cstr);
+        bool concat(char c);
+        bool concat(unsigned char c);
+        bool concat(int num);
+        bool concat(unsigned int num);
+        bool concat(long num);
+        bool concat(unsigned long num);
+        bool concat(long long num);
+        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)
@@ -142,44 +135,44 @@ class String {
             return *this;
         }
 
-        // comparison (only works w/ Strings and "strings")
-        operator StringIfHelperType() const {
-            return buffer() ? &String::StringIfHelper : 0;
+        explicit operator bool() const {
+            return buffer() != nullptr;
         }
+
         int compareTo(const String &s) const;
-        unsigned char equals(const String &s) const;
-        unsigned char equals(const char *cstr) const;
-        unsigned char operator ==(const String &rhs) const {
+        bool equals(const String &s) const;
+        bool equals(const char *cstr) const;
+        bool operator ==(const String &rhs) const {
             return equals(rhs);
         }
-        unsigned char operator ==(const char *cstr) const {
+        bool operator ==(const char *cstr) const {
             return equals(cstr);
         }
-        unsigned char operator !=(const String &rhs) const {
+        bool operator !=(const String &rhs) const {
             return !equals(rhs);
         }
-        unsigned char operator !=(const char *cstr) const {
+        bool operator !=(const char *cstr) const {
             return !equals(cstr);
         }
-        unsigned char operator <(const String &rhs) const;
-        unsigned char operator >(const String &rhs) const;
-        unsigned char operator <=(const String &rhs) const;
-        unsigned char operator >=(const String &rhs) const;
-        unsigned char equalsIgnoreCase(const String &s) const;
+        bool operator <(const String &rhs) const;
+        bool operator >(const String &rhs) const;
+        bool operator <=(const String &rhs) const;
+        bool operator >=(const String &rhs) const;
+        bool equalsIgnoreCase(const String &s) const;
         unsigned char equalsConstantTime(const String &s) const;
-        unsigned char startsWith(const String &prefix) const;
-        unsigned char startsWith(const char *prefix) const {
+        bool startsWith(const String &prefix) const;
+        bool startsWith(const char *prefix) const {
             return this->startsWith(String(prefix));
         }
-        unsigned char startsWith(const __FlashStringHelper *prefix) const {
+        bool startsWith(const __FlashStringHelper *prefix) const {
             return this->startsWith(String(prefix));
         }
-        unsigned char startsWith(const String &prefix, unsigned int offset) const;
-        unsigned char endsWith(const String &suffix) const;
-        unsigned char endsWith(const char *suffix) const {
+        bool startsWith(const String &prefix, unsigned int offset) const;
+        bool endsWith(const String &suffix) const;
+        bool endsWith(const char *suffix) const {
             return this->endsWith(String(suffix));
         }
-        unsigned char endsWith(const __FlashStringHelper *suffix) const {
+        bool endsWith(const __FlashStringHelper *suffix) const {
             return this->endsWith(String(suffix));
         }
 
@@ -310,7 +303,7 @@ class String {
             // `always_inline` attribute is necessary in order to keep inlining.
         }
         void invalidate(void);
-        unsigned char changeBuffer(unsigned int maxStrLen);
+        bool changeBuffer(unsigned int maxStrLen);
 
         // copy or insert at a specific position
         String &copy(const char *cstr, unsigned int length);