keyfile: various refactoring and restructure nm_keyfile_read()

- in nm_keyfile_read(), unify _read_setting() and
  _read_setting_vpn_secret() in they way they are called
  (that is, they no longer return any value and don't accept
  any arguments aside @info).

- use cleanup attributes

- use nm_streq() instead of strcmp().

- wrap lines that have multiple statements or conditions.

4 files changed, 145 insertions, 125 deletions

diff --git a/libnm-core/nm-keyfile-utils.c b/libnm-core/nm-keyfile-utils.c
index bc9da61485..ff12fa1063 100644
--- a/libnm-core/nm-keyfile-utils.c
+++ b/libnm-core/nm-keyfile-utils.c
@@ -48,7 +48,7 @@ nm_keyfile_plugin_get_alias_for_setting_name (const char *setting_name)
 	g_return_val_if_fail (setting_name != NULL, NULL);
 
 	for (i = 0; i < G_N_ELEMENTS (alias_list); i++) {
-		if (strcmp (setting_name, alias_list[i].setting) == 0)
+		if (nm_streq (setting_name, alias_list[i].setting))
 			return alias_list[i].alias;
 	}
 	return NULL;
@@ -62,7 +62,7 @@ nm_keyfile_plugin_get_setting_name_for_alias (const char *alias)
 	g_return_val_if_fail (alias != NULL, NULL);
 
 	for (i = 0; i < G_N_ELEMENTS (alias_list); i++) {
-		if (strcmp (alias, alias_list[i].alias) == 0)
+		if (nm_streq (alias, alias_list[i].alias))
 			return alias_list[i].setting;
 	}
 	return NULL;
diff --git a/libnm-core/nm-keyfile-utils.h b/libnm-core/nm-keyfile-utils.h
index 46a6d56432..18d5dce831 100644
--- a/libnm-core/nm-keyfile-utils.h
+++ b/libnm-core/nm-keyfile-utils.h
@@ -90,4 +90,3 @@ const char *nm_keyfile_key_decode (const char *key,
                                    char **out_to_free);
 
 #endif  /* __NM_KEYFILE_UTILS_H__ */
-
diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c
index b3c4cdd53b..cec09c9f38 100644
--- a/libnm-core/nm-keyfile.c
+++ b/libnm-core/nm-keyfile.c
@@ -106,17 +106,18 @@ static void
 setting_alias_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key)
 {
 	const char *setting_name = nm_setting_get_name (setting);
-	char *s;
 	const char *key_setting_name;
+	gs_free char *s = NULL;
 
 	s = nm_keyfile_plugin_kf_get_string (info->keyfile, setting_name, key, NULL);
-	if (s) {
-		key_setting_name = nm_keyfile_plugin_get_setting_name_for_alias (s);
-		g_object_set (G_OBJECT (setting),
-		              key, key_setting_name ?: s,
-		              NULL);
-		g_free (s);
-	}
+	if (!s)
+		return;
+
+	key_setting_name = nm_keyfile_plugin_get_setting_name_for_alias (s);
+	g_object_set (G_OBJECT (setting),
+	              key,
+	              key_setting_name ?: s,
+	              NULL);
 }
 
 static void
@@ -129,7 +130,7 @@ sriov_vfs_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key)
 	int i;
 
 	keys = nm_keyfile_plugin_kf_get_keys (info->keyfile, setting_name, &n_keys, NULL);
-	if (!keys || n_keys == 0)
+	if (n_keys == 0)
 		return;
 
 	vfs = g_ptr_array_new_with_free_func ((GDestroyNotify) nm_sriov_vf_unref);
