From 22c6c8dba89184a15631cfc19e8b26102cda9c2b Mon Sep 17 00:00:00 2001
From: nerix <nerixdev@outlook.de>
Date: Sat, 22 Feb 2025 13:49:38 +0100
Subject: [PATCH] fix(eventsub/string): minor cleanups (#5973)

---
 CHANGELOG.md                                  |  2 +-
 .../include/twitch-eventsub-ws/string.hpp     | 47 +++++++++----------
 lib/twitch-eventsub-ws/tests/src/string.cpp   | 31 +++++++++++-
 3 files changed, 54 insertions(+), 26 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 50f1976a..0062e10e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -29,7 +29,7 @@
 - Bugfix: Fixed channel point redemptions with messages not showing up if PubSub is disconnected. (#5948)
 - Bugfix: Fixed the input font not immediately updating when zooming in/out. (#5960)
 - Dev: Subscriptions to PubSub channel points redemption topics now use no auth token, making it continue to work during PubSub shutdown. (#5947)
-- Dev: Add initial experimental EventSub support. (#5837, #5895, #5897, #5904, #5910, #5903, #5915, #5916, #5930, #5935, #5932, #5943, #5952, #5953, #5968)
+- Dev: Add initial experimental EventSub support. (#5837, #5895, #5897, #5904, #5910, #5903, #5915, #5916, #5930, #5935, #5932, #5943, #5952, #5953, #5968, #5973)
 - Dev: Remove unneeded platform specifier for toasts. (#5914)
 - Dev: Highlight checks now use non-capturing groups for the boundaries. (#5784)
 - Dev: Removed unused PubSub whisper code. (#5898)
diff --git a/lib/twitch-eventsub-ws/include/twitch-eventsub-ws/string.hpp b/lib/twitch-eventsub-ws/include/twitch-eventsub-ws/string.hpp
index 674047ad..7fe37ee3 100644
--- a/lib/twitch-eventsub-ws/include/twitch-eventsub-ws/string.hpp
+++ b/lib/twitch-eventsub-ws/include/twitch-eventsub-ws/string.hpp
@@ -24,19 +24,22 @@ namespace chatterino::eventsub::lib {
 /// is **not** null-terminated.
 struct String {
     constexpr String() noexcept = default;
-    constexpr String(std::string_view sv)
+    String(std::string_view sv)
         : flags(sv.length() & LENGTH_MASK)
     {
-        if (sv.length() <= SSO_CAPACITY)
+        char *data = this->storage.inPlace;
+
+        if (sv.length() > SSO_CAPACITY)
         {
-            std::memcpy(this->storage.inPlace, sv.data(), sv.length());
-            return;
+            data = new char[sv.length()];
+            this->flags |= ALLOC_BIT;
+            this->storage.data = data;
         }
 
-        this->flags |= ALLOC_BIT;
-        auto *data = new char[sv.length()];
-        std::memcpy(data, sv.data(), sv.length());
-        this->storage.data = data;
+        for (size_t i = 0; i < sv.length(); i++)
+        {
+            data[i] = sv[i];
+        }
     }
 
     ~String()
@@ -117,7 +120,7 @@ struct String {
         return this->storage.qt;
     }
 
-    QAnyStringView view() const noexcept
+    constexpr QAnyStringView view() const noexcept
     {
         if (this->isQt())
         {
@@ -158,15 +161,6 @@ private:
     static_assert((LENGTH_MASK & ALLOC_BIT) == 0);
     static_assert((LENGTH_MASK & QT_BIT) == 0);
 
-    constexpr size_t length() const noexcept
-    {
-        if ((this->flags & QT_BIT) != 0)
-        {
-            return this->storage.qt.length();
-        }
-        return this->flags & LENGTH_MASK;
-    }
-
     mutable union Storage {
         constexpr Storage() noexcept
         {
@@ -178,9 +172,6 @@ private:
         Storage(const Storage &) = delete;
         Storage &operator=(const Storage &) = delete;
 
-        // we can memcpy QStrings as they're relocatable
-        static_assert(QTypeInfo<QString>::isRelocatable != 0);
-
         Storage(Storage &&other) noexcept
         {
             Storage::move(std::addressof(other), this);
@@ -198,10 +189,18 @@ private:
     private:
         static void move(Storage *from, Storage *to)
         {
+            // we can memcpy QStrings as they're relocatable
+            static_assert(QTypeInfo<QString>::isRelocatable != 0);
+
             // copy `from` -> `to`
-            std::memcpy(to, from, sizeof(Storage));
-            // clear `from`
-            std::memset(from, 0, sizeof(Storage));
+            std::memcpy(static_cast<void *>(to), from, sizeof(Storage));
+#ifndef NDEBUG
+            // Mark `from` as unused.
+            // Because the parent `String` sets the flags to 0,
+            // we don't _need_ to overwrite the data. In debug mode we write a
+            // tombstone value here.
+            std::memset(static_cast<void *>(from), 0xff, sizeof(Storage));
+#endif
         }
     } storage;
     static_assert(sizeof(Storage) == sizeof(QString));
diff --git a/lib/twitch-eventsub-ws/tests/src/string.cpp b/lib/twitch-eventsub-ws/tests/src/string.cpp
index caac0535..fa913103 100644
--- a/lib/twitch-eventsub-ws/tests/src/string.cpp
+++ b/lib/twitch-eventsub-ws/tests/src/string.cpp
@@ -479,10 +479,39 @@ TEST(String, MoveAssign)
 TEST(String, QtLifetime)
 {
     QString qt;
+
     {
         String s(REALLY_LONG);
         qt = s.qt();
-        ASSERT_TRUE(!qt.isDetached());  // s holds the string too
+        ASSERT_FALSE(qt.isDetached());  // s holds the string too
+    }
+    ASSERT_TRUE(qt.isDetached());
+
+    // move assignments
+    {
+        String s(REALLY_LONG);
+        qt = s.qt();
+        ASSERT_FALSE(qt.isDetached());
+        s = {};
+        ASSERT_TRUE(qt.isDetached());
+    }
+    ASSERT_TRUE(qt.isDetached());
+
+    {
+        String s(REALLY_LONG);
+        qt = s.qt();
+        ASSERT_FALSE(qt.isDetached());
+        s = {LONGEST_SSO};
+        ASSERT_TRUE(qt.isDetached());
+    }
+    ASSERT_TRUE(qt.isDetached());
+
+    {
+        String s(REALLY_LONG);
+        qt = s.qt();
+        ASSERT_FALSE(qt.isDetached());
+        s = {REALLY_LONG};
+        ASSERT_TRUE(qt.isDetached());
     }
     ASSERT_TRUE(qt.isDetached());
 }