[Linux] Reuse AdapterIterator in all places where listing managed objects (#32372)

* Use dedicated iterator for listing BlueZ objects

* Use GAutoPtr to manage GDBusObjectManager instance

* Wrap static function with lambda

* Fix iterator initialization for AdapterIterator

Author: arkq

src/platform/GLibTypeDeleter.h

@@ -120,6 +120,12 @@ struct GAutoPtrDeleter<GDBusConnection>
     using deleter = GObjectDeleter;
 };
 
+template <>
+struct GAutoPtrDeleter<GDBusObjectManager>
+{
+    using deleter = GObjectDeleter;
+};
+
 template <>
 struct GAutoPtrDeleter<GDBusObjectManagerServer>
 {

src/platform/Linux/bluez/AdapterIterator.cpp

@@ -26,69 +26,38 @@ namespace chip {
 namespace DeviceLayer {
 namespace Internal {
 
-AdapterIterator::~AdapterIterator()
-{
-    if (mManager != nullptr)
-    {
-        g_object_unref(mManager);
-    }
-
-    if (mObjectList != nullptr)
-    {
-        g_list_free_full(mObjectList, g_object_unref);
-    }
-}
-
-CHIP_ERROR AdapterIterator::Initialize(AdapterIterator * self)
+CHIP_ERROR AdapterIterator::Initialize()
 {
     // When creating D-Bus proxy object, the thread default context must be initialized. Otherwise,
     // all D-Bus signals will be delivered to the GLib global default main context.
     VerifyOrDie(g_main_context_get_thread_default() != nullptr);
 
-    CHIP_ERROR err = CHIP_NO_ERROR;
     GAutoPtr<GError> error;
-
-    self->mManager = g_dbus_object_manager_client_new_for_bus_sync(
+    mManager.reset(g_dbus_object_manager_client_new_for_bus_sync(
         G_BUS_TYPE_SYSTEM, G_DBUS_OBJECT_MANAGER_CLIENT_FLAGS_NONE, BLUEZ_INTERFACE, "/",
         bluez_object_manager_client_get_proxy_type, nullptr /* unused user data in the Proxy Type Func */,
-        nullptr /* destroy notify */, nullptr /* cancellable */, &error.GetReceiver());
+        nullptr /* destroy notify */, nullptr /* cancellable */, &error.GetReceiver()));
 
-    VerifyOrExit(self->mManager != nullptr, ChipLogError(DeviceLayer, "Failed to get DBUS object manager for listing adapters.");
-                 err = CHIP_ERROR_INTERNAL);
+    VerifyOrReturnError(mManager, CHIP_ERROR_INTERNAL,
+                        ChipLogError(DeviceLayer, "Failed to get D-Bus object manager for listing adapters: %s", error->message));
 
-    self->mObjectList      = g_dbus_object_manager_get_objects(self->mManager);
-    self->mCurrentListItem = self->mObjectList;
+    mObjectList.Init(mManager.get());
+    mIterator = mObjectList.begin();
 
-exit:
-    if (error != nullptr)
-    {
-        ChipLogError(DeviceLayer, "DBus error: %s", error->message);
-    }
-
-    return err;
+    return CHIP_NO_ERROR;
 }
 
 bool AdapterIterator::Advance()
 {
-    if (mCurrentListItem == nullptr)
+    for (; mIterator != BluezObjectList::end(); ++mIterator)
     {
-        return false;
-    }
-
-    while (mCurrentListItem != nullptr)
-    {
-        BluezAdapter1 * adapter = bluez_object_get_adapter1(BLUEZ_OBJECT(mCurrentListItem->data));
-        if (adapter == nullptr)
+        BluezAdapter1 * adapter = bluez_object_get_adapter1(&(*mIterator));
+        if (adapter != nullptr)
         {
-            mCurrentListItem = mCurrentListItem->next;
-            continue;
+            mCurrentAdapter.reset(adapter);
+            ++mIterator;
+            return true;
         }
-
-        mCurrentAdapter.reset(adapter);
-
-        mCurrentListItem = mCurrentListItem->next;
-
-        return true;
     }
 
     return false;
@@ -112,9 +81,10 @@ uint32_t AdapterIterator::GetIndex() const
 
 bool AdapterIterator::Next()
 {
-    if (mManager == nullptr)
+    if (!mManager)
     {
-        CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync(Initialize, this);
+        CHIP_ERROR err = PlatformMgrImpl().GLibMatterContextInvokeSync(
+            +[](AdapterIterator * self) { return self->Initialize(); }, this);
         VerifyOrReturnError(err == CHIP_NO_ERROR, false, ChipLogError(DeviceLayer, "Failed to initialize adapter iterator"));
     }
 

src/platform/Linux/bluez/AdapterIterator.h

@@ -17,13 +17,15 @@
 
 #pragma once
 
-#include <string>
+#include <cstdint>
 
 #include <gio/gio.h>
 
 #include <lib/core/CHIPError.h>
+#include <platform/GLibTypeDeleter.h>
 #include <platform/Linux/dbus/bluez/DbusBluez.h>
 
+#include "BluezObjectList.h"
 #include "Types.h"
 
 namespace chip {
@@ -46,8 +48,6 @@ namespace Internal {
 class AdapterIterator
 {
 public:
-    ~AdapterIterator();
-
     /// Moves to the next DBUS interface.
     ///
     /// MUST be called before any of the 'current value' methods are
@@ -65,17 +65,17 @@ class AdapterIterator
 
 private:
     /// Sets up the DBUS manager and loads the list
-    static CHIP_ERROR Initialize(AdapterIterator * self);
+    CHIP_ERROR Initialize();
 
     /// Loads the next value in the list.
     ///
     /// Returns true if a value could be loaded, false if no more items to
     /// iterate through.
     bool Advance();
 
-    GDBusObjectManager * mManager = nullptr; // DBus connection
-    GList * mObjectList           = nullptr; // listing of objects on the bus
-    GList * mCurrentListItem      = nullptr; // current item viewed in the list
+    GAutoPtr<GDBusObjectManager> mManager;
+    BluezObjectList mObjectList;
+    BluezObjectIterator mIterator;
     // Data valid only if Next() returns true
     GAutoPtr<BluezAdapter1> mCurrentAdapter;
 };

src/platform/Linux/bluez/BluezConnection.cpp

@@ -34,6 +34,7 @@
 #include <system/SystemPacketBuffer.h>
 
 #include "BluezEndpoint.h"
+#include "BluezObjectList.h"
 #include "Types.h"
 
 namespace chip {
@@ -95,10 +96,6 @@ BluezConnection::ConnectionDataBundle::ConnectionDataBundle(const BluezConnectio
 
 CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint)
 {
-    // populate the service and the characteristics
-    GList * objects = nullptr;
-    GList * l;
-
     if (!aEndpoint.mIsCentral)
     {
         mpService = reinterpret_cast<BluezGattService1 *>(g_object_ref(aEndpoint.mpService));
@@ -107,11 +104,9 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint)
     }
     else
     {
-        objects = g_dbus_object_manager_get_objects(aEndpoint.mpObjMgr);
-
-        for (l = objects; l != nullptr; l = l->next)
+        for (BluezObject & object : BluezObjectList(aEndpoint.mpObjMgr))
         {
-            BluezGattService1 * service = bluez_object_get_gatt_service1(BLUEZ_OBJECT(l->data));
+            BluezGattService1 * service = bluez_object_get_gatt_service1(&object);
             if (service != nullptr)
             {
                 if ((BluezIsServiceOnDevice(service, mpDevice)) == TRUE &&
@@ -126,9 +121,9 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint)
 
         VerifyOrExit(mpService != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL service in %s", __func__));
 
-        for (l = objects; l != nullptr; l = l->next)
+        for (BluezObject & object : BluezObjectList(aEndpoint.mpObjMgr))
         {
-            BluezGattCharacteristic1 * char1 = bluez_object_get_gatt_characteristic1(BLUEZ_OBJECT(l->data));
+            BluezGattCharacteristic1 * char1 = bluez_object_get_gatt_characteristic1(&object);
             if (char1 != nullptr)
             {
                 if ((BluezIsCharOnService(char1, mpService) == TRUE) &&
@@ -164,8 +159,6 @@ CHIP_ERROR BluezConnection::Init(const BluezEndpoint & aEndpoint)
     }
 
 exit:
-    if (objects != nullptr)
-        g_list_free_full(objects, g_object_unref);
     return CHIP_NO_ERROR;
 }
 

src/platform/Linux/bluez/BluezEndpoint.cpp

@@ -72,6 +72,7 @@
 #include <system/SystemPacketBuffer.h>
 
 #include "BluezConnection.h"
+#include "BluezObjectList.h"
 #include "Types.h"
 
 namespace chip {
@@ -420,15 +421,14 @@ BluezGattService1 * BluezEndpoint::CreateGattService(const char * aUUID)
     return service;
 }
 
-void BluezEndpoint::SetupAdapter()
+CHIP_ERROR BluezEndpoint::SetupAdapter()
 {
     char expectedPath[32];
     snprintf(expectedPath, sizeof(expectedPath), BLUEZ_PATH "/hci%u", mAdapterId);
 
-    GList * objects = g_dbus_object_manager_get_objects(mpObjMgr);
-    for (auto l = objects; l != nullptr && mAdapter.get() == nullptr; l = l->next)
+    for (BluezObject & object : BluezObjectList(mpObjMgr))
     {
-        GAutoPtr<BluezAdapter1> adapter(bluez_object_get_adapter1(BLUEZ_OBJECT(l->data)));
+        GAutoPtr<BluezAdapter1> adapter(bluez_object_get_adapter1(&object));
         if (adapter.get() != nullptr)
         {
             if (mpAdapterAddr == nullptr) // no adapter address provided, bind to the hci indicated by nodeid
@@ -450,7 +450,7 @@ void BluezEndpoint::SetupAdapter()
         }
     }
 
-    VerifyOrExit(mAdapter.get() != nullptr, ChipLogError(DeviceLayer, "FAIL: NULL mAdapter in %s", __func__));
+    VerifyOrReturnError(mAdapter, CHIP_ERROR_INTERNAL, ChipLogError(DeviceLayer, "FAIL: NULL mAdapter in %s", __func__));
 
     bluez_adapter1_set_powered(mAdapter.get(), TRUE);
 
@@ -459,8 +459,7 @@ void BluezEndpoint::SetupAdapter()
     // and the flag is necessary to force using LE transport.
     bluez_adapter1_set_discoverable(mAdapter.get(), FALSE);
 
-exit:
-    g_list_free_full(objects, g_object_unref);
+    return CHIP_NO_ERROR;
 }
 
 BluezConnection * BluezEndpoint::GetBluezConnection(const char * aPath)

src/platform/Linux/bluez/BluezEndpoint.h

@@ -85,7 +85,7 @@ class BluezEndpoint
 private:
     CHIP_ERROR StartupEndpointBindings();
 
-    void SetupAdapter();
+    CHIP_ERROR SetupAdapter();
     void SetupGattServer(GDBusConnection * aConn);
     void SetupGattService();
 

src/platform/Linux/bluez/BluezObjectIterator.h

@@ -17,6 +17,8 @@
 
 #pragma once
 
+#include <iterator>
+
 #include <glib.h>
 
 #include <platform/CHIPDeviceConfig.h>

src/platform/Linux/bluez/BluezObjectList.h

@@ -35,27 +35,26 @@ namespace Internal {
 class BluezObjectList
 {
 public:
-    explicit BluezObjectList(GDBusObjectManager * manager) { Initialize(manager); }
+    BluezObjectList() = default;
+    explicit BluezObjectList(GDBusObjectManager * manager) { Init(manager); }
 
-    ~BluezObjectList() { g_list_free_full(mObjectList, g_object_unref); }
-
-    BluezObjectIterator begin() const { return BluezObjectIterator(mObjectList); }
-    BluezObjectIterator end() const { return BluezObjectIterator(); }
-
-protected:
-    BluezObjectList() {}
-
-    void Initialize(GDBusObjectManager * manager)
+    ~BluezObjectList()
     {
-        if (manager == nullptr)
-        {
-            ChipLogError(DeviceLayer, "Manager is NULL in %s", __func__);
-            return;
-        }
+        if (mObjectList != nullptr)
+            g_list_free_full(mObjectList, g_object_unref);
+    }
 
+    CHIP_ERROR Init(GDBusObjectManager * manager)
+    {
+        VerifyOrReturnError(manager != nullptr, CHIP_ERROR_INVALID_ARGUMENT,
+                            ChipLogError(DeviceLayer, "Manager is NULL in %s", __func__));
         mObjectList = g_dbus_object_manager_get_objects(manager);
+        return CHIP_NO_ERROR;
     }
 
+    BluezObjectIterator begin() const { return BluezObjectIterator(mObjectList); }
+    static BluezObjectIterator end() { return BluezObjectIterator(); }
+
 private:
     GList * mObjectList = nullptr;
 };