dns: ensure that update_dns() always returns a GError on failure

Callers may expect this, so make sure we do it.

4 files changed, 63 insertions, 63 deletions

diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index f28b783e58..66108afe2b 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -152,27 +152,26 @@ nm_utils_ip6_address_clear_host_address (struct in6_addr *dst, const struct in6_
 
 
 int
-nm_spawn_process (const char *args)
+nm_spawn_process (const char *args, GError **error)
 {
+	GError *local = NULL;
 	gint num_args;
 	char **argv = NULL;
 	int status = -1;
-	GError *error = NULL;
 
 	g_return_val_if_fail (args != NULL, -1);
+	g_return_val_if_fail (!error || !*error, -1);
 
-	if (!g_shell_parse_argv (args, &num_args, &argv, &error)) {
-		nm_log_warn (LOGD_CORE, "could not parse arguments for '%s': %s", args, error->message);
-		g_error_free (error);
-		return -1;
+	if (g_shell_parse_argv (args, &num_args, &argv, &local)) {
+		g_spawn_sync ("/", argv, NULL, 0, NULL, NULL, NULL, NULL, &status, &local);
+		g_strfreev (argv);
 	}
 
-	if (!g_spawn_sync ("/", argv, NULL, 0, NULL, NULL, NULL, NULL, &status, &error)) {
-		nm_log_warn (LOGD_CORE, "could not spawn process '%s': %s", args, error->message);
-		g_error_free (error);
+	if (local) {
+		nm_log_warn (LOGD_CORE, "could not spawn process '%s': %s", args, local->message);
+		g_propagate_error (error, local);
 	}
 
-	g_strfreev (argv);
 	return status;
 }
 
diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h
index 11dc496b4f..b74fd1cbf3 100644
--- a/src/NetworkManagerUtils.h
+++ b/src/NetworkManagerUtils.h
@@ -49,7 +49,7 @@ nm_utils_ip6_route_metric_normalize (guint32 metric)
 	return metric ? metric : 1024 /*NM_PLATFORM_ROUTE_METRIC_DEFAULT_IP6*/;
 }
 
-int nm_spawn_process (const char *args);
+int nm_spawn_process (const char *args, GError **error);
 
 int nm_utils_modprobe (GError **error, const char *arg1, ...) G_GNUC_NULL_TERMINATED;
 
diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-dns-manager.c
index 05f2457824..7ebedb8f43 100644
--- a/src/dns-manager/nm-dns-manager.c
+++ b/src/dns-manager/nm-dns-manager.c
@@ -310,12 +310,19 @@ dispatch_netconfig (char **searches,
 
  again:
 
-	ret = waitpid (pid, NULL, 0);
-	if (ret < 0 && errno == EINTR)
-		goto again;
-	else if (ret < 0 && errno == ECHILD) {
-		/* When the netconfig exist, the errno is ECHILD, it should return TRUE */
-		return TRUE;
+	if (waitpid (pid, NULL, 0) < 0) {
+		if (errno == EINTR)
+			goto again;
+		else if (errno == ECHILD) {
+			/* child already exited */
+			ret = pid;
+		} else {
+			g_set_error_literal (error,
+			                     NM_MANAGER_ERROR,
+			                     NM_MANAGER_ERROR_FAILED,
+			                     "Error waiting for netconfig to exit: %s",
+			                     strerror (errno));
+		}
 	}
 
 	return ret > 0;
@@ -331,22 +338,12 @@ write_resolv_conf (FILE *f,
 {
 	char *searches_str = NULL;
 	char *nameservers_str = NULL;
-	int i;
 	gboolean retval = FALSE;
+	char *tmp_str;
 	GString *str;
-
-	if (fprintf (f, "%s","# Generated by NetworkManager\n") < 0) {
-		g_set_error (error,
-		             NM_MANAGER_ERROR,
-		             NM_MANAGER_ERROR_FAILED,
-		             "Could not write " _PATH_RESCONF ": %s\n",
-		             g_strerror (errno));
-		return FALSE;
-	}
+	int i;
 
 	if (searches) {
-		char *tmp_str;
-
 		tmp_str = g_strjoinv (" ", searches);
 		searches_str = g_strconcat ("search ", tmp_str, "\n", NULL);
 		g_free (tmp_str);
@@ -374,10 +371,17 @@ write_resolv_conf (FILE *f,
 
 	nameservers_str = g_string_free (str, FALSE);
 
-	if (fprintf (f, "%s%s",
+	if (fprintf (f, "# Generated by NetworkManager\n%s%s",
 	             searches_str ? searches_str : "",
-	             strlen (nameservers_str) ? nameservers_str : "") != -1)
+	             nameservers_str) > 0)
 		retval = TRUE;
+	else {
+		g_set_error (error,
+		             NM_MANAGER_ERROR,
+		             NM_MANAGER_ERROR_FAILED,
+		             "Could not write " _PATH_RESCONF ": %s\n",
+		             g_strerror (errno));
+	}
 
 	g_free (searches_str);
 	g_free (nameservers_str);
@@ -394,9 +398,15 @@ dispatch_resolvconf (char **searches,
 	char *cmd;
 	FILE *f;
 	gboolean retval = FALSE;
+	int errnosv, err;
 
-	if (! g_file_test (RESOLVCONF_PATH, G_FILE_TEST_IS_EXECUTABLE))
+	if (!g_file_test (RESOLVCONF_PATH, G_FILE_TEST_IS_EXECUTABLE)) {
+		g_set_error_literal (error,
+		                     NM_MANAGER_ERROR,
+		                     NM_MANAGER_ERROR_FAILED,
+		                     RESOLVCONF_PATH " is not executable");
 		return FALSE;
+	}
 
 	if (searches || nameservers) {
 		cmd = g_strconcat (RESOLVCONF_PATH, " -a ", "NetworkManager", NULL);
@@ -410,12 +420,21 @@ dispatch_resolvconf (char **searches,
 			             g_strerror (errno));
 		else {
 			retval = write_resolv_conf (f, searches, nameservers, error);
-			retval &= (pclose (f) == 0);
+			err = pclose (f);
+			if (err < 0) {
+				errnosv = errno;
+				g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errnosv),
+				             "Failed to close pipe to resolvconf: %d", errnosv);
+				retval = FALSE;
+			} else if (err > 0) {
+				nm_log_warn (LOGD_DNS, "resolvconf failed with status %d", err);
+				retval = FALSE;
+			}
 		}
 	} else {
 		cmd = g_strconcat (RESOLVCONF_PATH, " -d ", "NetworkManager", NULL);
 		nm_log_info (LOGD_DNS, "Removing DNS information from %s", RESOLVCONF_PATH);
-		if (nm_spawn_process (cmd) == 0)
+		if (nm_spawn_process (cmd, error) == 0)
 			retval = TRUE;
 	}
 
@@ -636,8 +655,7 @@ update_dns (NMDnsManager *self,
 	int num, i, len;
 	gboolean success = FALSE, caching = FALSE;
 
-	g_return_val_if_fail (error != NULL, FALSE);
-	g_return_val_if_fail (*error == NULL, FALSE);
+	g_return_val_if_fail (!error || !*error, FALSE);
 
 	priv = NM_DNS_MANAGER_GET_PRIVATE (self);
 
@@ -823,9 +841,7 @@ plugin_failed (NMDnsPlugin *plugin, gpointer user_data)
 
 	/* Disable caching until the next DNS update */
 	if (!update_dns (self, TRUE, &error)) {
-		nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s",
-		             error ? error->code : -1,
-		             error && error->message ? error->message : "(unknown)");
+		nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message);
 		g_clear_error (&error);
 	}
 }
@@ -862,9 +878,7 @@ nm_dns_manager_add_ip4_config (NMDnsManager *mgr,
 		priv->configs = g_slist_append (priv->configs, g_object_ref (config));
 
 	if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) {
-		nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s",
-		             error ? error->code : -1,
-		             error && error->message ? error->message : "(unknown)");
+		nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message);
 		g_clear_error (&error);
 	}
 
@@ -896,9 +910,7 @@ nm_dns_manager_remove_ip4_config (NMDnsManager *mgr, NMIP4Config *config)
 	g_object_unref (config);
 
 	if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) {
-		nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s",
-		             error ? error->code : -1,
-		             error && error->message ? error->message : "(unknown)");
+		nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message);
 		g_clear_error (&error);
 	}
 
