Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 14 commits into from
Aug 10, 2023
4 changes: 2 additions & 2 deletions src/controller/python/chip/ble/LinuxImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@

#pragma once

#include <memory>

#include <gio/gio.h>
#include <glib.h>

namespace chip {

template <typename T, typename Deleter>
class UniquePointerReceiver
Expand Down Expand Up @@ -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
30 changes: 17 additions & 13 deletions src/platform/Linux/BLEManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
{
Expand All @@ -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;
}
Expand Down Expand Up @@ -837,5 +843,3 @@ void BLEManagerImpl::OnScanError(CHIP_ERROR err)
} // namespace Internal
} // namespace DeviceLayer
} // namespace chip

#endif // CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE
12 changes: 4 additions & 8 deletions src/platform/Linux/BLEManagerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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.
Expand All @@ -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();
Expand Down Expand Up @@ -246,5 +244,3 @@ inline bool BLEManagerImpl::_IsAdvertising()
} // namespace Internal
} // namespace DeviceLayer
} // namespace chip

#endif // CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE
13 changes: 5 additions & 8 deletions src/platform/Linux/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -125,7 +126,6 @@ static_library("Linux") {

if (chip_enable_openthread) {
sources += [
"GlibTypeDeleter.h",
"ThreadStackManagerImpl.cpp",
"ThreadStackManagerImpl.h",
]
Expand All @@ -138,10 +138,7 @@ static_library("Linux") {
}

if (chip_enable_wifi) {
sources += [
"GlibTypeDeleter.h",
"NetworkCommissioningWiFiDriver.cpp",
]
sources += [ "NetworkCommissioningWiFiDriver.cpp" ]

public_deps += [ "dbus/wpa" ]
}
Expand Down
55 changes: 36 additions & 19 deletions src/platform/Linux/ConnectivityManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand All @@ -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)
{
Expand All @@ -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);
Expand All @@ -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");
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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()));

Expand All @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
Loading