From bbfd366805f8c389a9445337499aabd04cb0929f Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 18 Apr 2019 08:08:32 +0200
Subject: [PATCH] cli: cleanup _get_fcn_vlan_xgress_priority_map()

- merge the pointless helper function vlan_priorities_to_string()
  into the only caller _get_fcn_vlan_xgress_priority_map().

- minor cleanups, like setting out-is-default if num==0, not
  based on whether we have a non-empty string. There is not difference
  in practice, because nm_setting_vlan_get_priority() never fails.
  Hence they are identical. But nm_setting_vlan_get_priority() has
  an API that allows it to fail, so we should declare the default
  depending on the number of vlan priorities.

- don't allocate the temporary GString instance if we won't need it.

- only append the delimiter if needed, and not truncate it afterwards.
  It might have even worse performance this way, but it feels more
  correct to me.

- also cache the result of nm_setting_vlan_get_num_priorities().
  NMSettingVlan's implementation is horrible and uses a GSList to
  track the list of priorities. This makes it relatively expensive
  to call get-num-priorities repeatedly (and pointless).
---
 clients/common/nm-meta-setting-desc.c | 44 +++++++++++++--------------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c
index 1d1a5c6d92..53a945c827 100644
--- a/clients/common/nm-meta-setting-desc.c
+++ b/clients/common/nm-meta-setting-desc.c
@@ -1777,25 +1777,6 @@ vlan_flags_to_string (guint32 flags, NMMetaAccessorGetType get_type)
 	return g_string_free (flag_str, FALSE);
 }
 
-static char *
-vlan_priorities_to_string (NMSettingVlan *s_vlan, NMVlanPriorityMap map)
-{
-	GString *priorities;
-	int i;
-
-	priorities = g_string_new (NULL);
-	for (i = 0; i < nm_setting_vlan_get_num_priorities (s_vlan, map); i++) {
-		guint32 from, to;
-
-		if (nm_setting_vlan_get_priority (s_vlan, map, i, &from, &to))
-			g_string_append_printf (priorities, "%d:%d,", from, to);
-	}
-	if (priorities->len)
-		g_string_truncate (priorities, priorities->len-1);  /* chop off trailing ',' */
-
-	return g_string_free (priorities, FALSE);
-}
-
 static char *
 secret_flags_to_string (guint32 flags, NMMetaAccessorGetType get_type)
 {
@@ -3862,14 +3843,31 @@ _vlan_priority_map_type_from_property_info (const NMMetaPropertyInfo *property_i
 static gconstpointer
 _get_fcn_vlan_xgress_priority_map (ARGS_GET_FCN)
 {
+	NMVlanPriorityMap map_type = _vlan_priority_map_type_from_property_info (property_info);
 	NMSettingVlan *s_vlan = NM_SETTING_VLAN (setting);
-	char *str;
+	GString *str = NULL;
+	guint32 i, num;
 
 	RETURN_UNSUPPORTED_GET_TYPE ();
 
-	str = vlan_priorities_to_string (s_vlan, _vlan_priority_map_type_from_property_info (property_info));
-	NM_SET_OUT (out_is_default, !str || !str[0]);
-	RETURN_STR_TO_FREE (str);
+	num = nm_setting_vlan_get_num_priorities (s_vlan, map_type);
+	for (i = 0; i < num; i++) {
+		guint32 from, to;
+
+		if (!nm_setting_vlan_get_priority (s_vlan, map_type, i, &from, &to))
+			continue;
+
+		if (!str)
+			str = g_string_new (NULL);
+		else
+			g_string_append_c (str, ',');
+		g_string_append_printf (str, "%d:%d", from, to);
+	}
+
+	NM_SET_OUT (out_is_default, num == 0);
+	if (!str)
+		return NULL;
+	RETURN_STR_TO_FREE (g_string_free (str, FALSE));
 }
 
 static gboolean