@@ -939,9 +951,7 @@ nm_dns_manager_add_ip6_config (NMDnsManager *mgr,
 		priv->configs = g_slist_append (priv->configs, g_object_ref (config));
 
 	if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) {
-		nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s",
-		             error ? error->code : -1,
-		             error && error->message ? error->message : "(unknown)");
+		nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message);
 		g_clear_error (&error);
 	}
 
@@ -973,9 +983,7 @@ nm_dns_manager_remove_ip6_config (NMDnsManager *mgr, NMIP6Config *config)
 	g_object_unref (config);	
 
 	if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) {
-		nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s",
-		             error ? error->code : -1,
-		             error && error->message ? error->message : "(unknown)");
+		nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message);
 		g_clear_error (&error);
 	}
 
@@ -1017,9 +1025,7 @@ nm_dns_manager_set_hostname (NMDnsManager *mgr,
 	priv->hostname = g_strdup (filtered);
 
 	if (!priv->updates_queue && !update_dns (mgr, FALSE, &error)) {
-		nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s",
-		             error ? error->code : -1,
-		             error && error->message ? error->message : "(unknown)");
+		nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message);
 		g_clear_error (&error);
 	}
 }
@@ -1073,9 +1079,7 @@ nm_dns_manager_end_updates (NMDnsManager *mgr, const char *func)
 	/* Commit all the outstanding changes */
 	nm_log_dbg (LOGD_DNS, "(%s): committing DNS changes (%d)", func, priv->updates_queue);
 	if (!update_dns (mgr, FALSE, &error)) {
-		nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s",
-			         error ? error->code : -1,
-			         error && error->message ? error->message : "(unknown)");
+		nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message);
 		g_clear_error (&error);
 	}
 
