Skip to content

Commit

Permalink
Review changes
Browse files Browse the repository at this point in the history 
* Replace g_variant_dict_lookup_value to g_variant_lookup_value in src/platform/Linux/bluez/Helper.cpp
* Add template <> struct GAutoPtrDeleter<const char *>
* Convert keyMgmts to GAutoPtr in src/platform/Linux/ConnectivityManagerImpl.cpp
* Revert in ConnectionDataBundle GVariant convert to GAutoPtr
  • Loading branch information
@DamMicSzm
DamMicSzm committed Oct 6, 2023
1 parent ed62e07 commit 84e1d20
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 27 deletions.
6 changes: 6 additions & 0 deletions src/platform/GLibTypeDeleter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ struct GAutoPtrDeleter<char>
using deleter = GFree;
};

template <>
struct GAutoPtrDeleter<const char *>
{
using deleter = GFree;
};

template <>
struct GAutoPtrDeleter<GBytes>
{
Expand Down
24 changes: 12 additions & 12 deletions src/platform/Linux/ConnectivityManagerImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1557,11 +1557,11 @@ bool ConnectivityManagerImpl::_GetBssInfo(const gchar * bssPath, NetworkCommissi
{
return 0;
}
const gchar ** keyMgmts = g_variant_get_strv(keyMgmt.get(), nullptr);
const gchar ** keyMgmtsForFree = keyMgmts;
uint8_t res = 0;
for (const gchar * keyMgmtVal = (keyMgmts != nullptr ? *keyMgmts : nullptr); keyMgmtVal != nullptr;
keyMgmtVal = *(++keyMgmts))
GAutoPtr<const char *> keyMgmts(g_variant_get_strv(keyMgmt.get(), nullptr));
const gchar ** keyMgmtsHendle = keyMgmts.get();
uint8_t res = 0;
for (const gchar * keyMgmtVal = (keyMgmtsHendle != nullptr ? *keyMgmtsHendle : nullptr); keyMgmtVal != nullptr;
keyMgmtVal = *(++keyMgmtsHendle))
{
if (g_strcasecmp(keyMgmtVal, "wpa-psk") == 0 || g_strcasecmp(keyMgmtVal, "wpa-none") == 0)
{
Expand All @@ -1572,7 +1572,7 @@ bool ConnectivityManagerImpl::_GetBssInfo(const gchar * bssPath, NetworkCommissi
res |= (kEAP);
}
}
g_free(keyMgmtsForFree);

return res;
};
auto IsNetworkWPA2PSK = [](GVariant * rsn) -> uint8_t {
Expand All @@ -1585,11 +1585,11 @@ bool ConnectivityManagerImpl::_GetBssInfo(const gchar * bssPath, NetworkCommissi
{
return 0;
}
const gchar ** keyMgmts = g_variant_get_strv(keyMgmt.get(), nullptr);
const gchar ** keyMgmtsForFree = keyMgmts;
uint8_t res = 0;
for (const gchar * keyMgmtVal = (keyMgmts != nullptr ? *keyMgmts : nullptr); keyMgmtVal != nullptr;
keyMgmtVal = *(++keyMgmts))
GAutoPtr<const char *> keyMgmts(g_variant_get_strv(keyMgmt.get(), nullptr));
const gchar ** keyMgmtsHendle = keyMgmts.get();
uint8_t res = 0;
for (const gchar * keyMgmtVal = (keyMgmtsHendle != nullptr ? *keyMgmtsHendle : nullptr); keyMgmtVal != nullptr;
keyMgmtVal = *(++keyMgmtsHendle))
{
if (g_strcasecmp(keyMgmtVal, "wpa-psk") == 0 || g_strcasecmp(keyMgmtVal, "wpa-psk-sha256") == 0 ||
g_strcasecmp(keyMgmtVal, "wpa-ft-psk") == 0)
Expand All @@ -1608,7 +1608,7 @@ bool ConnectivityManagerImpl::_GetBssInfo(const gchar * bssPath, NetworkCommissi
res |= (1 << 4); // SecurityType::WPA3_PERSONAL
}
}
g_free(keyMgmtsForFree);

return res;
};
auto GetNetworkSecurityType = [IsNetworkWPAPSK, IsNetworkWPA2PSK](WpaFiW1Wpa_supplicant1BSSProxy * proxy) -> uint8_t {
Expand Down
23 changes: 9 additions & 14 deletions src/platform/Linux/bluez/Helper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,12 +400,10 @@ static gboolean BluezCharacteristicAcquireWrite(BluezGattCharacteristic1 * aChar
return FALSE;
}

options = GAutoPtr<GVariantDict>(g_variant_dict_new(aOptions));
option_mtu = GAutoPtr<GVariant>(g_variant_dict_lookup_value(options.get(), "mtu", G_VARIANT_TYPE_UINT16));
VerifyOrReturnValue(
option_mtu != nullptr, FALSE, ChipLogError(DeviceLayer, "FAIL: No MTU in options in %s", __func__);
g_variant_lookup(aOptions, "mtu", "q", &conn->mMtu), FALSE,
ChipLogError(DeviceLayer, "FAIL: No MTU in options in %s", __func__);
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.InvalidArguments", "MTU negotiation failed"));
conn->mMtu = g_variant_get_uint16(option_mtu.get());

channel = g_io_channel_unix_new(fds[0]);
g_io_channel_set_encoding(channel, nullptr, nullptr);
Expand Down Expand Up @@ -462,13 +460,10 @@ static gboolean BluezCharacteristicAcquireNotify(BluezGattCharacteristic1 * aCha
conn != nullptr, FALSE,
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.Failed", "No Chipoble connection"));

options = GAutoPtr<GVariantDict>(g_variant_dict_new(aOptions));
option_mtu = GAutoPtr<GVariant>(g_variant_dict_lookup_value(options.get(), "mtu", G_VARIANT_TYPE_UINT16));
VerifyOrReturnValue(option_mtu != nullptr, FALSE, {
VerifyOrReturnValue(g_variant_lookup(aOptions, "mtu", "q", &conn->mMtu), FALSE, {
ChipLogError(DeviceLayer, "FAIL: No MTU in options in %s", __func__);
g_dbus_method_invocation_return_dbus_error(aInvocation, "org.bluez.Error.InvalidArguments", "MTU negotiation failed");
});
conn->mMtu = g_variant_get_uint16(option_mtu.get());

if (socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0, fds) < 0)
{
Expand Down Expand Up @@ -1237,19 +1232,19 @@ static CHIP_ERROR BluezC2Indicate(ConnectionDataBundle * closure)

if (bluez_gatt_characteristic1_get_notify_acquired(conn->mpC2) == TRUE)
{
buf = (char *) g_variant_get_fixed_array(closure->mpVal.get(), &len, sizeof(uint8_t));
buf = (char *) g_variant_get_fixed_array(closure->mpVal, &len, sizeof(uint8_t));
VerifyOrExit(len <= static_cast<size_t>(std::numeric_limits<gssize>::max()),
ChipLogError(DeviceLayer, "FAIL: buffer too large in %s", __func__));
status = g_io_channel_write_chars(conn->mC2Channel.mpChannel, buf, static_cast<gssize>(len), &written,
&MakeUniquePointerReceiver(error).Get());
g_variant_unref(closure->mpVal.get());
g_variant_unref(closure->mpVal);
closure->mpVal = nullptr;

VerifyOrExit(status == G_IO_STATUS_NORMAL, ChipLogError(DeviceLayer, "FAIL: C2 Indicate: %s", error->message));
}
else
{
bluez_gatt_characteristic1_set_value(conn->mpC2, closure->mpVal.get());
bluez_gatt_characteristic1_set_value(conn->mpC2, closure->mpVal);
closure->mpVal = nullptr;
}

Expand All @@ -1266,8 +1261,8 @@ static ConnectionDataBundle * MakeConnectionDataBundle(BLE_CONNECTION_OBJECT apC
{
ConnectionDataBundle * bundle = g_new(ConnectionDataBundle, 1);
bundle->mpConn = static_cast<BluezConnection *>(apConn);
bundle->mpVal = GAutoPtr<GVariant>(
g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, apBuf->Start(), apBuf->DataLength() * sizeof(uint8_t), sizeof(uint8_t)));
bundle->mpVal =
g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE, apBuf->Start(), apBuf->DataLength() * sizeof(uint8_t), sizeof(uint8_t));
return bundle;
}

Expand Down Expand Up @@ -1434,7 +1429,7 @@ static CHIP_ERROR SendWriteRequestImpl(ConnectionDataBundle * data)
g_variant_builder_add(&optionsBuilder, "{sv}", "type", g_variant_new_string("request"));
options = g_variant_builder_end(&optionsBuilder);

bluez_gatt_characteristic1_call_write_value(data->mpConn->mpC1, data->mpVal.get(), options, nullptr, SendWriteRequestDone,
bluez_gatt_characteristic1_call_write_value(data->mpConn->mpC1, data->mpVal, options, nullptr, SendWriteRequestDone,
data->mpConn);

exit:
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Linux/bluez/Types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ struct BluezConnection
struct ConnectionDataBundle
{
BluezConnection * mpConn;
GAutoPtr<GVariant> mpVal;
GVariant * mpVal;
};

} // namespace Internal
Expand Down

0 comments on commit 84e1d20

Please sign in to comment.