FIX: move() of scoped_cleanup

... moving an optional<> (as the default move c'tor would simply do) actually moves the contained object from other&& to this. But IT DOES NOT reset() the other optional. TIL. As a result, the d'tor of other&& may or may not call the cleanup callback after it was destroyed. Different scenarios and compilers produce different results. Co-Authored-By: Fabian Albert <[email protected]>
randombit · Jul 12, 2024 · 4e05de7 · 4e05de7
1 parent 6c92e4b
commit 4e05de7
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 4 deletions.
diff --git a/src/lib/utils/stl_util.h b/src/lib/utils/stl_util.h
@@ -357,8 +357,16 @@ class scoped_cleanup {
 
       scoped_cleanup(const scoped_cleanup&) = delete;
       scoped_cleanup& operator=(const scoped_cleanup&) = delete;
-      scoped_cleanup(scoped_cleanup&&) = default;
-      scoped_cleanup& operator=(scoped_cleanup&&) = default;
+
+      scoped_cleanup(scoped_cleanup&& other) : m_cleanup(std::move(other.m_cleanup)) { other.disengage(); }
+
+      scoped_cleanup& operator=(scoped_cleanup&& other) {
+         if(this != &other) {
+            m_cleanup = std::move(other.m_cleanup);
+            other.disengage();
+         }
+         return *this;
+      }
 
       ~scoped_cleanup() {
          if(m_cleanup.has_value()) {

diff --git a/src/tests/test_utils.cpp b/src/tests/test_utils.cpp
@@ -1232,6 +1232,8 @@ class UUID_Tests : public Test {
 
 BOTAN_REGISTER_TEST("utils", "uuid", UUID_Tests);
 
+#endif
+
 class Formatter_Tests : public Test {
    public:
       std::vector<Test::Result> run() override {
@@ -1323,8 +1325,6 @@ class ScopedCleanup_Tests : public Test {
 
 BOTAN_REGISTER_TEST("utils", "scoped_cleanup", ScopedCleanup_Tests);
 
-#endif
-
 }  // namespace
 
 }  // namespace Botan_Tests