@@ -1144,8 +1148,7 @@ config_changed_cb (NMConfig *config,
 
 	init_resolv_conf_mode (self);
 	if (!update_dns (self, TRUE, &error)) {
-		nm_log_warn (LOGD_DNS, "could not commit DNS changes: (%d) %s",
-		             error->code, error->message);
+		nm_log_warn (LOGD_DNS, "could not commit DNS changes: %s", error->message);
 		g_clear_error (&error);
 	}
 }
@@ -1181,9 +1184,7 @@ dispose (GObject *object)
 	 * DNS updates yet, there's no reason to touch resolv.conf on shutdown.
 	 */
 	if (priv->dns_touched && !update_dns (self, TRUE, &error)) {
-		nm_log_warn (LOGD_DNS, "could not commit DNS changes on shutdown: (%d) %s",
-		             error ? error->code : -1,
-		             error && error->message ? error->message : "(unknown)");
+		nm_log_warn (LOGD_DNS, "could not commit DNS changes on shutdown: %s", error->message);
 		g_clear_error (&error);
 		priv->dns_touched = FALSE;
 	}
diff --git a/src/dns-manager/nm-dns-unbound.c b/src/dns-manager/nm-dns-unbound.c
index 533290c11d..5723bf8229 100644
--- a/src/dns-manager/nm-dns-unbound.c
+++ b/src/dns-manager/nm-dns-unbound.c
@@ -42,7 +42,7 @@ update (NMDnsPlugin *plugin,
 	 * without calling custom scripts. The dnssec-trigger functionality
 	 * may be eventually merged into NetworkManager.
 	 */
-	return nm_spawn_process ("/usr/libexec/dnssec-trigger-script --async --update") == 0;
+	return nm_spawn_process ("/usr/libexec/dnssec-trigger-script --async --update", NULL) == 0;
 }
 
 static gboolean