Hopefully the first patch for error handling is something we can do in
the rest of the codebase too, because it's s 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