[Linux] C++ version of autoptr for glib objects (#28304)

* Remove CHIPOBLE ifdefs in files included by GN

* Pass arguments as const if possible

* Fix typo in enum name

* Get rid of excessive casting

* Fix memory leak on new CHIP device scanned

* Free memory returned by g_variant_lookup_value

* Move glib deleter helpers to lib/support

* Define GAutoPtr for glib objects memory management

* Build break fix

* Fix build break without WPA

* Add missing include

* Move GLibTypeDeleter.h to src/platform

src/controller/python/chip/ble/LinuxImpl.cpp

@@ -78,11 +78,11 @@ class ScannerDelegateImpl : public ChipDeviceScannerDelegate
 
     void SetScanner(std::unique_ptr<ChipDeviceScanner> scanner) { mScanner = std::move(scanner); }
 
-    void OnDeviceScanned(BluezDevice1 * device, const chip::Ble::ChipBLEDeviceIdentificationInfo & info) override
+    void OnDeviceScanned(BluezDevice1 & device, const chip::Ble::ChipBLEDeviceIdentificationInfo & info) override
     {
         if (mScanCallback)
         {
-            mScanCallback(mContext, bluez_device1_get_address(device), info.GetDeviceDiscriminator(), info.GetProductId(),
+            mScanCallback(mContext, bluez_device1_get_address(&device), info.GetDeviceDiscriminator(), info.GetProductId(),
                           info.GetVendorId());
         }
     }

src/platform/Linux/GlibTypeDeleter.h → src/platform/GLibTypeDeleter.h

@@ -17,7 +17,12 @@
 
 #pragma once
 
+#include <memory>
+
 #include <gio/gio.h>
+#include <glib.h>
+
+namespace chip {
 
 template <typename T, typename Deleter>
 class UniquePointerReceiver
@@ -69,3 +74,49 @@ struct GBytesDeleter
 {
     void operator()(GBytes * object) { g_bytes_unref(object); }
 };
+
+template <typename T>
+struct GAutoPtrDeleter
+{
+};
+
+template <>
+struct GAutoPtrDeleter<char>
+{
+    using deleter = GFree;
+};
+
+template <>
+struct GAutoPtrDeleter<GBytes>
+{
+    using deleter = GBytesDeleter;
+};
+
+template <>
+struct GAutoPtrDeleter<GDBusConnection>
+{
+    using deleter = GObjectDeleter;
+};
+
+template <>
+struct GAutoPtrDeleter<GError>
+{
+    using deleter = GErrorDeleter;
+};
+
+template <>
+struct GAutoPtrDeleter<GVariant>
+{
+    using deleter = GVariantDeleter;
+};
+
+template <>
+struct GAutoPtrDeleter<GVariantIter>
+{
+    using deleter = GVariantIterDeleter;
+};
+
+template <typename T>
+using GAutoPtr = std::unique_ptr<T, typename GAutoPtrDeleter<T>::deleter>;
+
+} // namespace chip

src/platform/Linux/BLEManagerImpl.cpp

@@ -21,20 +21,26 @@
  *          Provides an implementation of the BLEManager singleton object
  *          for Linux platforms.
  */
-#include <platform/internal/CHIPDeviceLayerInternal.h>
 
-#include <ble/CHIPBleServiceData.h>
-#include <lib/support/CodeUtils.h>
-#include <lib/support/SafeInt.h>
-#include <new>
-#include <platform/CommissionableDataProvider.h>
-#include <platform/internal/BLEManager.h>
+/**
+ * Note: BLEManager requires ConnectivityManager to be defined beforehand,
+ *       otherwise we will face circular dependency between them. */
+#include <platform/ConnectivityManager.h>
+
+/**
+ * Note: Use public include for BLEManager which includes our local
+ *       platform/<PLATFORM>/BLEManagerImpl.h after defining interface class. */
+#include "platform/internal/BLEManager.h"
 
 #include <cassert>
 #include <type_traits>
 #include <utility>
 
-#if CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE
+#include <ble/CHIPBleServiceData.h>
+#include <lib/support/CodeUtils.h>
+#include <lib/support/SafeInt.h>
+#include <platform/CHIPDeviceLayer.h>
+#include <platform/CommissionableDataProvider.h>
 
 #include "bluez/Helper.h"
 
@@ -773,9 +779,9 @@ void BLEManagerImpl::NotifyBLEPeripheralAdvStopComplete(bool aIsSuccess, void *
     PlatformMgr().PostEventOrDie(&event);
 }
 
-void BLEManagerImpl::OnDeviceScanned(BluezDevice1 * device, const chip::Ble::ChipBLEDeviceIdentificationInfo & info)
+void BLEManagerImpl::OnDeviceScanned(BluezDevice1 & device, const chip::Ble::ChipBLEDeviceIdentificationInfo & info)
 {
-    ChipLogProgress(Ble, "New device scanned: %s", bluez_device1_get_address(device));
+    ChipLogProgress(Ble, "New device scanned: %s", bluez_device1_get_address(&device));
 
     if (mBLEScanConfig.mBleScanState == BleScanState::kScanForDiscriminator)
     {
@@ -787,7 +793,7 @@ void BLEManagerImpl::OnDeviceScanned(BluezDevice1 * device, const chip::Ble::Chi
     }
     else if (mBLEScanConfig.mBleScanState == BleScanState::kScanForAddress)
     {
-        if (strcmp(bluez_device1_get_address(device), mBLEScanConfig.mAddress.c_str()) != 0)
+        if (strcmp(bluez_device1_get_address(&device), mBLEScanConfig.mAddress.c_str()) != 0)
         {
             return;
         }
@@ -837,5 +843,3 @@ void BLEManagerImpl::OnScanError(CHIP_ERROR err)
 } // namespace Internal
 } // namespace DeviceLayer
 } // namespace chip
-
-#endif // CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE

src/platform/Linux/BLEManagerImpl.h

@@ -26,8 +26,6 @@
 #include <ble/BleLayer.h>
 #include <platform/internal/BLEManager.h>
 
-#if CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE
-
 #include "bluez/ChipDeviceScanner.h"
 #include "bluez/Types.h"
 
@@ -154,7 +152,7 @@ class BLEManagerImpl final : public BLEManager,
     CHIP_ERROR CancelConnection() override;
 
     // ===== Members that implement virtual methods on ChipDeviceScannerDelegate
-    void OnDeviceScanned(BluezDevice1 * device, const chip::Ble::ChipBLEDeviceIdentificationInfo & info) override;
+    void OnDeviceScanned(BluezDevice1 & device, const chip::Ble::ChipBLEDeviceIdentificationInfo & info) override;
     void OnScanComplete() override;
 
     // ===== Members for internal use by the following friends.
@@ -181,9 +179,9 @@ class BLEManagerImpl final : public BLEManager,
 
     enum
     {
-        kMaxConnections             = 1,  // TODO: right max connection
-        kMaxDeviceNameLength        = 20, // TODO: right-size this
-        kMaxAdvertismentDataSetSize = 31  // TODO: verify this
+        kMaxConnections              = 1,  // TODO: right max connection
+        kMaxDeviceNameLength         = 20, // TODO: right-size this
+        kMaxAdvertisementDataSetSize = 31  // TODO: verify this
     };
 
     CHIP_ERROR StartBLEAdvertising();
@@ -246,5 +244,3 @@ inline bool BLEManagerImpl::_IsAdvertising()
 } // namespace Internal
 } // namespace DeviceLayer
 } // namespace chip
