From 991b8efca0d3136d8c63b202a9346572c8197da5 Mon Sep 17 00:00:00 2001
From: Dan Williams <dcbw@redhat.com>
Date: Thu, 26 Feb 2015 15:04:36 -0600
Subject: [PATCH 3/4] dns: ensure that update_dns() always returns a GError on
failure
Callers may expect this, so make sure we do it.
(cherry picked from commit 06f25a3ec7c07eac5785daeb99f648200abe3feb)
---
src/NetworkManagerUtils.c | 19 ++++----
src/NetworkManagerUtils.h | 2 +-
src/dns-manager/nm-dns-manager.c | 99 +++++++++++++++++++++-------------------
src/dns-manager/nm-dns-unbound.c | 2 +-
4 files changed, 62 insertions(+), 60 deletions(-)
diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index e6814e3..0a7a9b6 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -134,27 +134,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, nm_unblock_posix_signals, NULL, NULL, NULL, &status, &local);
+ g_strfreev (argv);
}
- if (!g_spawn_sync ("/", argv, NULL, 0, nm_unblock_posix_signals, 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 7be316e..22d665e 100644
--- a/src/NetworkManagerUtils.h
+++ b/src/NetworkManagerUtils.h
@@ -53,7 +53,7 @@ nm_utils_ip6_route_metric_normalize (guint32 metric)
return metric ? metric : 1024 /*NM_PLATFORM_ROUTE_METRIC_DEFAULT*/;
}
-int nm_spawn_process (const char *args);
+int nm_spawn_process (const char *args, GError **error);
/* macro to return strlen() of a compile time string. */
#define STRLEN(str) ( sizeof ("" str) - 1 )
diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-dns-manager.c
index e6984e3..e833ce4 100644
--- a/src/dns-manager/nm-dns-manager.c
+++ b/src/dns-manager/nm-dns-manager.c
@@ -323,12 +323,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;
@@ -344,22 +351,13 @@ 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_DNS_MANAGER_ERROR,
- NM_DNS_MANAGER_ERROR_SYSTEM,
- "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);
@@ -387,10 +385,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_DNS_MANAGER_ERROR,
+ NM_DNS_MANAGER_ERROR_SYSTEM,
+ "Could not write " _PATH_RESCONF ": %s\n",
+ g_strerror (errno));
+ }
g_free (searches_str);
g_free (nameservers_str);
@@ -407,9 +412,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);
@@ -423,12 +434,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;
}
@@ -612,8 +632,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);
@@ -799,9 +818,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);
}
}
@@ -838,9 +855,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);
}
@@ -872,9 +887,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);
}
@@ -915,9 +928,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);
}
@@ -949,9 +960,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);
}
@@ -994,9 +1003,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);
}
}
@@ -1050,9 +1057,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);
}
@@ -1152,9 +1157,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 137fd20..5520d38 100644
--- a/src/dns-manager/nm-dns-unbound.c
+++ b/src/dns-manager/nm-dns-unbound.c
@@ -40,7 +40,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
--
2.4.0