core/dhcp: merge branch 'th/dhcp-cleanup'

https://github.com/NetworkManager/NetworkManager/pull/122
This commit is contained in:
Thomas Haller 2018-05-26 20:13:02 +02:00
commit c9b0d3a425
9 changed files with 132 additions and 116 deletions

View file

@ -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 ();
}

View file

@ -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) {

View file

@ -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
@ -660,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);
}
}
@ -770,8 +771,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 +816,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,7 +845,8 @@ 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);
@ -853,10 +855,6 @@ nm_dhcp_client_handle_event (gpointer unused,
nm_dhcp_client_set_state (self, new_state, ip_config, str_options);
}
if (str_options)
g_hash_table_destroy (str_options);
g_clear_object (&ip_config);
return TRUE;
}

View file

@ -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,

View file

@ -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,16 @@ 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;
}
}
@ -441,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
@ -544,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) {
@ -604,7 +607,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);
@ -622,31 +625,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,

View file

@ -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);

View file

@ -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);

View file

@ -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) \

View file

@ -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);