-
-#endif // CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE

src/platform/Linux/BUILD.gn

@@ -41,10 +41,8 @@ static_library("Linux") {
   sources = [
     "../DeviceSafeQueue.cpp",
     "../DeviceSafeQueue.h",
+    "../GLibTypeDeleter.h",
     "../SingletonConfigurationManager.cpp",
-    "BLEManagerImpl.cpp",
-    "BLEManagerImpl.h",
-    "BlePlatformConfig.h",
     "CHIPDevicePlatformConfig.h",
     "CHIPDevicePlatformEvent.h",
     "CHIPLinuxStorage.cpp",
@@ -85,6 +83,9 @@ static_library("Linux") {
 
   if (chip_enable_ble) {
     sources += [
+      "BLEManagerImpl.cpp",
+      "BLEManagerImpl.h",
+      "BlePlatformConfig.h",
       "bluez/AdapterIterator.cpp",
       "bluez/AdapterIterator.h",
       "bluez/ChipDeviceScanner.cpp",
@@ -125,7 +126,6 @@ static_library("Linux") {
 
   if (chip_enable_openthread) {
     sources += [
-      "GlibTypeDeleter.h",
       "ThreadStackManagerImpl.cpp",
       "ThreadStackManagerImpl.h",
     ]
@@ -138,10 +138,7 @@ static_library("Linux") {
   }
 
   if (chip_enable_wifi) {
-    sources += [
-      "GlibTypeDeleter.h",
-      "NetworkCommissioningWiFiDriver.cpp",
-    ]
+    sources += [ "NetworkCommissioningWiFiDriver.cpp" ]
 
     public_deps += [ "dbus/wpa" ]
   }

src/platform/Linux/ConnectivityManagerImpl.cpp

@@ -58,7 +58,7 @@
 #endif
 
 #if CHIP_DEVICE_CONFIG_ENABLE_WPA
-#include <platform/Linux/GlibTypeDeleter.h>
+#include <platform/GLibTypeDeleter.h>
 #include <platform/internal/GenericConnectivityManagerImpl_WiFi.ipp>
 #endif
 
@@ -76,6 +76,23 @@ using namespace ::chip::app::Clusters::WiFiNetworkDiagnostics;
 using namespace ::chip::DeviceLayer::NetworkCommissioning;
 
 namespace chip {
+
+#if CHIP_DEVICE_CONFIG_ENABLE_WPA
+
+template <>
+struct GAutoPtrDeleter<WpaFiW1Wpa_supplicant1BSS>
+{
+    using deleter = GObjectDeleter;
+};
+
+template <>
+struct GAutoPtrDeleter<WpaFiW1Wpa_supplicant1Network>
+{
+    using deleter = GObjectDeleter;
+};
+
+#endif // CHIP_DEVICE_CONFIG_ENABLE_WPA
+
 namespace DeviceLayer {
 
 ConnectivityManagerImpl ConnectivityManagerImpl::sInstance;
@@ -1070,8 +1087,7 @@ ConnectivityManagerImpl::ConnectWiFiNetworkAsync(ByteSpan ssid, ByteSpan credent
 void ConnectivityManagerImpl::_ConnectWiFiNetworkAsyncCallback(GObject * source_object, GAsyncResult * res, gpointer user_data)
 {
     ConnectivityManagerImpl * this_ = reinterpret_cast<ConnectivityManagerImpl *>(user_data);
-    std::unique_ptr<GVariant, GVariantDeleter> attachRes;
-    std::unique_ptr<GError, GErrorDeleter> err;
+    GAutoPtr<GError> err;
 
     std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
 
@@ -1159,7 +1175,7 @@ void ConnectivityManagerImpl::PostNetworkConnect()
 CHIP_ERROR ConnectivityManagerImpl::CommitConfig()
 {
     gboolean result;
-    std::unique_ptr<GError, GErrorDeleter> err;
+    GAutoPtr<GError> err;
 
     std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
 
@@ -1290,7 +1306,7 @@ CHIP_ERROR ConnectivityManagerImpl::GetWiFiVersion(WiFiVersionEnum & wiFiVersion
 int32_t ConnectivityManagerImpl::GetDisconnectReason()
 {
     std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
-    std::unique_ptr<GError, GErrorDeleter> err;
+    GAutoPtr<GError> err;
 
     gint errorValue = wpa_fi_w1_wpa_supplicant1_interface_get_disconnect_reason(mWpaSupplicant.iface);
     // wpa_supplicant DBus API: DisconnectReason: The most recent IEEE 802.11 reason code for disconnect. Negative value
@@ -1306,7 +1322,7 @@ CHIP_ERROR ConnectivityManagerImpl::GetConfiguredNetwork(NetworkCommissioning::N
     // with the proxy object.
 
     std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
-    std::unique_ptr<GError, GErrorDeleter> err;
+    GAutoPtr<GError> err;
 
     if (mWpaSupplicant.iface == nullptr)
     {
@@ -1322,20 +1338,19 @@ CHIP_ERROR ConnectivityManagerImpl::GetConfiguredNetwork(NetworkCommissioning::N
         return CHIP_ERROR_KEY_NOT_FOUND;
     }
 
-    std::unique_ptr<WpaFiW1Wpa_supplicant1Network, GObjectDeleter> networkInfo(
-        wpa_fi_w1_wpa_supplicant1_network_proxy_new_for_bus_sync(G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_NONE,
-                                                                 kWpaSupplicantServiceName, networkPath, nullptr,
-                                                                 &MakeUniquePointerReceiver(err).Get()));
+    GAutoPtr<WpaFiW1Wpa_supplicant1Network> networkInfo(wpa_fi_w1_wpa_supplicant1_network_proxy_new_for_bus_sync(
+        G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_NONE, kWpaSupplicantServiceName, networkPath, nullptr,
+        &MakeUniquePointerReceiver(err).Get()));
     if (networkInfo == nullptr)
     {
         return CHIP_ERROR_INTERNAL;
     }
 
     network.connected     = wpa_fi_w1_wpa_supplicant1_network_get_enabled(networkInfo.get());
     GVariant * properties = wpa_fi_w1_wpa_supplicant1_network_get_properties(networkInfo.get());
-    GVariant * ssid       = g_variant_lookup_value(properties, "ssid", nullptr);
+    GAutoPtr<GVariant> ssid(g_variant_lookup_value(properties, "ssid", nullptr));
     gsize length;
-    const gchar * ssidStr = g_variant_get_string(ssid, &length);
+    const gchar * ssidStr = g_variant_get_string(ssid.get(), &length);
     // TODO: wpa_supplicant will return ssid with quotes! We should have a better way to get the actual ssid in bytes.
     gsize length_actual = length - 2;
     VerifyOrReturnError(length_actual <= sizeof(network.networkID), CHIP_ERROR_INTERNAL);
@@ -1350,7 +1365,7 @@ CHIP_ERROR ConnectivityManagerImpl::StopAutoScan()
     std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
     VerifyOrReturnError(mWpaSupplicant.iface != nullptr, CHIP_ERROR_INCORRECT_STATE);
 
-    std::unique_ptr<GError, GErrorDeleter> err;
+    GAutoPtr<GError> err;
     gboolean result;
 
     ChipLogDetail(DeviceLayer, "wpa_supplicant: disabling auto scan");
@@ -1407,6 +1422,7 @@ CHIP_ERROR ConnectivityManagerImpl::StartWiFiScan(ByteSpan ssid, WiFiDriver::Sca
 }
 
 namespace {
+
 // wpa_supplicant's scan results don't contains the channel infomation, so we need this lookup table for resolving the band and
 // channel infomation.
 std::pair<WiFiBand, uint16_t> GetBandAndChannelFromFrequency(uint32_t freq)
@@ -1505,6 +1521,7 @@ std::pair<WiFiBand, uint16_t> GetBandAndChannelFromFrequency(uint32_t freq)
     }
     return ret;
 }
+
 } // namespace
 
 bool ConnectivityManagerImpl::_GetBssInfo(const gchar * bssPath, NetworkCommissioning::WiFiScanResponse & result)
@@ -1514,8 +1531,8 @@ bool ConnectivityManagerImpl::_GetBssInfo(const gchar * bssPath, NetworkCommissi
     // completed before this function returns. Also, no external callbacks are registered
     // with the proxy object.
 
-    std::unique_ptr<GError, GErrorDeleter> err;
-    std::unique_ptr<WpaFiW1Wpa_supplicant1BSS, GObjectDeleter> bss(
+    GAutoPtr<GError> err;
+    GAutoPtr<WpaFiW1Wpa_supplicant1BSS> bss(
         wpa_fi_w1_wpa_supplicant1_bss_proxy_new_for_bus_sync(G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_NONE, kWpaSupplicantServiceName,
                                                              bssPath, nullptr, &MakeUniquePointerReceiver(err).Get()));
 
@@ -1526,8 +1543,8 @@ bool ConnectivityManagerImpl::_GetBssInfo(const gchar * bssPath, NetworkCommissi
 
     WpaFiW1Wpa_supplicant1BSSProxy * bssProxy = WPA_FI_W1_WPA_SUPPLICANT1_BSS_PROXY(bss.get());
 
-    std::unique_ptr<GVariant, GVariantDeleter> ssid(g_dbus_proxy_get_cached_property(G_DBUS_PROXY(bssProxy), "SSID"));
-    std::unique_ptr<GVariant, GVariantDeleter> bssid(g_dbus_proxy_get_cached_property(G_DBUS_PROXY(bssProxy), "BSSID"));
+    GAutoPtr<GVariant> ssid(g_dbus_proxy_get_cached_property(G_DBUS_PROXY(bssProxy), "SSID"));
+    GAutoPtr<GVariant> bssid(g_dbus_proxy_get_cached_property(G_DBUS_PROXY(bssProxy), "BSSID"));
 
     // Network scan is performed in the background, so the BSS
     // may be gone when we try to get the properties.
@@ -1635,8 +1652,8 @@ bool ConnectivityManagerImpl::_GetBssInfo(const gchar * bssPath, NetworkCommissi
         return res;
     };
     auto GetNetworkSecurityType = [IsNetworkWPAPSK, IsNetworkWPA2PSK](WpaFiW1Wpa_supplicant1BSSProxy * proxy) -> uint8_t {
-        std::unique_ptr<GVariant, GVariantDeleter> wpa(g_dbus_proxy_get_cached_property(G_DBUS_PROXY(proxy), "WPA"));
-        std::unique_ptr<GVariant, GVariantDeleter> rsn(g_dbus_proxy_get_cached_property(G_DBUS_PROXY(proxy), "RSN"));
+        GAutoPtr<GVariant> wpa(g_dbus_proxy_get_cached_property(G_DBUS_PROXY(proxy), "WPA"));
+        GAutoPtr<GVariant> rsn(g_dbus_proxy_get_cached_property(G_DBUS_PROXY(proxy), "RSN"));
 
         uint8_t res = IsNetworkWPAPSK(wpa.get()) | IsNetworkWPA2PSK(rsn.get());
         if (res == 0)