@@ -622,17 +623,16 @@ ip_address_or_route_parser (KeyfileReaderInfo *info, NMSetting *setting, const c
 	gs_free char *gateway = NULL;
 	gs_unref_ptrarray GPtrArray *list = NULL;
 	gs_strfreev char **keys = NULL;
-	gsize i_keys, keys_len;
+	gsize i_keys, n_keys;
 	gs_free IPAddrRouteBuildListData *build_list = NULL;
 	gsize i_build_list, build_list_len = 0;
 
-	keys = nm_keyfile_plugin_kf_get_keys (info->keyfile, setting_name, &keys_len, NULL);
-
-	if (keys_len == 0)
+	keys = nm_keyfile_plugin_kf_get_keys (info->keyfile, setting_name, &n_keys, NULL);
+	if (n_keys == 0)
 		return;
 
 	/* first create a list of all relevant keys, and sort them. */
-	for (i_keys = 0; i_keys < keys_len; i_keys++) {
+	for (i_keys = 0; i_keys < n_keys; i_keys++) {
 		const char *s_key = keys[i_keys];
 		gint32 key_idx;
 		gint8 key_type;
@@ -641,7 +641,7 @@ ip_address_or_route_parser (KeyfileReaderInfo *info, NMSetting *setting, const c
 			continue;
 
 		if (G_UNLIKELY (!build_list))
-			build_list = g_new (IPAddrRouteBuildListData, keys_len - i_keys);
+			build_list = g_new (IPAddrRouteBuildListData, n_keys - i_keys);
 
 		build_list[build_list_len].s_key = s_key;
 		build_list[build_list_len].key_idx = key_idx;
@@ -786,7 +786,6 @@ mac_address_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key
 	const char *p, *mac_str;
 	gs_free guint8 *buf_arr = NULL;
 	guint buf_len = 0;
-	gsize length;
 
 	tmp_string = nm_keyfile_plugin_kf_get_string (info->keyfile, setting_name, key, NULL);
 
@@ -819,6 +818,7 @@ mac_address_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key
 
 	if (!buf_arr) {
 		gs_free int *tmp_list = NULL;
+		gsize length;
 
 		/* Old format; list of ints */
 		tmp_list = nm_keyfile_plugin_kf_get_integer_list (info->keyfile, setting_name, key, &length, NULL);
@@ -879,9 +879,10 @@ read_hash_of_string (GKeyFile *file, NMSetting *setting, const char *key)
 	const char *const*iter;
 	const char *setting_name = nm_setting_get_name (setting);
 	gboolean is_vpn;
+	gsize n_keys;
 
-	keys = nm_keyfile_plugin_kf_get_keys (file, setting_name, NULL, NULL);
-	if (!keys || !*keys)
+	keys = nm_keyfile_plugin_kf_get_keys (file, setting_name, &n_keys, NULL);
+	if (n_keys == 0)
 		return;
 
 	if (   (is_vpn = NM_IS_SETTING_VPN (setting))
@@ -902,7 +903,7 @@ read_hash_of_string (GKeyFile *file, NMSetting *setting, const char *key)
 				if (!g_object_class_find_property (G_OBJECT_GET_CLASS (setting), name))
 					nm_setting_vpn_add_data_item (NM_SETTING_VPN (setting), name, value);
 			} else {
-				if (strcmp (name, "interface-name"))
+				if (!nm_streq (name, "interface-name"))
 					nm_setting_bond_add_option (NM_SETTING_BOND (setting), name, value);
 			}
 		}
@@ -1433,7 +1434,9 @@ team_config_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key
 	gs_free_error GError *error = NULL;
 
 	conf = nm_keyfile_plugin_kf_get_string (info->keyfile, setting_name, key, NULL);
-	if (conf && conf[0] && !nm_utils_is_json_object (conf, &error)) {
+	if (   conf
+	    && conf[0]
+	    && !nm_utils_is_json_object (conf, &error)) {
 		handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN,
 		             _("ignoring invalid team configuration: %s"),
 		             error->message);
@@ -1455,7 +1458,7 @@ qdisc_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key)
 	qdiscs = g_ptr_array_new_with_free_func ((GDestroyNotify) nm_tc_qdisc_unref);
 
 	keys = nm_keyfile_plugin_kf_get_keys (info->keyfile, setting_name, &n_keys, NULL);
-	if (!keys || n_keys == 0)
+	if (n_keys == 0)
 		return;
 
 	for (i = 0; i < n_keys; i++) {
@@ -1503,7 +1506,7 @@ tfilter_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key)
 	tfilters = g_ptr_array_new_with_free_func ((GDestroyNotify) nm_tc_tfilter_unref);
 
 	keys = nm_keyfile_plugin_kf_get_keys (info->keyfile, setting_name, &n_keys, NULL);
-	if (!keys || n_keys == 0)
+	if (n_keys == 0)
 		return;
 
 	for (i = 0; i < n_keys; i++) {
@@ -1663,16 +1666,21 @@ write_ip_values (GKeyFile *file,
                  const char *gateway,
                  gboolean is_route)
 {
-	GString *output;
-	int family, i;
-	const char *addr, *gw;
+	nm_auto_free_gstring GString *output = NULL;
+	int addr_family;
+	guint i;
+	const char *addr;
+	const char *gw;
 	guint32 plen;
-	char key_name[64], *key_name_idx;
+	char key_name[64];
+	char *key_name_idx;
 
 	if (!array->len)
 		return;
 
-	family = !strcmp (setting_name, NM_SETTING_IP4_CONFIG_SETTING_NAME) ? AF_INET : AF_INET6;
+	addr_family =   nm_streq (setting_name, NM_SETTING_IP4_CONFIG_SETTING_NAME)
+	              ? AF_INET
+	              : AF_INET6;
 
 	strcpy (key_name, is_route ? "route" : "address");
 	key_name_idx = key_name + strlen (key_name);
@@ -1693,7 +1701,9 @@ write_ip_values (GKeyFile *file,
 
 			addr = nm_ip_address_get_address (address);
 			plen = nm_ip_address_get_prefix (address);
-			gw = i == 0 ? gateway : NULL;
+			gw =   (i == 0)
+			     ? gateway
+			     : NULL;
 		}
 
 		g_string_set_size (output, 0);
@@ -1706,18 +1716,19 @@ write_ip_values (GKeyFile *file,
 			 * The current version supports reading of the above form.
 			 */
 			if (!gw) {
-				if (family == AF_INET)
+				if (addr_family == AF_INET)
 					gw = "0.0.0.0";
 				else
 					gw = "::";
 			}
 
 			g_string_append_printf (output, ",%s", gw);
-			if (is_route && metric != -1)
+			if (   is_route
+			    && metric != -1)
 				g_string_append_printf (output, ",%lu", (unsigned long) metric);
 		}
 
-		sprintf (key_name_idx, "%d", i + 1);
+		sprintf (key_name_idx, "%u", i + 1);
 		nm_keyfile_plugin_kf_set_string (file, setting_name, key_name, output->str);
 
 		if (is_route) {
@@ -1732,7 +1743,6 @@ write_ip_values (GKeyFile *file,
 			}
 		}
 	}
-	g_string_free (output, TRUE);
 }
 
 static void
@@ -1843,7 +1853,8 @@ write_hash_of_string (GKeyFile *file,
 	guint i, l;
 
 	/* Write VPN secrets out to a different group to keep them separate */
-	if (NM_IS_SETTING_VPN (setting) && !strcmp (key, NM_SETTING_VPN_SECRETS)) {
+	if (   NM_IS_SETTING_VPN (setting)
+	    && nm_streq (key, NM_SETTING_VPN_SECRETS)) {
 		group_name = NM_KEYFILE_GROUP_VPN_SECRETS;
 		vpn_secrets = TRUE;
 	}
@@ -1976,9 +1987,10 @@ cert_writer_default (NMConnection *connection,
 
 	scheme = cert_data->vtable->scheme_func (cert_data->setting);
 	if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) {
-		const char *path;
-		char *path_free = NULL, *tmp;
+		gs_free char *path_free = NULL;
 		gs_free char *base_dir = NULL;
+		gs_free char *tmp = NULL;
+		const char *path;
 
 		path = cert_data->vtable->path_func (cert_data->setting);
 		g_assert (path);
@@ -1988,7 +2000,8 @@ cert_writer_default (NMConnection *connection,
 		 * context. */
 		if (path[0] && path[0] != '/') {
 			base_dir = g_get_current_dir ();
-			path = path_free = g_strconcat (base_dir, "/", path, NULL);
+			path_free = g_strconcat (base_dir, "/", path, NULL);
+			path = path_free;
 		} else
 			base_dir = g_path_get_dirname (path);
 
@@ -1998,20 +2011,21 @@ cert_writer_default (NMConnection *connection,
 		tmp = nm_keyfile_detect_unqualified_path_scheme (base_dir, path, -1, FALSE, NULL);
 		if (tmp)
 			g_clear_pointer (&tmp, g_free);
-		else
-			path = tmp = g_strconcat (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH, path, NULL);
+		else {
+			tmp = g_strconcat (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH, path, NULL);
+			path = tmp;
+		}
 
 		/* Path contains at least a '/', hence it cannot be recognized as the old
 		 * binary format consisting of a list of integers. */
 
 		nm_keyfile_plugin_kf_set_string (file, setting_name, cert_data->vtable->setting_key, path);
-		g_free (tmp);
-		g_free (path_free);
 	} else if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) {
 		GBytes *blob;
 		const guint8 *blob_data;
 		gsize blob_len;
-		char *blob_base64, *val;
+		gs_free char *blob_base64 = NULL;
+		gs_free char *val = NULL;
 
 		blob = cert_data->vtable->blob_func (cert_data->setting);
 		g_assert (blob);
@@ -2021,8 +2035,6 @@ cert_writer_default (NMConnection *connection,
 		val = g_strconcat (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB, blob_base64, NULL);
 
 		nm_keyfile_plugin_kf_set_string (file, setting_name, cert_data->vtable->setting_key, val);
-		g_free (val);
-		g_free (blob_base64);
 	} else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PKCS11) {
 		nm_keyfile_plugin_kf_set_string (file, setting_name, cert_data->vtable->setting_key,
 		                                 cert_data->vtable->uri_func (cert_data->setting));
@@ -2048,7 +2060,7 @@ cert_writer (KeyfileWriterInfo *info,
 	NMKeyfileWriteTypeDataCert type_data = { 0 };
 
 	for (i = 0; nm_setting_8021x_scheme_vtable[i].setting_key; i++) {
-		if (g_strcmp0 (nm_setting_8021x_scheme_vtable[i].setting_key, key) == 0) {
+		if (nm_streq0 (nm_setting_8021x_scheme_vtable[i].setting_key, key)) {
 			objtype = &nm_setting_8021x_scheme_vtable[i];
 			break;
 		}
@@ -2621,7 +2633,7 @@ read_one_setting_value (NMSetting *setting,
 		} else
 			g_object_set (setting, key, int_val, NULL);
 	} else if (type == G_TYPE_BYTES) {
-		int *tmp;
+		gs_free int *tmp = NULL;
 		GByteArray *array;
 		GBytes *bytes;
 		gsize length;
@@ -2632,7 +2644,7 @@ read_one_setting_value (NMSetting *setting,
 
 		array = g_byte_array_sized_new (length);
 		for (i = 0; i < length; i++) {
-			int val = tmp[i];
+			const int val = tmp[i];
 			unsigned char v = (unsigned char) (val & 0xFF);
 
 			if (val < 0 || val > 255) {
@@ -2641,7 +2653,6 @@ read_one_setting_value (NMSetting *setting,
 				                     _("ignoring invalid byte element '%d' (not between 0 and 255 inclusive)"),
 				                     val)) {
 					g_byte_array_unref (array);
-					g_free (tmp);
 					return;
 				}
 				already_warned = TRUE;
@@ -2652,14 +2663,12 @@ read_one_setting_value (NMSetting *setting,
 		bytes = g_byte_array_free_to_bytes (array);
 		g_object_set (setting, key, bytes, NULL);
 		g_bytes_unref (bytes);
-		g_free (tmp);
 	} else if (type == G_TYPE_STRV) {
-		char **sa;
+		gs_strfreev char **sa = NULL;
 		gsize length;
 
 		sa = nm_keyfile_plugin_kf_get_string_list (keyfile, setting_name, key, &length, NULL);
 		g_object_set (setting, key, sa, NULL);
-		g_strfreev (sa);
 	} else if (type == G_TYPE_HASH_TABLE) {
 		read_hash_of_string (keyfile, setting, key);
 	} else if (type == G_TYPE_ARRAY) {
@@ -2693,8 +2702,8 @@ read_one_setting_value (NMSetting *setting,
 	}
 }
 
-static NMSetting *
-read_setting (KeyfileReaderInfo *info)
+static void
+_read_setting (KeyfileReaderInfo *info)
 {
 	const NMSettInfoSetting *sett_info;
 	gs_unref_object NMSetting *setting = NULL;
@@ -2709,7 +2718,7 @@ read_setting (KeyfileReaderInfo *info)
 	if (!type) {
 		handle_warn (info, NULL, NM_KEYFILE_WARN_SEVERITY_WARN,
 		             _("invalid setting name '%s'"), info->group);
-		return NULL;
+		return;
 	}
 
 	setting = g_object_new (type, NULL);
@@ -2735,7 +2744,7 @@ read_setting (KeyfileReaderInfo *info)
 				const GVariantType *variant_type;
 				GVariant *variant;
 
-				/* a GKeyfile can return duplicate keys, there is just no API to make sense
+				/* a GKeyFile can return duplicate keys, there is just no API to make sense
 				 * of them. Skip them. */
 				if (   i + 1 < n_keys
 				    && nm_streq (key, keys[i + 1]))
@@ -2786,31 +2795,39 @@ read_setting (KeyfileReaderInfo *info)
 			for (; i < n_keys; i++)
 				g_free (keys[i]);
 		}
-	} else
-		nm_setting_enumerate_values (setting, read_one_setting_value, info);
+		goto out;
+	}
 
-	info->setting = NULL;
+	nm_setting_enumerate_values (setting, read_one_setting_value, info);
 
-	if (info->error)
-		return NULL;
-	return g_steal_pointer (&setting);
+out:
+	info->setting = NULL;
+	if (!info->error)
+		nm_connection_add_setting (info->connection, g_steal_pointer (&setting));
 }
 
 static void
-read_vpn_secrets (KeyfileReaderInfo *info, NMSettingVpn *s_vpn)
+_read_setting_vpn_secrets (KeyfileReaderInfo *info)
 {
 	gs_strfreev char **keys = NULL;
 	gsize i, n_keys;
+	NMSettingVpn *s_vpn;
+
+	s_vpn = nm_connection_get_setting_vpn (info->connection);
+	if (!s_vpn) {
+		/* if we don't also have a [vpn] section (which must be parsed earlier),
+		 * we don't do anything. */
+		nm_assert (!g_key_file_has_group (info->keyfile, "vpn"));
+		return;
+	}
 
 	keys = nm_keyfile_plugin_kf_get_keys (info->keyfile, NM_KEYFILE_GROUP_VPN_SECRETS, &n_keys, NULL);
 	for (i = 0; i < n_keys; i++) {
-		char *secret;
+		gs_free char *secret = NULL;
 
 		secret = nm_keyfile_plugin_kf_get_string (info->keyfile, NM_KEYFILE_GROUP_VPN_SECRETS, keys[i], NULL);
-		if (secret) {
+		if (secret)
 			nm_setting_vpn_add_secret (s_vpn, keys[i], secret);
-			g_free (secret);
-		}
 	}
 }
 
@@ -2878,12 +2895,11 @@ nm_keyfile_read (GKeyFile *keyfile,
 {
 	gs_unref_object NMConnection *connection = NULL;
 	NMSettingConnection *s_con;
-	NMSetting *setting;
 	gs_strfreev char **groups = NULL;
-	gsize length;
+	gsize n_groups;
 	gsize i;
 	gboolean vpn_secrets = FALSE;
-	KeyfileReaderInfo info = { 0 };
+	KeyfileReaderInfo info;
 
 	g_return_val_if_fail (keyfile, NULL);
 	g_return_val_if_fail (!error || !*error, NULL);
@@ -2891,32 +2907,32 @@ nm_keyfile_read (GKeyFile *keyfile,
 
 	connection = nm_simple_connection_new ();
 
-	info.connection = connection;
-	info.keyfile = (GKeyFile *) keyfile;
-	info.base_dir = base_dir;
-	info.handler = handler;
-	info.user_data = user_data;
+	info = (KeyfileReaderInfo) {
+		.connection = connection,
+		.keyfile    = keyfile,
+		.base_dir   = base_dir,
+		.handler    = handler,
+		.user_data  = user_data,
+	};
 
-	groups = g_key_file_get_groups (keyfile, &length);
+	groups = g_key_file_get_groups (keyfile, &n_groups);
 	if (!groups)
-		length = 0;
+		n_groups = 0;
 
-	for (i = 0; i < length; i++) {
-		/* Only read out secrets when needed */
-		if (!strcmp (groups[i], NM_KEYFILE_GROUP_VPN_SECRETS)) {
-			vpn_secrets = TRUE;
-			continue;
-		}
+	for (i = 0; i < n_groups; i++) {
 
 		info.group = groups[i];
-		setting = read_setting (&info);
+
+		if (nm_streq (groups[i], NM_KEYFILE_GROUP_VPN_SECRETS)) {
+			/* Only read out secrets when needed */
+			vpn_secrets = TRUE;
+		} else
+			_read_setting (&info);
+
 		info.group = NULL;
-		if (info.error) {
-			g_propagate_error (error, info.error);
-			return NULL;
-		}
-		if (setting)
-			nm_connection_add_setting (connection, setting);
+
+		if (info.error)
+			goto out_with_info_error;
 	}
 
 	s_con = nm_connection_get_setting_connection (connection);
@@ -2930,33 +2946,29 @@ nm_keyfile_read (GKeyFile *keyfile,
 	 */
 	if (   !nm_setting_connection_get_interface_name (s_con)
 	    && nm_setting_connection_get_connection_type (s_con)) {
-		char *interface_name;
+		gs_free char *interface_name = NULL;
 
 		interface_name = g_key_file_get_string (keyfile,
 		                                        nm_setting_connection_get_connection_type (s_con),
 		                                        "interface-name",
 		                                        NULL);
-		if (interface_name) {
+		if (interface_name)
 			g_object_set (s_con, NM_SETTING_CONNECTION_INTERFACE_NAME, interface_name, NULL);
-			g_free (interface_name);
-		}
 	}
 
-	/* Handle vpn secrets after the 'vpn' setting was read */
 	if (vpn_secrets) {
-		NMSettingVpn *s_vpn;
-
-		s_vpn = nm_connection_get_setting_vpn (connection);
-		if (s_vpn) {
-			read_vpn_secrets (&info, s_vpn);
-			if (info.error) {
-				g_propagate_error (error, info.error);
-				return NULL;
-			}
-		}
+		info.group = NM_KEYFILE_GROUP_VPN_SECRETS;
+		_read_setting_vpn_secrets (&info);
+		info.group = NULL;;
+		if (info.error)
+			goto out_with_info_error;
 	}
 
 	return g_steal_pointer (&connection);
+
+out_with_info_error:
+	g_propagate_error (error, info.error);
+	return NULL;
 }
 
 /*****************************************************************************/
@@ -3086,9 +3098,10 @@ nm_keyfile_write (NMConnection *connection,
                   void *user_data,
                   GError **error)
 {
-	KeyfileWriterInfo info = { 0 };
+	gs_unref_keyfile GKeyFile *keyfile = NULL;
+	KeyfileWriterInfo info;
 	gs_free NMSetting **settings = NULL;
-	guint i, length = 0;
+	guint i, n_settings = 0;
 
 	g_return_val_if_fail (NM_IS_CONNECTION (connection), NULL);
 	g_return_val_if_fail (!error || !*error, NULL);
@@ -3096,14 +3109,18 @@ nm_keyfile_write (NMConnection *connection,
 	if (!nm_connection_verify (connection, error))
 		return NULL;
 
-	info.connection = connection;
-	info.keyfile = g_key_file_new ();
-	info.error = NULL;
-	info.handler = handler;
-	info.user_data = user_data;
+	keyfile = g_key_file_new ();
 
-	settings = nm_connection_get_settings (connection, &length);
-	for (i = 0; i < length; i++) {
+	info = (KeyfileWriterInfo) {
+		.connection = connection,
+		.keyfile    = keyfile,
+		.error      = NULL,
+		.handler    = handler,
+		.user_data  = user_data,
+	};
+
+	settings = nm_connection_get_settings (connection, &n_settings);
+	for (i = 0; i < n_settings; i++) {
 		const NMSettInfoSetting *sett_info;
 		NMSetting *setting = settings[i];
 
@@ -3142,20 +3159,24 @@ nm_keyfile_write (NMConnection *connection,
 					}
 				}
 			}
-		} else
-			nm_setting_enumerate_values (setting, write_setting_value, &info);
+			goto next;
+		}
 
+		nm_setting_enumerate_values (setting, write_setting_value, &info);
+
+next:
 		if (info.error)
-			break;
+			goto out_with_info_error;
 	}
 
-	if (info.error) {
-		g_propagate_error (error, info.error);
-		g_key_file_unref (info.keyfile);
-		return NULL;
-	}
+	if (info.error)
+		goto out_with_info_error;
 
-	return info.keyfile;
+	return g_steal_pointer (&keyfile);
+
+out_with_info_error:
+	g_propagate_error (error, info.error);
+	return NULL;
 }
 
 /*****************************************************************************/
diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c
index 87094a30df..9b020429d6 100644
--- a/shared/nm-utils/nm-shared-utils.c
+++ b/shared/nm-utils/nm-shared-utils.c
@@ -1196,7 +1196,7 @@ nm_g_object_set_property_boolean (GObject *object,
 
 gboolean
 nm_g_object_set_property_uint (GObject *object,
-                               const char   *property_name,
+                               const char *property_name,
                                guint value,
                                GError **error)
 {