[PATCH] core: improve keyfile error handling

[Thomas Bechtold <thomasbechtold jpberlin de>]

  * better error messages
  * fix memory leak in parse_state_file ()
  * create intermediate parent directories as needed for state file
---
 src/main.c      |   30 +++++++++++++-----------------
 src/nm-config.c |   13 ++++++++++---
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/src/main.c b/src/main.c
index 59d7cd1..682cfcd 100644
--- a/src/main.c
+++ b/src/main.c
@@ -327,8 +327,8 @@ parse_state_file (const char *filename,
 
 	state_file = g_key_file_new ();
 	if (!state_file) {
-		g_set_error (error, 0, 0,
-		             "Not enough memory to load state file.");
+		g_set_error (error, NM_CONFIG_ERROR, NM_CONFIG_ERROR_NO_MEMORY,
+		             "Not enough memory to load state file %s.", filename);
 		return FALSE;
 	}
 
@@ -348,11 +348,12 @@ parse_state_file (const char *filename,
 			/* try to create the directory if it doesn't exist */
 			dirname = g_path_get_dirname (filename);
 			errno = 0;
-			if (mkdir (dirname, 0755) != 0) {
+			if (g_mkdir_with_parents (dirname, 0755) != 0) {
 				if (errno != EEXIST) {
 					g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_ACCES,
-					             "Error creating state directory %s: %d", dirname, errno);
+					             "Error creating state directory %s: %s", dirname, strerror(errno));
 					g_free (dirname);
+					g_clear_error (&tmp_error);
 					return FALSE;
 				}
 			}
@@ -368,13 +369,15 @@ parse_state_file (const char *filename,
 			if (data)
 				ret = g_file_set_contents (filename, data, len, error);
 			g_free (data);
+			g_clear_error (&tmp_error);
 
 			return ret;
 		} else {
-			g_set_error_literal (error, tmp_error->domain, tmp_error->code, tmp_error->message);
-			g_clear_error (&tmp_error);
+			/* the error is not "No such file or directory" - propagate the error */
+			g_propagate_error (error, tmp_error);
 		}
 
+		g_clear_error (&tmp_error);
 		/* Otherwise, file probably corrupt or inaccessible */
 		return FALSE;
 	}
@@ -384,34 +387,27 @@ parse_state_file (const char *filename,
 	 */
 	net = g_key_file_get_boolean (state_file, "main", "NetworkingEnabled", &tmp_error);
 	if (tmp_error)
-		g_set_error_literal (error, tmp_error->domain, tmp_error->code, tmp_error->message);
+		g_clear_error (&tmp_error);
 	else
 		*net_enabled = net;
-	g_clear_error (&tmp_error);
 
 	wifi = g_key_file_get_boolean (state_file, "main", "WirelessEnabled", &tmp_error);
 	if (tmp_error) {
-		g_clear_error (error);
-		g_set_error_literal (error, tmp_error->domain, tmp_error->code, tmp_error->message);
+		g_clear_error (&tmp_error);
 	} else
 		*wifi_enabled = wifi;
-	g_clear_error (&tmp_error);
 
 	wwan = g_key_file_get_boolean (state_file, "main", "WWANEnabled", &tmp_error);
 	if (tmp_error) {
-		g_clear_error (error);
-		g_set_error_literal (error, tmp_error->domain, tmp_error->code, tmp_error->message);
+		g_clear_error (&tmp_error);
 	} else
 		*wwan_enabled = wwan;
-	g_clear_error (&tmp_error);
 
 	wimax = g_key_file_get_boolean (state_file, "main", "WimaxEnabled", &tmp_error);
 	if (tmp_error) {
-		g_clear_error (error);
-		g_set_error_literal (error, tmp_error->domain, tmp_error->code, tmp_error->message);
+		g_clear_error (&tmp_error);
 	} else
 		*wimax_enabled = wimax;
-	g_clear_error (&tmp_error);
 
 	g_key_file_free (state_file);
 
diff --git a/src/nm-config.c b/src/nm-config.c
index f52f06f..7e582f4 100644
--- a/src/nm-config.c
+++ b/src/nm-config.c
@@ -130,14 +130,14 @@ fill_from_file (NMConfig *config,
 	gboolean success = FALSE;
 
 	if (g_file_test (path, G_FILE_TEST_EXISTS) == FALSE) {
-		g_set_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_NOT_FOUND, "file not found");
+		g_set_error (error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_NOT_FOUND, "file %s not found", path);
 		return FALSE;
 	}
 
 	kf = g_key_file_new ();
 	if (!kf) {
 		g_set_error (error, NM_CONFIG_ERROR, NM_CONFIG_ERROR_NO_MEMORY,
-		             "Not enough memory to load config file");
+		             "Not enough memory to load config file %s", path);
 		return FALSE;
 	}
 
@@ -219,9 +219,16 @@ nm_config_new (const char *cli_config_path,
 		         NM_DEFAULT_SYSTEM_CONF_FILE,
 		         local ? local->code : -1,
 		         (local && local->message) ? local->message : "unknown");
+		g_propagate_error (error, local);
+		nm_config_free (config);
+		return NULL;
+	}
+	else
+	{
+		/* ignore error if config file not found */
+		g_clear_error (&local);
 	}
 
-	g_propagate_error (error, local);
 	return config;
 }
 
-- 
1.7.5.4