From f15498eda31bb5f229c56cc668fe484c1ef37d3a Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 26 Aug 2021 17:25:11 +0200
Subject: [PATCH] ifcfg-rh: cleanup make_bond_port_setting()

- use svGetValue() instead of svGetValueStr(). The difference is that
  svGetValueStr() coerces "" to NULL. "" is not a valid value, but we
  want to parse the value and print an warning message about it. Also,
  the presence of the variable determines whether we add the bond-port
  setting or not.

- don't use nm_clear_g_free(). @value_to_free is gs_free, it will be
  cleared automatically.

- use g_object_set() instead of nm_g_object_set_property_uint(). The
  latter is our own implementation that does error checking (e.g., that
  the value is in range (0..2^16-1). But we already ensured that to
  be the case. So just call g_object_set(), it cannot fail and if it
  would, we want the assertion failure that it would cause.

- queue_id should be a "guint". It is always true on Linux/glib that
  sizeof(guint) >= sizeof(guint32), the opposite theoretically might not
  be true.
  But later we use the variable in the variadic function g_object_set(),
  where it should be guint.

- the errno from _nm_utils_ascii_str_to_uint64() isn't very useful for
  logging. It's either ERANGE or EINVAL, and logging the numeric values
  of these error codes isn't gonna help the user. We could stringify
  with nm_strerror_native(errno), but that message is also not very
  useful. Just say that the string is not a number.
---
 .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c         | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
index 200e9a588e..f48b04e2df 100644
--- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
+++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
@@ -5500,26 +5500,20 @@ make_bond_port_setting(shvarFile *ifcfg)
     NMSetting *   s_port        = NULL;
     gs_free char *value_to_free = NULL;
     const char *  value;
-    guint32       queue_id = 0;
+    guint         queue_id;
 
     g_return_val_if_fail(ifcfg != NULL, FALSE);
 
-    value = svGetValueStr(ifcfg, "BOND_PORT_QUEUE_ID", &value_to_free);
+    value = svGetValue(ifcfg, "BOND_PORT_QUEUE_ID", &value_to_free);
     if (value) {
         s_port = nm_setting_bond_port_new();
         queue_id =
             _nm_utils_ascii_str_to_uint64(value, 10, 0, G_MAXUINT16, NM_BOND_PORT_QUEUE_ID_DEF);
         if (errno != 0) {
-            PARSE_WARNING("Invalid bond port queue_id value '%s': error %d", value, errno);
-            nm_clear_g_free(&value_to_free);
+            PARSE_WARNING("Invalid bond port queue_id value '%s'", value);
             return s_port;
-        } else {
-            nm_clear_g_free(&value_to_free);
-            nm_g_object_set_property_uint(G_OBJECT(s_port),
-                                          NM_SETTING_BOND_PORT_QUEUE_ID,
-                                          queue_id,
-                                          NULL);
         }
+        g_object_set(G_OBJECT(s_port), NM_SETTING_BOND_PORT_QUEUE_ID, queue_id, NULL);
     }
 
     return s_port;