From fd9f1b7cdd86286b2239e4f771e759c8a5b82aad Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 25 May 2018 15:58:09 +0200 Subject: [PATCH 1/8] dhcp: fix leaking error in dhclient_start() Fixes: 1b1b4bd91c29425c25e8e979cd77b7a67deb9bf5 --- src/dhcp/nm-dhcp-dhclient.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/dhcp/nm-dhcp-dhclient.c b/src/dhcp/nm-dhcp-dhclient.c index 5230ed4add..1c0b4bb3ac 100644 --- a/src/dhcp/nm-dhcp-dhclient.c +++ b/src/dhcp/nm-dhcp-dhclient.c @@ -387,6 +387,7 @@ dhclient_start (NMDhcpClient *client, if (!success) { _LOGW ("failed to save DUID to %s: %s", priv->lease_file, error->message); g_free (pid_file); + g_clear_error (&error); return FALSE; } } From f11bb3d93d3557847018871cacf918ea24664ddf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 25 May 2018 15:38:21 +0200 Subject: [PATCH 2/8] shared: minor cleanup of nm_utils_get_start_time_for_pid() --- libnm-core/tests/test-general.c | 18 ++++++++++++++++++ shared/nm-utils/nm-shared-utils.c | 11 ++++------- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 0f04aee99b..c853754da0 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -6873,6 +6873,22 @@ test_nm_set_out (void) /*****************************************************************************/ +static void +test_get_start_time_for_pid (void) +{ + guint64 x_start_time; + char x_state; + pid_t x_ppid; + + x_start_time = nm_utils_get_start_time_for_pid (getpid (), &x_state, &x_ppid); + + g_assert (x_start_time > 0); + g_assert (x_ppid == getppid ()); + g_assert (!NM_IN_SET (x_state, '\0', ' ')); +} + +/*****************************************************************************/ + NMTST_DEFINE (); int main (int argc, char **argv) @@ -7026,6 +7042,8 @@ int main (int argc, char **argv) g_test_add_func ("/core/general/route_attributes/parse", test_route_attributes_parse); g_test_add_func ("/core/general/route_attributes/format", test_route_attributes_format); + g_test_add_func ("/core/general/get_start_time_for_pid", test_get_start_time_for_pid); + return g_test_run (); } diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c index 96e5da350b..d0019c1174 100644 --- a/shared/nm-utils/nm-shared-utils.c +++ b/shared/nm-utils/nm-shared-utils.c @@ -1371,8 +1371,7 @@ nm_utils_get_start_time_for_pid (pid_t pid, char *out_state, pid_t *out_ppid) char filename[256]; gs_free gchar *contents = NULL; size_t length; - gs_strfreev gchar **tokens = NULL; - guint num_tokens; + gs_free const char **tokens = NULL; gchar *p; char state = ' '; gint64 ppid = 0; @@ -1392,7 +1391,7 @@ nm_utils_get_start_time_for_pid (pid_t pid, char *out_state, pid_t *out_ppid) * processes trying to fool us */ p = strrchr (contents, ')'); - if (p == NULL) + if (!p) goto fail; p += 2; /* skip ') ' */ if (p - contents >= (int) length) @@ -1400,11 +1399,9 @@ nm_utils_get_start_time_for_pid (pid_t pid, char *out_state, pid_t *out_ppid) state = p[0]; - tokens = g_strsplit (p, " ", 0); + tokens = nm_utils_strsplit_set (p, " "); - num_tokens = g_strv_length (tokens); - - if (num_tokens < 20) + if (NM_PTRARRAY_LEN (tokens) < 20) goto fail; if (out_ppid) { From 1c5e7902775b895a9c57abfeec5a63da98d628e9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 25 May 2018 11:10:13 +0200 Subject: [PATCH 3/8] core: let NM_IS_IP_CONFIG() check for expected addr-family Still unused... --- src/dns/nm-dns-manager.c | 6 +++--- src/nm-ip4-config.h | 12 +++++++++--- src/nm-policy.c | 4 ++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c index 669371378d..f5500ee794 100644 --- a/src/dns/nm-dns-manager.c +++ b/src/dns/nm-dns-manager.c @@ -232,7 +232,7 @@ _ASSERT_ip_config_data (const NMDnsIPConfigData *ip_data) { nm_assert (ip_data); _ASSERT_config_data (ip_data->data); - nm_assert (NM_IS_IP_CONFIG (ip_data->ip_config)); + nm_assert (NM_IS_IP_CONFIG (ip_data->ip_config, AF_UNSPEC)); nm_assert (c_list_contains (&ip_data->data->data_lst_head, &ip_data->data_lst)); nm_assert (ip_data->data->ifindex == nm_ip_config_get_ifindex (ip_data->ip_config)); } @@ -245,7 +245,7 @@ _ip_config_data_new (NMDnsConfigData *data, NMDnsIPConfigData *ip_data; _ASSERT_config_data (data); - nm_assert (NM_IS_IP_CONFIG (ip_config)); + nm_assert (NM_IS_IP_CONFIG (ip_config, AF_UNSPEC)); nm_assert (ip_config_type != NM_DNS_IP_CONFIG_TYPE_REMOVED); ip_data = g_slice_new0 (NMDnsIPConfigData); @@ -1562,7 +1562,7 @@ nm_dns_manager_set_ip_config (NMDnsManager *self, NMDnsIPConfigData **p_best; g_return_val_if_fail (NM_IS_DNS_MANAGER (self), FALSE); - g_return_val_if_fail (NM_IS_IP_CONFIG (ip_config), FALSE); + g_return_val_if_fail (NM_IS_IP_CONFIG (ip_config, AF_UNSPEC), FALSE); ifindex = nm_ip_config_get_ifindex (ip_config); g_return_val_if_fail (ifindex > 0, FALSE); diff --git a/src/nm-ip4-config.h b/src/nm-ip4-config.h index e2f40fd345..78aca14e43 100644 --- a/src/nm-ip4-config.h +++ b/src/nm-ip4-config.h @@ -295,9 +295,15 @@ gboolean _nm_ip_config_check_and_add_domain (GPtrArray *array, const char *domai #include "nm-ip6-config.h" static inline gboolean -NM_IS_IP_CONFIG (gconstpointer config) +NM_IS_IP_CONFIG (gconstpointer config, int addr_family) { - return NM_IS_IP4_CONFIG (config) || NM_IS_IP6_CONFIG (config); + if (addr_family == AF_UNSPEC) + return NM_IS_IP4_CONFIG (config) || NM_IS_IP6_CONFIG (config); + if (addr_family == AF_INET) + return NM_IS_IP4_CONFIG (config); + if (addr_family == AF_INET6) + return NM_IS_IP6_CONFIG (config); + g_return_val_if_reached (FALSE); } #if _NM_CC_SUPPORT_GENERIC @@ -334,7 +340,7 @@ NM_IS_IP_CONFIG (gconstpointer config) NMIP6Config * : (NM_IS_IP6_CONFIG (_config))); \ }) #else -#define _NM_IS_IP_CONFIG(typeexpr, config) NM_IS_IP_CONFIG(config) +#define _NM_IS_IP_CONFIG(typeexpr, config) NM_IS_IP_CONFIG(config, AF_UNSPEC) #endif #define NM_IP_CONFIG_CAST(config) \ diff --git a/src/nm-policy.c b/src/nm-policy.c index 61cc001f0b..a90dd97bf6 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1910,8 +1910,8 @@ device_ip_config_changed (NMDevice *device, int addr_family; nm_assert (new_config || old_config); - nm_assert (!new_config || NM_IS_IP_CONFIG (new_config)); - nm_assert (!old_config || NM_IS_IP_CONFIG (old_config)); + nm_assert (!new_config || NM_IS_IP_CONFIG (new_config, AF_UNSPEC)); + nm_assert (!old_config || NM_IS_IP_CONFIG (old_config, AF_UNSPEC)); if (new_config) { addr_family = nm_ip_config_get_addr_family (new_config); From 97dc1ec1e4026e9bd41db73e6ce318097e192612 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 25 May 2018 10:55:50 +0200 Subject: [PATCH 4/8] dhcp: minor cleanup initializing name of leasefile for NMDhcpDhclint --- src/dhcp/nm-dhcp-dhclient.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/dhcp/nm-dhcp-dhclient.c b/src/dhcp/nm-dhcp-dhclient.c index 1c0b4bb3ac..0dfc448810 100644 --- a/src/dhcp/nm-dhcp-dhclient.c +++ b/src/dhcp/nm-dhcp-dhclient.c @@ -605,7 +605,7 @@ get_duid (NMDhcpClient *client) g_free (leasefile); } - if (!duid && priv->def_leasefile) { + if (!duid) { /* Otherwise read the default machine-wide DUID */ _LOGD ("looking for default DUID in '%s'", priv->def_leasefile); duid = nm_dhcp_dhclient_read_duid (priv->def_leasefile, &error); @@ -623,31 +623,25 @@ get_duid (NMDhcpClient *client) /*****************************************************************************/ -static const char *def_leasefiles[] = { - SYSCONFDIR "/dhclient6.leases", - LOCALSTATEDIR "/lib/dhcp/dhclient6.leases", - LOCALSTATEDIR "/lib/dhclient/dhclient6.leases", - NULL -}; - static void nm_dhcp_dhclient_init (NMDhcpDhclient *self) { + static const char *const FILES[] = { + SYSCONFDIR "/dhclient6.leases", /* default */ + LOCALSTATEDIR "/lib/dhcp/dhclient6.leases", + LOCALSTATEDIR "/lib/dhclient/dhclient6.leases", + }; NMDhcpDhclientPrivate *priv = NM_DHCP_DHCLIENT_GET_PRIVATE (self); - const char **iter = &def_leasefiles[0]; + int i; - while (iter && *iter) { - if (g_file_test (*iter, G_FILE_TEST_EXISTS)) { - priv->def_leasefile = *iter; + priv->def_leasefile = FILES[0]; + for (i = 0; i < G_N_ELEMENTS (FILES); i++) { + if (g_file_test (FILES[i], G_FILE_TEST_EXISTS)) { + priv->def_leasefile = FILES[i]; break; } - iter++; } - /* Fallback option */ - if (!priv->def_leasefile) - priv->def_leasefile = SYSCONFDIR "/dhclient6.leases"; - priv->dhcp_listener = g_object_ref (nm_dhcp_listener_get ()); g_signal_connect (priv->dhcp_listener, NM_DHCP_LISTENER_EVENT, From ef8782127f2a91ee75cbdeeba257c489eb5bec9f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 25 May 2018 11:03:48 +0200 Subject: [PATCH 5/8] dhcp: use cleanup attribute in nm_dhcp_client_handle_event() and use NMIPConfig type. --- src/dhcp/nm-dhcp-client.c | 41 ++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c index 7d42a786ec..f1a62f2474 100644 --- a/src/dhcp/nm-dhcp-client.c +++ b/src/dhcp/nm-dhcp-client.c @@ -770,8 +770,8 @@ nm_dhcp_client_handle_event (gpointer unused, NMDhcpClientPrivate *priv; guint32 old_state; guint32 new_state; - GHashTable *str_options = NULL; - GObject *ip_config = NULL; + gs_unref_hashtable GHashTable *str_options = NULL; + gs_unref_object NMIPConfig *ip_config = NULL; NMPlatformIP6Address prefix = { 0, }; g_return_val_if_fail (NM_IS_DHCP_CLIENT (self), FALSE); @@ -815,24 +815,24 @@ nm_dhcp_client_handle_event (gpointer unused, } /* Create the IP config */ - g_warn_if_fail (g_hash_table_size (str_options)); - if (g_hash_table_size (str_options)) { + if (g_hash_table_size (str_options) > 0) { if (priv->addr_family == AF_INET) { - ip_config = (GObject *) nm_dhcp_utils_ip4_config_from_options (nm_dhcp_client_get_multi_idx (self), - priv->ifindex, - priv->iface, - str_options, - priv->route_table, - priv->route_metric); + ip_config = NM_IP_CONFIG_CAST (nm_dhcp_utils_ip4_config_from_options (nm_dhcp_client_get_multi_idx (self), + priv->ifindex, + priv->iface, + str_options, + priv->route_table, + priv->route_metric)); } else { prefix = nm_dhcp_utils_ip6_prefix_from_options (str_options); - ip_config = (GObject *) nm_dhcp_utils_ip6_config_from_options (nm_dhcp_client_get_multi_idx (self), - priv->ifindex, - priv->iface, - str_options, - priv->info_only); + ip_config = NM_IP_CONFIG_CAST (nm_dhcp_utils_ip6_config_from_options (nm_dhcp_client_get_multi_idx (self), + priv->ifindex, + priv->iface, + str_options, + priv->info_only)); } - } + } else + g_warn_if_reached (); } if (!IN6_IS_ADDR_UNSPECIFIED (&prefix.address)) { @@ -844,19 +844,16 @@ nm_dhcp_client_handle_event (gpointer unused, &prefix); } else { /* Fail if no valid IP config was received */ - if (new_state == NM_DHCP_STATE_BOUND && ip_config == NULL) { + if ( new_state == NM_DHCP_STATE_BOUND + && !ip_config) { _LOGW ("client bound but IP config not received"); new_state = NM_DHCP_STATE_FAIL; g_clear_pointer (&str_options, g_hash_table_unref); } - nm_dhcp_client_set_state (self, new_state, ip_config, str_options); + nm_dhcp_client_set_state (self, new_state, G_OBJECT (ip_config), str_options); } - if (str_options) - g_hash_table_destroy (str_options); - g_clear_object (&ip_config); - return TRUE; } From 181cf5c709a35af51c6d49ad0455ec54d64f94a2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 25 May 2018 11:15:43 +0200 Subject: [PATCH 6/8] dhcp: use NMIPConfig type for argument of nm_dhcp_client_set_state() --- src/dhcp/nm-dhcp-client.c | 21 ++++++++++----------- src/dhcp/nm-dhcp-client.h | 2 +- src/dhcp/nm-dhcp-systemd.c | 4 ++-- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c index f1a62f2474..60901692fb 100644 --- a/src/dhcp/nm-dhcp-client.c +++ b/src/dhcp/nm-dhcp-client.c @@ -359,26 +359,25 @@ stop (NMDhcpClient *self, gboolean release, GBytes *duid) void nm_dhcp_client_set_state (NMDhcpClient *self, NMDhcpState new_state, - GObject *ip_config, + NMIPConfig *ip_config, GHashTable *options) { NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE (self); gs_free char *event_id = NULL; + if (new_state == NM_DHCP_STATE_BOUND) { + g_return_if_fail (NM_IS_IP_CONFIG (ip_config, priv->addr_family)); + g_return_if_fail (options); + } else { + g_return_if_fail (!ip_config); + g_return_if_fail (!options); + } + if (new_state >= NM_DHCP_STATE_BOUND) timeout_cleanup (self); if (new_state >= NM_DHCP_STATE_TIMEOUT) watch_cleanup (self); - if (new_state == NM_DHCP_STATE_BOUND) { - g_assert ( (priv->addr_family == AF_INET && NM_IS_IP4_CONFIG (ip_config)) - || (priv->addr_family == AF_INET6 && NM_IS_IP6_CONFIG (ip_config))); - g_assert (options); - } else { - g_assert (ip_config == NULL); - g_assert (options == NULL); - } - /* The client may send same-state transitions for RENEW/REBIND events and * the lease may have changed, so handle same-state transitions for the * BOUND state. Ignore same-state transitions for other events since @@ -851,7 +850,7 @@ nm_dhcp_client_handle_event (gpointer unused, g_clear_pointer (&str_options, g_hash_table_unref); } - nm_dhcp_client_set_state (self, new_state, G_OBJECT (ip_config), str_options); + nm_dhcp_client_set_state (self, new_state, ip_config, str_options); } return TRUE; diff --git a/src/dhcp/nm-dhcp-client.h b/src/dhcp/nm-dhcp-client.h index f3c34d3dcf..111b063bd5 100644 --- a/src/dhcp/nm-dhcp-client.h +++ b/src/dhcp/nm-dhcp-client.h @@ -168,7 +168,7 @@ void nm_dhcp_client_watch_child (NMDhcpClient *self, pid_t pid); void nm_dhcp_client_set_state (NMDhcpClient *self, NMDhcpState new_state, - GObject *ip_config, /* NMIP4Config or NMIP6Config */ + NMIPConfig *ip_config, GHashTable *options); /* str:str hash */ gboolean nm_dhcp_client_handle_event (gpointer unused, diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index 8cd80abe82..2d0202bb70 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -513,7 +513,7 @@ bound4_handle (NMDhcpSystemd *self) nm_dhcp_client_set_state (NM_DHCP_CLIENT (self), NM_DHCP_STATE_BOUND, - G_OBJECT (ip4_config), + NM_IP_CONFIG_CAST (ip4_config), options); } else { _LOGW ("%s", error->message); @@ -822,7 +822,7 @@ bound6_handle (NMDhcpSystemd *self) if (ip6_config) { nm_dhcp_client_set_state (NM_DHCP_CLIENT (self), NM_DHCP_STATE_BOUND, - G_OBJECT (ip6_config), + NM_IP_CONFIG_CAST (ip6_config), options); } else { _LOGW ("%s", error->message); From 5bd3f7bd67987de7e1194e5e3caa063d106c7e07 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 25 May 2018 15:41:39 +0200 Subject: [PATCH 7/8] dhcp: cache errno in nm_dhcp_client_stop_existing() before using it --- src/dhcp/nm-dhcp-client.c | 6 ++++-- src/dhcp/nm-dhcp-dhclient.c | 10 ++++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c index 60901692fb..ba517606ed 100644 --- a/src/dhcp/nm-dhcp-client.c +++ b/src/dhcp/nm-dhcp-client.c @@ -659,8 +659,10 @@ nm_dhcp_client_stop_existing (const char *pid_file, const char *binary_name) out: if (remove (pid_file) == -1) { - nm_log_dbg (LOGD_DHCP, "dhcp: could not remove pid file \"%s\": %d (%s)", - pid_file, errno, g_strerror (errno)); + int errsv = errno; + + nm_log_dbg (LOGD_DHCP, "dhcp: could not remove pid file \"%s\": %s (%d)", + pid_file, g_strerror (errsv), errsv); } } diff --git a/src/dhcp/nm-dhcp-dhclient.c b/src/dhcp/nm-dhcp-dhclient.c index 0dfc448810..38ad205709 100644 --- a/src/dhcp/nm-dhcp-dhclient.c +++ b/src/dhcp/nm-dhcp-dhclient.c @@ -545,10 +545,12 @@ stop (NMDhcpClient *client, gboolean release, GBytes *duid) if (remove (priv->conf_file) == -1) _LOGD ("could not remove dhcp config file \"%s\": %d (%s)", priv->conf_file, errno, g_strerror (errno)); if (priv->pid_file) { - if (remove (priv->pid_file) == -1) - _LOGD ("could not remove dhcp pid file \"%s\": %d (%s)", priv->pid_file, errno, g_strerror (errno)); - g_free (priv->pid_file); - priv->pid_file = NULL; + if (remove (priv->pid_file) == -1) { + int errsv = errno; + + _LOGD ("could not remove dhcp pid file \"%s\": %s (%d)", priv->pid_file, g_strerror (errsv), errsv); + } + nm_clear_g_free (&priv->pid_file); } if (release) { From 6d41864c8bbc7a00d42d0eac4b97719a9bd1c21d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 25 May 2018 15:59:37 +0200 Subject: [PATCH 8/8] dhcp: refactor dhclient_start() to use cleanup attribute --- src/dhcp/nm-dhcp-dhclient.c | 88 ++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/src/dhcp/nm-dhcp-dhclient.c b/src/dhcp/nm-dhcp-dhclient.c index 38ad205709..93306ddd5f 100644 --- a/src/dhcp/nm-dhcp-dhclient.c +++ b/src/dhcp/nm-dhcp-dhclient.c @@ -127,11 +127,13 @@ get_dhclient_leasefile (int addr_family, _addr_family_to_path_part (addr_family), uuid, iface); - if (out_preferred_path) - *out_preferred_path = g_strdup (path); - if (g_file_test (path, G_FILE_TEST_EXISTS)) + if (g_file_test (path, G_FILE_TEST_EXISTS)) { + NM_SET_OUT (out_preferred_path, g_strdup (path)); return path; + } + + NM_SET_OUT (out_preferred_path, g_steal_pointer (&path)); /* If the leasefile we're looking for doesn't exist yet in the new location * (eg, /var/lib/NetworkManager) then look in old locations to maintain @@ -317,20 +319,23 @@ dhclient_start (NMDhcpClient *client, { NMDhcpDhclient *self = NM_DHCP_DHCLIENT (client); NMDhcpDhclientPrivate *priv = NM_DHCP_DHCLIENT_GET_PRIVATE (self); - GPtrArray *argv = NULL; + gs_unref_ptrarray GPtrArray *argv = NULL; pid_t pid; GError *error = NULL; - const char *iface, *uuid, *system_bus_address, *dhclient_path = NULL; - char *binary_name, *cmd_str, *pid_file = NULL, *system_bus_address_env = NULL; - int addr_family; - gboolean success; - char *escaped, *preferred_leasefile_path = NULL; + const char *iface; + const char *uuid; + const char *system_bus_address; + const char *dhclient_path; + char *binary_name; + gs_free char *cmd_str = NULL; + gs_free char *pid_file = NULL; + gs_free char *system_bus_address_env = NULL; + gs_free char *preferred_leasefile_path = NULL; + const int addr_family = nm_dhcp_client_get_addr_family (client); - g_return_val_if_fail (priv->pid_file == NULL, FALSE); + g_return_val_if_fail (!priv->pid_file, FALSE); - iface = nm_dhcp_client_get_iface (client); - uuid = nm_dhcp_client_get_uuid (client); - addr_family = nm_dhcp_client_get_addr_family (client); + NM_SET_OUT (out_pid, 0); dhclient_path = nm_dhcp_dhclient_get_path (); if (!dhclient_path) { @@ -338,6 +343,9 @@ dhclient_start (NMDhcpClient *client, return FALSE; } + iface = nm_dhcp_client_get_iface (client); + uuid = nm_dhcp_client_get_uuid (client); + pid_file = g_strdup_printf (RUNSTATEDIR "/dhclient%s-%s.pid", _addr_family_to_path_part (addr_family), iface); @@ -349,18 +357,18 @@ dhclient_start (NMDhcpClient *client, if (release) { /* release doesn't use the pidfile after killing an old client */ - g_free (pid_file); - pid_file = NULL; + nm_clear_g_free (&pid_file); } g_free (priv->lease_file); priv->lease_file = get_dhclient_leasefile (addr_family, iface, uuid, &preferred_leasefile_path); + nm_assert (preferred_leasefile_path); if (!priv->lease_file) { /* No existing leasefile, dhclient will create one at the preferred path */ - priv->lease_file = g_strdup (preferred_leasefile_path); - } else if (g_strcmp0 (priv->lease_file, preferred_leasefile_path) != 0) { - GFile *src = g_file_new_for_path (priv->lease_file); - GFile *dst = g_file_new_for_path (preferred_leasefile_path); + priv->lease_file = g_steal_pointer (&preferred_leasefile_path); + } else if (!nm_streq0 (priv->lease_file, preferred_leasefile_path)) { + gs_unref_object GFile *src = g_file_new_for_path (priv->lease_file); + gs_unref_object GFile *dst = g_file_new_for_path (preferred_leasefile_path); /* Try to copy the existing leasefile to the preferred location */ if (g_file_copy (src, dst, G_FILE_COPY_OVERWRITE, NULL, NULL, NULL, &error)) { @@ -374,19 +382,15 @@ dhclient_start (NMDhcpClient *client, error->message); g_clear_error (&error); } - g_object_unref (src); - g_object_unref (dst); } - g_free (preferred_leasefile_path); /* Save the DUID to the leasefile dhclient will actually use */ if (addr_family == AF_INET6) { + gs_free char *escaped = NULL; + escaped = nm_dhcp_dhclient_escape_duid (duid); - success = nm_dhcp_dhclient_save_duid (priv->lease_file, escaped, &error); - g_free (escaped); - if (!success) { + if (!nm_dhcp_dhclient_save_duid (priv->lease_file, escaped, &error)) { _LOGW ("failed to save DUID to %s: %s", priv->lease_file, error->message); - g_free (pid_file); g_clear_error (&error); return FALSE; } @@ -442,30 +446,26 @@ dhclient_start (NMDhcpClient *client, g_ptr_array_add (argv, (gpointer) iface); g_ptr_array_add (argv, NULL); - cmd_str = g_strjoinv (" ", (gchar **) argv->pdata); - _LOGD ("running: %s", cmd_str); - g_free (cmd_str); + _LOGD ("running: %s", + (cmd_str = g_strjoinv (" ", (gchar **) argv->pdata))); - if (g_spawn_async (NULL, (char **) argv->pdata, NULL, - G_SPAWN_DO_NOT_REAP_CHILD | G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL, - nm_utils_setpgid, NULL, &pid, &error)) { - g_assert (pid > 0); - _LOGI ("dhclient started with pid %d", pid); - if (release == FALSE) - nm_dhcp_client_watch_child (client, pid); - priv->pid_file = pid_file; - } else { + if (!g_spawn_async (NULL, (char **) argv->pdata, NULL, + G_SPAWN_DO_NOT_REAP_CHILD | G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL, + nm_utils_setpgid, NULL, &pid, &error)) { _LOGW ("dhclient failed to start: '%s'", error->message); g_error_free (error); - g_free (pid_file); + return FALSE; } - if (out_pid) - *out_pid = pid; + _LOGI ("dhclient started with pid %lld", (long long int) pid); - g_ptr_array_free (argv, TRUE); - g_free (system_bus_address_env); - return pid > 0 ? TRUE : FALSE; + if (!release) + nm_dhcp_client_watch_child (client, pid); + + priv->pid_file = g_steal_pointer (&pid_file); + + NM_SET_OUT (out_pid, pid); + return TRUE; } static gboolean