Fix some clang static analyzer warnings

CHANGELOG.md

@@ -9,6 +9,7 @@
 * Access token refresh for websockets was not updating the location metadata ([#6630](https://github.com/realm/realm-core/issues/6630), since v13.9.3)
 * Fix several UBSan failures which did not appear to result in functional bugs ([#6649](https://github.com/realm/realm-core/pull/6649)).
 * Fix an out-of-bounds read in sectioned results when sectioned are removed by modifying all objects in that section to no longer appear in that section ([#6649](https://github.com/realm/realm-core/pull/6649), since v13.12.0)
+* Fix some clang static analyzer warnings. None of them were actual bugs.
 
 ### Breaking changes
 * None.

src/realm/bplustree.cpp

@@ -742,9 +742,9 @@ void BPlusTreeBase::bptree_erase(size_t n, BPlusTreeNode::EraseFunc func)
         ref_type new_root_ref = node->clear_first_bp_node_ref();
         node->destroy_deep();
 
-        auto new_root = create_root_from_ref(new_root_ref);
-
-        replace_root(std::move(new_root));
+        if (auto new_root = create_root_from_ref(new_root_ref)) {
+            replace_root(std::move(new_root));
+        }
         root_size = m_root->get_node_size();
     }
 }
@@ -757,7 +757,7 @@ std::unique_ptr<BPlusTreeNode> BPlusTreeBase::create_root_from_ref(ref_type ref)
 
     if (reuse_root) {
         m_root->init_from_ref(ref);
-        return std::move(m_root);
+        return nullptr;
     }
 
     if (is_leaf) {

src/realm/bplustree.hpp

@@ -172,10 +172,10 @@ class BPlusTreeBase {
 
     void init_from_ref(ref_type ref)
     {
-        auto new_root = create_root_from_ref(ref);
-        new_root->bp_set_parent(m_parent, m_ndx_in_parent);
-
-        m_root = std::move(new_root);
+        if (auto new_root = create_root_from_ref(ref)) {
+            new_root->bp_set_parent(m_parent, m_ndx_in_parent);
+            m_root = std::move(new_root);
+        }
 
         invalidate_leaf_cache();
         m_size = m_root->get_tree_size();
@@ -187,9 +187,10 @@ class BPlusTreeBase {
         if (!ref) {
             return false;
         }
-        auto new_root = create_root_from_ref(ref);
-        new_root->bp_set_parent(m_parent, m_ndx_in_parent);
-        m_root = std::move(new_root);
+        if (auto new_root = create_root_from_ref(ref)) {
+            new_root->bp_set_parent(m_parent, m_ndx_in_parent);
+            m_root = std::move(new_root);
+        }
         invalidate_leaf_cache();
         m_size = m_root->get_tree_size();
         return true;
@@ -374,6 +375,11 @@ class BPlusTree : public BPlusTreeBase {
     REALM_NOINLINE T get_uncached(size_t n) const
     {
         T value;
+#ifdef __clang_analyzer__
+        // Clang's static analyzer can't see that value will always be initialized
+        // inside bptree_access()
+        value = {};
+#endif
 
         auto func = [&value](BPlusTreeNode* node, size_t ndx) {
             LeafNode* leaf = static_cast<LeafNode*>(node);

src/realm/object-store/c_api/realm.cpp

@@ -305,7 +305,8 @@ RLM_API bool realm_remove_table(realm_t* realm, const char* table_name, bool* ta
                                  "Attempt to remove a table that is currently part of the schema");
             }
             (*realm)->read_group().remove_table(table->get_key());
-            *table_deleted = true;
+            if (table_deleted)
+                *table_deleted = true;
         }
         return true;
     });

src/realm/sync/network/network.hpp

@@ -1997,7 +1997,7 @@ class Service::PostOper : public PostOperBase {
 public:
     PostOper(std::size_t size, Impl& service, H&& handler)
         : PostOperBase{size, service}
-        , m_handler{std::move(handler)}
+        , m_handler{std::forward<H>(handler)}
     {
     }
     void recycle_and_execute() override final
@@ -2255,7 +2255,7 @@ class Service::BasicStreamOps {
         char* begin = buffer;
         char* end = buffer + size;
         LendersReadOperPtr op = Service::alloc<ReadOper<H>>(stream.lowest_layer().m_read_oper, stream, is_read_some,
-                                                            begin, end, std::move(handler)); // Throws
+                                                            begin, end, std::forward<H>(handler)); // Throws
         stream.lowest_layer().m_desc.initiate_oper(std::move(op));                           // Throws
     }
 
@@ -2264,8 +2264,9 @@ class Service::BasicStreamOps {
     {
         const char* begin = data;
         const char* end = data + size;
-        LendersWriteOperPtr op = Service::alloc<WriteOper<H>>(
-            stream.lowest_layer().m_write_oper, stream, is_write_some, begin, end, std::move(handler)); // Throws
+        LendersWriteOperPtr op =
+            Service::alloc<WriteOper<H>>(stream.lowest_layer().m_write_oper, stream, is_write_some, begin, end,
+                                         std::forward<H>(handler));                                     // Throws
         stream.lowest_layer().m_desc.initiate_oper(std::move(op));                                      // Throws
     }
 
@@ -2277,7 +2278,7 @@ class Service::BasicStreamOps {
         char* end = buffer + size;
         LendersBufferedReadOperPtr op =
             Service::alloc<BufferedReadOper<H>>(stream.lowest_layer().m_read_oper, stream, begin, end, delim, rab,
-                                                std::move(handler)); // Throws
+                                                std::forward<H>(handler)); // Throws
         stream.lowest_layer().m_desc.initiate_oper(std::move(op));   // Throws
     }
 };
@@ -2553,7 +2554,7 @@ class Service::BasicStreamOps<S>::ReadOper : public ReadOperBase {
 public:
     ReadOper(std::size_t size, S& stream, bool is_read_some, char* begin, char* end, H&& handler)
         : ReadOperBase{size, stream, is_read_some, begin, end}
-        , m_handler{std::move(handler)}
+        , m_handler{std::forward<H>(handler)}
     {
     }
     void recycle_and_execute() override final
@@ -2583,7 +2584,7 @@ class Service::BasicStreamOps<S>::WriteOper : public WriteOperBase {
 public:
     WriteOper(std::size_t size, S& stream, bool is_write_some, const char* begin, const char* end, H&& handler)
         : WriteOperBase{size, stream, is_write_some, begin, end}
-        , m_handler{std::move(handler)}
+        , m_handler{std::forward<H>(handler)}
     {
     }
     void recycle_and_execute() override final
@@ -2614,7 +2615,7 @@ class Service::BasicStreamOps<S>::BufferedReadOper : public BufferedReadOperBase
     BufferedReadOper(std::size_t size, S& stream, char* begin, char* end, int delim, ReadAheadBuffer& rab,
                      H&& handler)
         : BufferedReadOperBase{size, stream, begin, end, delim, rab}
-        , m_handler{std::move(handler)}
+        , m_handler{std::forward<H>(handler)}
     {
     }
     void recycle_and_execute() override final
@@ -2702,7 +2703,7 @@ template <class H>
 inline Service::PostOperBase* Service::post_oper_constr(void* addr, std::size_t size, Impl& service, void* cookie)
 {
     H& handler = *static_cast<H*>(cookie);
-    return new (addr) PostOper<H>(size, service, std::move(handler)); // Throws
+    return new (addr) PostOper<H>(size, service, std::forward<H>(handler)); // Throws
 }
 
 inline bool Service::AsyncOper::in_use() const noexcept
@@ -2761,7 +2762,7 @@ inline void Service::AsyncOper::do_recycle_and_execute(bool orphaned, H& handler
         // happens. For that reason, copying and moving of arguments must not
         // happen until we are in a scope (this scope) that catches and deals
         // correctly with such exceptions.
-        do_recycle_and_execute_helper(orphaned, was_recycled, std::move(handler),
+        do_recycle_and_execute_helper(orphaned, was_recycled, std::forward<H>(handler),
                                       std::forward<Args>(args)...); // Throws
     }
     catch (...) {
@@ -2801,7 +2802,7 @@ class Resolver::ResolveOper : public Service::ResolveOperBase {
 public:
     ResolveOper(std::size_t size, Resolver& r, Query q, H&& handler)
         : ResolveOperBase{size, r, std::move(q)}
-        , m_handler{std::move(handler)}
+        , m_handler{std::forward<H>(handler)}
     {
     }
     void recycle_and_execute() override final
@@ -2843,7 +2844,7 @@ template <class H>
 void Resolver::async_resolve(Query query, H&& handler)
 {
     Service::LendersResolveOperPtr op = Service::alloc<ResolveOper<H>>(m_resolve_oper, *this, std::move(query),
-                                                                       std::move(handler)); // Throws
+                                                                       std::forward<H>(handler)); // Throws
     initiate_oper(std::move(op));                                                           // Throws
 }
 
@@ -3082,7 +3083,7 @@ class Socket::ConnectOper : public ConnectOperBase {
 public:
     ConnectOper(std::size_t size, Socket& sock, H&& handler)
         : ConnectOperBase{size, sock}
-        , m_handler{std::move(handler)}
+        , m_handler{std::forward<H>(handler)}
     {
     }
     void recycle_and_execute() override final
@@ -3210,50 +3211,51 @@ inline std::size_t Socket::write_some(const char* data, std::size_t size, std::e
 template <class H>
 inline void Socket::async_connect(const Endpoint& ep, H&& handler)
 {
-    LendersConnectOperPtr op = Service::alloc<ConnectOper<H>>(m_write_oper, *this, std::move(handler)); // Throws
+    LendersConnectOperPtr op =
+        Service::alloc<ConnectOper<H>>(m_write_oper, *this, std::forward<H>(handler));                  // Throws
     m_desc.initiate_oper(std::move(op), ep);                                                            // Throws
 }
 
 template <class H>
 inline void Socket::async_read(char* buffer, std::size_t size, H&& handler)
 {
     bool is_read_some = false;
-    StreamOps::async_read(*this, buffer, size, is_read_some, std::move(handler)); // Throws
+    StreamOps::async_read(*this, buffer, size, is_read_some, std::forward<H>(handler)); // Throws
 }
 
 template <class H>
 inline void Socket::async_read(char* buffer, std::size_t size, ReadAheadBuffer& rab, H&& handler)
 {
     int delim = std::char_traits<char>::eof();
-    StreamOps::async_buffered_read(*this, buffer, size, delim, rab, std::move(handler)); // Throws
+    StreamOps::async_buffered_read(*this, buffer, size, delim, rab, std::forward<H>(handler)); // Throws
 }
 
 template <class H>
 inline void Socket::async_read_until(char* buffer, std::size_t size, char delim, ReadAheadBuffer& rab, H&& handler)
 {
     int delim_2 = std::char_traits<char>::to_int_type(delim);
-    StreamOps::async_buffered_read(*this, buffer, size, delim_2, rab, std::move(handler)); // Throws
+    StreamOps::async_buffered_read(*this, buffer, size, delim_2, rab, std::forward<H>(handler)); // Throws
 }
 
 template <class H>
 inline void Socket::async_write(const char* data, std::size_t size, H&& handler)
 {
     bool is_write_some = false;
-    StreamOps::async_write(*this, data, size, is_write_some, std::move(handler)); // Throws
+    StreamOps::async_write(*this, data, size, is_write_some, std::forward<H>(handler)); // Throws
 }
 
 template <class H>
 inline void Socket::async_read_some(char* buffer, std::size_t size, H&& handler)
 {
     bool is_read_some = true;
-    StreamOps::async_read(*this, buffer, size, is_read_some, std::move(handler)); // Throws
+    StreamOps::async_read(*this, buffer, size, is_read_some, std::forward<H>(handler)); // Throws
 }
 
 template <class H>
 inline void Socket::async_write_some(const char* data, std::size_t size, H&& handler)
 {
     bool is_write_some = true;
-    StreamOps::async_write(*this, data, size, is_write_some, std::move(handler)); // Throws
+    StreamOps::async_write(*this, data, size, is_write_some, std::forward<H>(handler)); // Throws
 }
 
 inline void Socket::shutdown(shutdown_type what)
@@ -3390,7 +3392,7 @@ class Acceptor::AcceptOper : public AcceptOperBase {
 public:
     AcceptOper(std::size_t size, Acceptor& a, Socket& s, Endpoint* e, H&& handler)
         : AcceptOperBase{size, a, s, e}
-        , m_handler{std::move(handler)}
+        , m_handler{std::forward<H>(handler)}
     {
     }
     void recycle_and_execute() override final
@@ -3452,13 +3454,13 @@ template <class H>
 inline void Acceptor::async_accept(Socket& sock, H&& handler)
 {
     Endpoint* ep = nullptr;
-    async_accept(sock, ep, std::move(handler)); // Throws
+    async_accept(sock, ep, std::forward<H>(handler)); // Throws
 }
 
 template <class H>
 inline void Acceptor::async_accept(Socket& sock, Endpoint& ep, H&& handler)
 {
-    async_accept(sock, &ep, std::move(handler)); // Throws
+    async_accept(sock, &ep, std::forward<H>(handler)); // Throws
 }
 
 inline std::error_code Acceptor::accept(Socket& socket, Endpoint* ep, std::error_code& ec)
@@ -3487,7 +3489,7 @@ inline void Acceptor::async_accept(Socket& sock, Endpoint* ep, H&& handler)
     if (REALM_UNLIKELY(sock.is_open()))
         throw util::runtime_error("Socket is already open");
     LendersAcceptOperPtr op = Service::alloc<AcceptOper<H>>(m_read_oper, *this, sock, ep,
-                                                            std::move(handler)); // Throws
+                                                            std::forward<H>(handler)); // Throws
     m_desc.initiate_oper(std::move(op));                                         // Throws
 }
 
@@ -3498,7 +3500,7 @@ class DeadlineTimer::WaitOper : public Service::WaitOperBase {
 public:
     WaitOper(std::size_t size, DeadlineTimer& timer, clock::time_point expiration_time, H&& handler)
         : Service::WaitOperBase{size, timer, expiration_time}
-        , m_handler{std::move(handler)}
+        , m_handler{std::forward<H>(handler)}
     {
     }
     void recycle_and_execute() override final
@@ -3538,7 +3540,7 @@ inline void DeadlineTimer::async_wait(std::chrono::duration<R, P> delay, H&& han
         throw util::overflow_error("Expiration time overflow");
     clock::time_point expiration_time = now + delay;
     initiate_oper(Service::alloc<WaitOper<H>>(m_wait_oper, *this, expiration_time,
-                                              std::move(handler))); // Throws
+                                              std::forward<H>(handler))); // Throws
 }
 
 // ---------------- ReadAheadBuffer ----------------

src/realm/table.cpp

@@ -560,6 +560,7 @@ void Table::remove_recursive(CascadeState& cascade_state)
     REALM_ASSERT(group);
     cascade_state.m_group = group;
 
+    std::vector<std::pair<TableKey, ObjKey>> to_delete;
     do {
         cascade_state.send_notifications();
 
@@ -572,14 +573,15 @@ void Table::remove_recursive(CascadeState& cascade_state)
         }
         cascade_state.m_to_be_nullified.clear();
 
-        auto to_delete = std::move(cascade_state.m_to_be_deleted);
-        for (auto obj : to_delete) {
-            auto table = group->get_table(obj.first);
+        to_delete.swap(cascade_state.m_to_be_deleted);
+        for (auto [table_key, obj_key] : to_delete) {
+            auto table = group->get_table(table_key);
             // This might add to the list of objects that should be deleted
-            REALM_ASSERT(!obj.second.is_unresolved());
-            table->m_clusters.erase(obj.second, cascade_state);
+            REALM_ASSERT(!obj_key.is_unresolved());
+            table->m_clusters.erase(obj_key, cascade_state);
         }
         nullify_links(cascade_state);
+        to_delete.clear();
     } while (!cascade_state.m_to_be_deleted.empty() || !cascade_state.m_to_be_nullified.empty());
 }
 
@@ -1679,7 +1681,7 @@ void copy_list<Timestamp>(ref_type sub_table_ref, Obj& obj, ColKey col, Allocato
 
 void Table::create_columns()
 {
-    size_t cnt;
+    size_t cnt = 0;
     auto get_column_cnt = [&cnt](const Cluster* cluster) {
         cnt = cluster->nb_columns();
         return IteratorControl::Stop;

src/realm/util/thread.hpp

@@ -557,6 +557,7 @@ inline void Mutex::unlock() noexcept
 #else
     int r = pthread_mutex_unlock(&m_impl);
     REALM_ASSERT(r == 0);
+    static_cast<void>(r);
 #endif
 }
 
@@ -780,6 +781,7 @@ inline void CondVar::notify() noexcept
 #else
     int r = pthread_cond_signal(&m_impl);
     REALM_ASSERT(r == 0);
+    static_cast<void>(r);
 #endif
 }
 
@@ -790,6 +792,7 @@ inline void CondVar::notify_all() noexcept
 #else
     int r = pthread_cond_broadcast(&m_impl);
     REALM_ASSERT(r == 0);
+    static_cast<void>(r);
 #endif
 }
 

src/realm/uuid.cpp

@@ -45,15 +45,15 @@ bool is_hex_digit(unsigned char c)
     return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F');
 }
 
-char parse_xdigit(char ch)
+uint8_t parse_xdigit(char ch)
 {
     if (ch >= '0' && ch <= '9')
         return ch - '0';
     if (ch >= 'a' && ch <= 'f')
         return ch - 'a' + 10;
     if (ch >= 'A' && ch <= 'F')
         return ch - 'A' + 10;
-    return char(-1);
+    REALM_UNREACHABLE();
 }
 } // anonymous namespace
 
@@ -80,7 +80,6 @@ bool UUID::is_valid_string(StringData str) noexcept
 }
 
 UUID::UUID(StringData init)
-    : m_bytes{}
 {
     if (!is_valid_string(init)) {
         throw InvalidUUIDString{

test/object-store/util/test_utils.hpp

@@ -105,7 +105,7 @@ namespace _impl {
 template <typename T>
 ExceptionMatcher<T> make_exception_matcher(ErrorCodes::Error code, T&& matcher)
 {
-    return ExceptionMatcher<T>(code, std::move(matcher));
+    return ExceptionMatcher<T>(code, std::forward<T>(matcher));
 }
 inline ExceptionMatcher<void> make_exception_matcher(ErrorCodes::Error code, const char* msg)
 {