Hopefully the first patch for error handling is something we can do in
the rest of the codebase too, because it's soooo much better.
>From de1e3e809d06529afc0eb95ba38b374e1487924b Mon Sep 17 00:00:00 2001
From: Colin Walters <walt...@verbum.org>
Date: Wed, 6 Mar 2013 14:00:41 -0500
Subject: [PATCH 1/2] keyfile: Use "goto out" style error handling
Just code cleanup: This is much less error-prone than manual nesting,
and will mesh very well with future changes to use the libgsystem
cleanup macros.
---
src/settings/plugins/keyfile/plugin.c | 146 ++++++++++++++++++---------------
1 files changed, 81 insertions(+), 65 deletions(-)
diff --git a/src/settings/plugins/keyfile/plugin.c b/src/settings/plugins/keyfile/plugin.c
index e7e350b..a50f8c5 100644
--- a/src/settings/plugins/keyfile/plugin.c
+++ b/src/settings/plugins/keyfile/plugin.c
@@ -412,48 +412,53 @@ get_unmanaged_specs (NMSystemConfigInterface *config)
GKeyFile *key_file;
GSList *specs = NULL;
GError *error = NULL;
+ char *str;
if (!priv->conf_file)
return NULL;
key_file = g_key_file_new ();
- if (g_key_file_load_from_file (key_file, priv->conf_file, G_KEY_FILE_NONE, &error)) {
- char *str;
-
- str = g_key_file_get_value (key_file, "keyfile", "unmanaged-devices", NULL);
- if (str) {
- char **udis;
- int i;
-
- udis = g_strsplit (str, ";", -1);
- g_free (str);
-
- for (i = 0; udis[i] != NULL; i++) {
- /* Verify unmanaged specification and add it to the list */
- if (strlen (udis[i]) > 4 && !strncmp (udis[i], "mac:", 4) && ether_aton (udis[i] + 4)) {
- char *p = udis[i];
-
- /* To accept uppercase MACs in configuration file, we have to convert values to lowercase here.
- * Unmanaged MACs in specs are always in lowercase. */
- while (*p) {
- *p = g_ascii_tolower (*p);
- p++;
- }
- specs = g_slist_append (specs, udis[i]);
- } else {
- g_warning ("Error in file '%s': invalid unmanaged-devices entry: '%s'", priv->conf_file, udis[i]);
- g_free (udis[i]);
+ if (!g_key_file_load_from_file (key_file, priv->conf_file, G_KEY_FILE_NONE, &error)) {
+ g_prefix_error (&error, "Error parsing file '%s': ", priv->conf_file);
+ goto out;
+ }
+
+ str = g_key_file_get_value (key_file, "keyfile", "unmanaged-devices", NULL);
+ if (str) {
+ char **udis;
+ int i;
+
+ udis = g_strsplit (str, ";", -1);
+ g_free (str);
+
+ for (i = 0; udis[i] != NULL; i++) {
+ /* Verify unmanaged specification and add it to the list */
+ if (strlen (udis[i]) > 4 && !strncmp (udis[i], "mac:", 4) && ether_aton (udis[i] + 4)) {
+ char *p = udis[i];
+
+ /* To accept uppercase MACs in configuration file, we have to convert values to lowercase here.
+ * Unmanaged MACs in specs are always in lowercase. */
+ while (*p) {
+ *p = g_ascii_tolower (*p);
+ p++;
}
+ specs = g_slist_append (specs, udis[i]);
+ } else {
+ g_warning ("Error in file '%s': invalid unmanaged-devices entry: '%s'", priv->conf_file, udis[i]);
+ g_free (udis[i]);
}
-
- g_free (udis); /* Yes, g_free, not g_strfreev because we need the strings in the list */
}
- } else {
- g_warning ("Error parsing file '%s': %s", priv->conf_file, error->message);
- g_error_free (error);
+
+ g_free (udis); /* Yes, g_free, not g_strfreev because we need the strings in the list */
}
- g_key_file_free (key_file);
+ out:
+ if (error) {
+ g_warning ("%s", error->message);
+ g_error_free (error);
+ }
+ if (key_file)
+ g_key_file_free (key_file);
return specs;
}
@@ -470,14 +475,20 @@ plugin_get_hostname (SCPluginKeyfile *plugin)
return NULL;
key_file = g_key_file_new ();
- if (g_key_file_load_from_file (key_file, priv->conf_file, G_KEY_FILE_NONE, &error))
- hostname = g_key_file_get_value (key_file, "keyfile", "hostname", NULL);
- else {
- g_warning ("Error parsing file '%s': %s", priv->conf_file, error->message);
- g_error_free (error);
+ if (!g_key_file_load_from_file (key_file, priv->conf_file, G_KEY_FILE_NONE, &error)) {
+ g_prefix_error (&error, "Error parsing file '%s': ", priv->conf_file);
+ goto out;
}
- g_key_file_free (key_file);
+ hostname = g_key_file_get_value (key_file, "keyfile", "hostname", NULL);
+
+ out:
+ if (error) {
+ g_warning ("%s", error->message);
+ g_error_free (error);
+ }
+ if (key_file)
+ g_key_file_free (key_file);
return hostname;
}
@@ -485,45 +496,50 @@ plugin_get_hostname (SCPluginKeyfile *plugin)
static gboolean
plugin_set_hostname (SCPluginKeyfile *plugin, const char *hostname)
{
+ gboolean ret = FALSE;
SCPluginKeyfilePrivate *priv = SC_PLUGIN_KEYFILE_GET_PRIVATE (plugin);
- GKeyFile *key_file;
+ GKeyFile *key_file = NULL;
GError *error = NULL;
- gboolean result = FALSE;
+ char *data = NULL;
+ gsize len;
if (!priv->conf_file) {
- g_warning ("Error saving hostname: no config file");
- return FALSE;
+ g_set_error (&error, G_IO_ERROR, G_IO_ERROR_FAILED,
+ "Error saving hostname: no config file");
+ goto out;
}
+
+ g_free (priv->hostname);
+ priv->hostname = g_strdup (hostname);
key_file = g_key_file_new ();
- if (g_key_file_load_from_file (key_file, priv->conf_file, G_KEY_FILE_NONE, &error)) {
- char *data;
- gsize len;
-
- g_key_file_set_string (key_file, "keyfile", "hostname", hostname);
-
- data = g_key_file_to_data (key_file, &len, &error);
- if (data) {
- g_file_set_contents (priv->conf_file, data, len, &error);
- g_free (data);
+ if (!g_key_file_load_from_file (key_file, priv->conf_file, G_KEY_FILE_NONE, &error)) {
+ g_prefix_error (&error, "Error parsing file '%s': ", priv->conf_file);
+ goto out;
+ }
- g_free (priv->hostname);
- priv->hostname = g_strdup (hostname);
- result = TRUE;
- }
+ g_key_file_set_string (key_file, "keyfile", "hostname", hostname);
+
+ data = g_key_file_to_data (key_file, &len, &error);
+ if (!data)
+ goto out;
- if (error) {
- g_warning ("Error saving hostname: %s", error->message);
- g_error_free (error);
- }
- } else {
- g_warning ("Error parsing file '%s': %s", priv->conf_file, error->message);
+ if (!g_file_set_contents (priv->conf_file, data, len, &error)) {
+ g_prefix_error (&error, "Error saving hostname: ");
+ goto out;
+ }
+
+ ret = TRUE;
+ out:
+ if (error) {
+ g_warning ("%s", error->message);
g_error_free (error);
}
+ g_free (data);
+ if (key_file)
+ g_key_file_free (key_file);
- g_key_file_free (key_file);
-
- return result;
+ return ret;
}
/* GObject */
--
1.7.1
>From 2713100c725a8d47fc37d54e91f2a19098b7dbd3 Mon Sep 17 00:00:00 2001
From: Colin Walters <walt...@verbum.org>
Date: Wed, 6 Mar 2013 14:13:26 -0500
Subject: [PATCH 2/2] keyfile: Handle /etc/NetworkManager/NetworkManager.conf not existing
Since in gnome-ostree we're trying to move towards an empty /etc.
We'll create it on demand if necessary.
---
src/settings/plugins/keyfile/plugin.c | 35 ++++++++++++++++++++++++--------
1 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/src/settings/plugins/keyfile/plugin.c b/src/settings/plugins/keyfile/plugin.c
index a50f8c5..4f177bf 100644
--- a/src/settings/plugins/keyfile/plugin.c
+++ b/src/settings/plugins/keyfile/plugin.c
@@ -405,6 +405,29 @@ add_connection (NMSystemConfigInterface *config,
return added;
}
+static gboolean
+parse_key_file_allow_none (SCPluginKeyfilePrivate *priv,
+ GKeyFile *key_file,
+ GError **error)
+{
+ gboolean ret = FALSE;
+ GError *local_error = NULL;
+
+ if (!g_key_file_load_from_file (key_file, priv->conf_file, G_KEY_FILE_NONE, &local_error)) {
+ if (g_error_matches (local_error, G_FILE_ERROR, G_FILE_ERROR_NOENT)) {
+ g_clear_error (&local_error);
+ } else {
+ g_propagate_prefixed_error (error, local_error,
+ "Error parsing file '%s': %s", priv->conf_file);
+ goto out;
+ }
+ }
+
+ ret = TRUE;
+ out:
+ return ret;
+}
+
static GSList *
get_unmanaged_specs (NMSystemConfigInterface *config)
{
@@ -418,10 +441,8 @@ get_unmanaged_specs (NMSystemConfigInterface *config)
return NULL;
key_file = g_key_file_new ();
- if (!g_key_file_load_from_file (key_file, priv->conf_file, G_KEY_FILE_NONE, &error)) {
- g_prefix_error (&error, "Error parsing file '%s': ", priv->conf_file);
+ if (!parse_key_file_allow_none (priv, key_file, &error))
goto out;
- }
str = g_key_file_get_value (key_file, "keyfile", "unmanaged-devices", NULL);
if (str) {
@@ -475,10 +496,8 @@ plugin_get_hostname (SCPluginKeyfile *plugin)
return NULL;
key_file = g_key_file_new ();
- if (!g_key_file_load_from_file (key_file, priv->conf_file, G_KEY_FILE_NONE, &error)) {
- g_prefix_error (&error, "Error parsing file '%s': ", priv->conf_file);
+ if (!parse_key_file_allow_none (priv, key_file, &error))
goto out;
- }
hostname = g_key_file_get_value (key_file, "keyfile", "hostname", NULL);
@@ -513,10 +532,8 @@ plugin_set_hostname (SCPluginKeyfile *plugin, const char *hostname)
priv->hostname = g_strdup (hostname);
key_file = g_key_file_new ();
- if (!g_key_file_load_from_file (key_file, priv->conf_file, G_KEY_FILE_NONE, &error)) {
- g_prefix_error (&error, "Error parsing file '%s': ", priv->conf_file);
+ if (!parse_key_file_allow_none (priv, key_file, &error))
goto out;
- }
g_key_file_set_string (key_file, "keyfile", "hostname", hostname);
--
1.7.1
_______________________________________________
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list
