Add host test, make SSO 8 bytes, remove magic num

Memory fragmentation is worse than not saving any RAM per String instance,
so extend the SSO size to 8 bytes (7 chars + '\0'), and making the total
String size the same size as it was before.

Remove any hardcoded SSO size and allow configuration via an enum, and
replace the magic max-capacity with an enum tor clarity.

Add a host test that verifies that no memory is allocated until a
full 8 characters are assigned to a string, as well as checking all
intermediate values.

Author: earlephilhower

cores/esp8266/WString.cpp

@@ -153,20 +153,16 @@ unsigned char String::reserve(unsigned int size) {
 }
 
 unsigned char String::changeBuffer(unsigned int maxStrLen) {
-    if (maxStrLen > 0x7fff) {
-        // Sanity check len can fit in 15-bits
-        return 0;
-    }
     // Can we use SSO here to avoid allocation?
-    if (maxStrLen < 4) {
+    if (maxStrLen < sizeof(sso_buff)) {
         if (sso || !bufptr) {
             // Already using SSO, nothing to do but set xap properly
             sso = true;
             capacity = sizeof(sso_buff) - 1; // We subtract off trailing 0 space from available
             return 1;
         } else { // if bufptr
             // Using bufptr, need to shrink into sso_buff
-            char temp[4];
+            char temp[sizeof(sso_buff)];
             memcpy(temp, buffer(), maxStrLen);
             free(bufptr);
             sso = true;
@@ -177,18 +173,22 @@ unsigned char String::changeBuffer(unsigned int maxStrLen) {
     }
     // Fallthrough to normal allocator
     size_t newSize = (maxStrLen + 16) & (~0xf);
-    char temp[4];
+    // Make sure we can fit newsize in the buffer
+    if (newSize > CAPACITY_MAX) {
+        return false;
+    }
+    char temp[sizeof(sso_buff)];
     if (buffer()) {
-        memcpy(temp, buffer(), 4); // Just in case SSO was on
+        memcpy(temp, buffer(), sizeof(sso_buff)); // Just in case SSO was on
     } else {
-        memset(temp, 0, 0);
+        memset(temp, 0, sizeof(sso_buff));
     }
     char *newbuffer = (char *) realloc(sso ? nullptr : bufptr, newSize);
     if(newbuffer) {
         size_t oldSize = capacity + 1; // include NULL.
         if (sso) {
             // Copy the SSO buffer into allocated space
-            memcpy(newbuffer, temp, 4);
+            memcpy(newbuffer, temp, sizeof(sso_buff));
             sso = false;
         }
         if (newSize > oldSize)

cores/esp8266/WString.h

@@ -242,14 +242,17 @@ class String {
         long toInt(void) const;
         float toFloat(void) const;
 
+        enum { SSOSIZE = 8 }; // Characters to allocate space for SSO
+
     protected:
 	union {
-          char *bufptr;     // the actual char array
-          char sso_buff[4]; // Overwrite the ptr with actual string for < 4 chars
+          char *bufptr;           // the actual char array
+          char sso_buff[SSOSIZE]; // Overwrite the ptr with actual string for < SSOSIZE chars
         };
 	unsigned sso      : 1;
+	enum { CAPACITY_MAX = 32767 }; // If size of capacity changed, be sure to update this enum
         unsigned capacity : 15;  // the array length minus one (for the '\0')
-	unsigned unused   : 1;
+        unsigned unused   : 1;
         unsigned len      : 15;       // the String length (not counting the '\0')
         // Buffer accessor functions
         inline const char *buffer() const { return (const char *)(sso ? sso_buff : bufptr); }

tests/host/core/test_string.cpp

@@ -268,3 +268,39 @@ TEST_CASE("String sizes near 8b", "[core][String]")
     REQUIRE(!strcmp(s16.c_str(),"123456789012345_"));
     REQUIRE(!strcmp(s17.c_str(),"1234567890123456_"));
 }
+
+TEST_CASE("String SSO works", "[core][String]")
+{
+  // This test assumes that SSO_SIZE==8, if that changes the test must as well
+  String s;
+  s += "0";
+  REQUIRE(s == "0");
+  const char *savesso = s.c_str();
+  s += 1;
+  REQUIRE(s.c_str() == savesso);
+  REQUIRE(s == "01");
+  s += "2";
+  REQUIRE(s.c_str() == savesso);
+  REQUIRE(s == "012");
+  s += 3;
+  REQUIRE(s.c_str() == savesso);
+  REQUIRE(s == "0123");
+  s += "4";
+  REQUIRE(s.c_str() == savesso);
+  REQUIRE(s == "01234");
+  s += "5";
+  REQUIRE(s.c_str() == savesso);
+  REQUIRE(s == "012345");
+  s += "6";
+  REQUIRE(s.c_str() == savesso);
+  REQUIRE(s == "0123456");
+  s += "7";
+  REQUIRE(s.c_str() != savesso);
+  REQUIRE(s == "01234567");
+  s += "8";
+  REQUIRE(s.c_str() != savesso);
+  REQUIRE(s == "012345678");
+  s += "9";
+  REQUIRE(s.c_str() != savesso);
+  REQUIRE(s == "0123456789");
+}