lldp: use GVariantBuilder instead of GVariantDict

GVariantDict is basically a GHashTable, and during g_variant_dict_end()
it uses a GVariantBuilder to create the variant.

This is totally unnecessary in this case. It's probably unnecessary in
most use cases, because commonly we construct variants in a determined series
of steps and don't need to add/remove keys.

Aside the overhead, GHashTable also does not give a stable sort order,
which seems a pretty bad property in this case.

Note that the code changes the order in which we call
g_variant_builder_add() for the fields in code, to preserve the previous
order that GVariantDict actually created (at least, with my version of
glib).
This commit is contained in:
Thomas Haller
2020-06-07 18:15:15 +02:00
parent cf4763207f
commit ae0da6dd60

View File

@@ -224,11 +224,17 @@ lldp_neighbor_equal (LldpNeighbor *a, LldpNeighbor *b)
} }
static GVariant * static GVariant *
parse_management_address_tlv (uint8_t *data, gsize len) parse_management_address_tlv (const uint8_t *data, gsize len)
{ {
GVariantDict dict; GVariantBuilder builder;
GVariant *variant; gsize addr_len;
gsize addr_len, oid_len; const guint8 *v_object_id_arr;
gsize v_object_id_len;
const guint8 *v_address_arr;
gsize v_address_len;
guint32 v_interface_number;
guint32 v_interface_number_subtype;
guint32 v_address_subtype;
/* 802.1AB-2009 - Figure 8-11 /* 802.1AB-2009 - Figure 8-11
* *
@@ -243,7 +249,7 @@ parse_management_address_tlv (uint8_t *data, gsize len)
*/ */
if (len < 11) if (len < 11)
goto err; return NULL;
nm_assert ((data[0] >> 1) == SD_LLDP_TYPE_MGMT_ADDRESS); nm_assert ((data[0] >> 1) == SD_LLDP_TYPE_MGMT_ADDRESS);
nm_assert ((((data[0] & 1) << 8) + data[1]) + 2 == len); nm_assert ((((data[0] & 1) << 8) + data[1]) + 2 == len);
@@ -253,43 +259,42 @@ parse_management_address_tlv (uint8_t *data, gsize len)
addr_len = *data; /* length of (address subtype + address) */ addr_len = *data; /* length of (address subtype + address) */
if (addr_len < 2 || addr_len > 32) if (addr_len < 2 || addr_len > 32)
goto err; return NULL;
if (len < ( 1 /* address stringth length */ if (len < ( 1 /* address stringth length */
+ addr_len /* address subtype + address */ + addr_len /* address subtype + address */
+ 5 /* interface */ + 5 /* interface */
+ 1)) /* oid */ + 1)) /* oid */
goto err; return NULL;
g_variant_dict_init (&dict, NULL);
data++; data++;
len--; len--;
g_variant_dict_insert (&dict, "address-subtype", "u", (guint32) *data); v_address_subtype = *data;
variant = g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, data + 1, addr_len - 1, 1); v_address_arr = &data[1];
g_variant_dict_insert_value (&dict, "address", variant); v_address_len = addr_len - 1;
data += addr_len; data += addr_len;
len -= addr_len; len -= addr_len;
g_variant_dict_insert (&dict, "interface-number-subtype", "u", (guint32) *data); v_interface_number_subtype = *data;
data++; data++;
len--; len--;
g_variant_dict_insert (&dict, "interface-number", "u", unaligned_read_be32 (data)); v_interface_number = unaligned_read_be32 (data);
data += 4; data += 4;
len -= 4; len -= 4;
oid_len = *data; v_object_id_len = *data;
if (len < (1 + v_object_id_len))
if (len < (1 + oid_len)) return NULL;
goto err;
data++; data++;
variant = g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, data, oid_len, 1); v_object_id_arr = data;
g_variant_dict_insert_value (&dict, "object-id", variant);
return g_variant_dict_end (&dict); g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
err: nm_g_variant_builder_add_sv_uint32 (&builder, "address-subtype", v_address_subtype);
g_variant_dict_clear (&dict); nm_g_variant_builder_add_sv_bytearray (&builder, "object-id", v_object_id_arr, v_object_id_len);
return NULL; nm_g_variant_builder_add_sv_uint32 (&builder, "interface-number", v_interface_number);
nm_g_variant_builder_add_sv_bytearray (&builder, "address", v_address_arr, v_address_len);
nm_g_variant_builder_add_sv_uint32 (&builder, "interface-number-subtype", v_interface_number_subtype);
return g_variant_builder_end (&builder);
} }
static LldpNeighbor * static LldpNeighbor *
@@ -448,6 +453,7 @@ lldp_neighbor_to_variant (LldpNeighbor *neigh)
GVariant *v_ieee_802_3_mac_phy_conf = NULL; GVariant *v_ieee_802_3_mac_phy_conf = NULL;
GVariant *v_ieee_802_3_power_via_mdi = NULL; GVariant *v_ieee_802_3_power_via_mdi = NULL;
GVariant *v_ieee_802_3_max_frame_size = NULL; GVariant *v_ieee_802_3_max_frame_size = NULL;
GVariantBuilder tmp_builder;
GVariant *tmp_variant; GVariant *tmp_variant;
do { do {
@@ -511,8 +517,6 @@ lldp_neighbor_to_variant (LldpNeighbor *neigh)
len -= 6; len -= 6;
if (memcmp (oui, SD_LLDP_OUI_802_1, sizeof (oui)) == 0) { if (memcmp (oui, SD_LLDP_OUI_802_1, sizeof (oui)) == 0) {
GVariantDict dict;
switch (subtype) { switch (subtype) {
case SD_LLDP_OUI_802_1_SUBTYPE_PORT_VLAN_ID: case SD_LLDP_OUI_802_1_SUBTYPE_PORT_VLAN_ID:
if (len != 2) if (len != 2)
@@ -528,10 +532,10 @@ lldp_neighbor_to_variant (LldpNeighbor *neigh)
v_ieee_802_1_ppvid = g_variant_new_uint32 (unaligned_read_be16 (&data8[1])); v_ieee_802_1_ppvid = g_variant_new_uint32 (unaligned_read_be16 (&data8[1]));
g_variant_builder_init (&v_ieee_802_1_ppvids, G_VARIANT_TYPE ("aa{sv}")); g_variant_builder_init (&v_ieee_802_1_ppvids, G_VARIANT_TYPE ("aa{sv}"));
} }
g_variant_dict_init (&dict, NULL); g_variant_builder_init (&tmp_builder, G_VARIANT_TYPE ("a{sv}"));
g_variant_dict_insert (&dict, "ppvid", "u", (guint32) unaligned_read_be16 (&data8[1])); nm_g_variant_builder_add_sv_uint32 (&tmp_builder, "flags", data8[0]);
g_variant_dict_insert (&dict, "flags", "u", (guint32) data8[0]); nm_g_variant_builder_add_sv_uint32 (&tmp_builder, "ppvid", unaligned_read_be16 (&data8[1]));
g_variant_builder_add_value (&v_ieee_802_1_ppvids, g_variant_dict_end (&dict)); g_variant_builder_add_value (&v_ieee_802_1_ppvids, g_variant_builder_end (&tmp_builder));
break; break;
case SD_LLDP_OUI_802_1_SUBTYPE_VLAN_NAME: { case SD_LLDP_OUI_802_1_SUBTYPE_VLAN_NAME: {
gs_free char *name_to_free = NULL; gs_free char *name_to_free = NULL;
@@ -556,29 +560,27 @@ lldp_neighbor_to_variant (LldpNeighbor *neigh)
v_ieee_802_1_vlan_name = g_variant_new_string (name); v_ieee_802_1_vlan_name = g_variant_new_string (name);
g_variant_builder_init (&v_ieee_802_1_vlans, G_VARIANT_TYPE ("aa{sv}")); g_variant_builder_init (&v_ieee_802_1_vlans, G_VARIANT_TYPE ("aa{sv}"));
} }
g_variant_dict_init (&dict, NULL); g_variant_builder_init (&tmp_builder, G_VARIANT_TYPE ("a{sv}"));
g_variant_dict_insert (&dict, "vid", "u", vid); nm_g_variant_builder_add_sv_uint32 (&tmp_builder, "vid", vid);
g_variant_dict_insert (&dict, "name", "s", name); nm_g_variant_builder_add_sv_str (&tmp_builder, "name", name);
g_variant_builder_add_value (&v_ieee_802_1_vlans, g_variant_dict_end (&dict)); g_variant_builder_add_value (&v_ieee_802_1_vlans, g_variant_builder_end (&tmp_builder));
break; break;
} }
default: default:
continue; continue;
} }
} else if (memcmp (oui, SD_LLDP_OUI_802_3, sizeof (oui)) == 0) { } else if (memcmp (oui, SD_LLDP_OUI_802_3, sizeof (oui)) == 0) {
GVariantDict dict;
switch (subtype) { switch (subtype) {
case SD_LLDP_OUI_802_3_SUBTYPE_MAC_PHY_CONFIG_STATUS: case SD_LLDP_OUI_802_3_SUBTYPE_MAC_PHY_CONFIG_STATUS:
if (len != 5) if (len != 5)
continue; continue;
if (!v_ieee_802_3_mac_phy_conf) { if (!v_ieee_802_3_mac_phy_conf) {
g_variant_dict_init (&dict, NULL); g_variant_builder_init (&tmp_builder, G_VARIANT_TYPE ("a{sv}"));
g_variant_dict_insert (&dict, "autoneg", "u", (guint32) data8[0]); nm_g_variant_builder_add_sv_uint32 (&tmp_builder, "operational-mau-type", unaligned_read_be16 (&data8[3]));
g_variant_dict_insert (&dict, "pmd-autoneg-cap", "u", (guint32) unaligned_read_be16 (&data8[1])); nm_g_variant_builder_add_sv_uint32 (&tmp_builder, "autoneg", data8[0]);
g_variant_dict_insert (&dict, "operational-mau-type", "u", (guint32) unaligned_read_be16 (&data8[3])); nm_g_variant_builder_add_sv_uint32 (&tmp_builder, "pmd-autoneg-cap", unaligned_read_be16 (&data8[1]));
v_ieee_802_3_mac_phy_conf = g_variant_dict_end (&dict); v_ieee_802_3_mac_phy_conf = g_variant_builder_end (&tmp_builder);
} }
break; break;
case SD_LLDP_OUI_802_3_SUBTYPE_POWER_VIA_MDI: case SD_LLDP_OUI_802_3_SUBTYPE_POWER_VIA_MDI:
@@ -586,11 +588,11 @@ lldp_neighbor_to_variant (LldpNeighbor *neigh)
continue; continue;
if (!v_ieee_802_3_power_via_mdi) { if (!v_ieee_802_3_power_via_mdi) {
g_variant_dict_init (&dict, NULL); g_variant_builder_init (&tmp_builder, G_VARIANT_TYPE ("a{sv}"));
g_variant_dict_insert (&dict, "mdi-power-support", "u", (guint32) data8[0]); nm_g_variant_builder_add_sv_uint32 (&tmp_builder, "pse-power-pair", data8[1]);
g_variant_dict_insert (&dict, "pse-power-pair", "u", (guint32) data8[1]); nm_g_variant_builder_add_sv_uint32 (&tmp_builder, "mdi-power-support", data8[0]);
g_variant_dict_insert (&dict, "power-class", "u", (guint32) data8[2]); nm_g_variant_builder_add_sv_uint32 (&tmp_builder, "power-class", data8[2]);
v_ieee_802_3_power_via_mdi = g_variant_dict_end (&dict); v_ieee_802_3_power_via_mdi = g_variant_builder_end (&tmp_builder);
} }
break; break;
case SD_LLDP_OUI_802_3_SUBTYPE_MAXIMUM_FRAME_SIZE: case SD_LLDP_OUI_802_3_SUBTYPE_MAXIMUM_FRAME_SIZE: