From 7729555a595871049caab7b5ea811ea04c4d3f8d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 20 Sep 2018 10:15:48 +0200 Subject: [PATCH] acd: make NMAcdManager no GObject NMAcdManager is a rather simple instance. It does not need (thread-safe) ref-counting, in fact, having it ref-counted makes it slighly ugly that we connect a signal, but never bother to disconnect it (while the ref-counted instance could outlife the signal subscriber). We also don't need GObject signals. They have more overhead and are less type-safe than a regular function pointers. Signals would make sense, if there could be multiple independent listeners, but that just doesn't make sense. Implementing it as a plain struct is less lines of code, and less runtime over head. Also drop the possiblitiy to reset the NMAcdManager instance. It wasn't needed and I think it was buggy because it wouldn't reset the n-acd instance. https://github.com/NetworkManager/NetworkManager/pull/213 --- Makefile.am | 8 +- src/devices/nm-acd-manager.c | 223 ++++++++++++----------------------- src/devices/nm-acd-manager.h | 24 ++-- src/devices/nm-device.c | 53 +++++---- src/devices/tests/test-acd.c | 37 ++++-- src/nm-types.h | 1 - 6 files changed, 143 insertions(+), 203 deletions(-) diff --git a/Makefile.am b/Makefile.am index 13c07612fc..c9137595e5 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1668,12 +1668,12 @@ src_libNetworkManager_la_SOURCES = \ src/nm-checkpoint-manager.c \ src/nm-checkpoint-manager.h \ \ - src/devices/nm-device.c \ - src/devices/nm-device.h \ - src/devices/nm-lldp-listener.c \ - src/devices/nm-lldp-listener.h \ src/devices/nm-acd-manager.c \ src/devices/nm-acd-manager.h \ + src/devices/nm-lldp-listener.c \ + src/devices/nm-lldp-listener.h \ + src/devices/nm-device.c \ + src/devices/nm-device.h \ src/devices/nm-device-ethernet-utils.c \ src/devices/nm-device-ethernet-utils.h \ src/devices/nm-device-factory.c \ diff --git a/src/devices/nm-acd-manager.c b/src/devices/nm-acd-manager.c index 51ff233f2a..42dff5379a 100644 --- a/src/devices/nm-acd-manager.c +++ b/src/devices/nm-acd-manager.c @@ -42,14 +42,7 @@ typedef struct { NAcdProbe *probe; } AddressInfo; -enum { - PROBE_TERMINATED, - LAST_SIGNAL, -}; - -static guint signals[LAST_SIGNAL] = { 0 }; - -typedef struct { +struct _NMAcdManager { int ifindex; guint8 hwaddr[ETH_ALEN]; State state; @@ -58,21 +51,11 @@ typedef struct { NAcd *acd; GIOChannel *channel; guint event_id; -} NMAcdManagerPrivate; -struct _NMAcdManager { - GObject parent; - NMAcdManagerPrivate _priv; + NMAcdCallbacks callbacks; + gpointer user_data; }; -struct _NMAcdManagerClass { - GObjectClass parent; -}; - -G_DEFINE_TYPE (NMAcdManager, nm_acd_manager, G_TYPE_OBJECT) - -#define NM_ACD_MANAGER_GET_PRIVATE(self) _NM_GET_PRIVATE (self, NMAcdManager, NM_IS_ACD_MANAGER) - /*****************************************************************************/ #define _NMLOG_DOMAIN LOGD_IP4 @@ -80,14 +63,13 @@ G_DEFINE_TYPE (NMAcdManager, nm_acd_manager, G_TYPE_OBJECT) #define _NMLOG(level, ...) \ G_STMT_START { \ char _sbuf[64]; \ - int _ifindex = (self) ? NM_ACD_MANAGER_GET_PRIVATE (self)->ifindex : 0; \ \ nm_log ((level), _NMLOG_DOMAIN, \ - nm_platform_link_get_name (NM_PLATFORM_GET, _ifindex), \ + self ? nm_platform_link_get_name (NM_PLATFORM_GET, self->ifindex) : "", \ NULL, \ "%s%s: " _NM_UTILS_MACRO_FIRST (__VA_ARGS__), \ _NMLOG_PREFIX_NAME, \ - self ? nm_sprintf_buf (_sbuf, "[%p,%d]", self, _ifindex) : "" \ + self ? nm_sprintf_buf (_sbuf, "[%p,%d]", self, self->ifindex) : "" \ _NM_UTILS_MACRO_REST (__VA_ARGS__)); \ } G_STMT_END @@ -145,20 +127,18 @@ acd_error_to_string (int error) gboolean nm_acd_manager_add_address (NMAcdManager *self, in_addr_t address) { - NMAcdManagerPrivate *priv; AddressInfo *info; - g_return_val_if_fail (NM_IS_ACD_MANAGER (self), FALSE); - priv = NM_ACD_MANAGER_GET_PRIVATE (self); - g_return_val_if_fail (priv->state == STATE_INIT, FALSE); + g_return_val_if_fail (self, FALSE); + g_return_val_if_fail (self->state == STATE_INIT, FALSE); - if (g_hash_table_lookup (priv->addresses, GUINT_TO_POINTER (address))) + if (g_hash_table_lookup (self->addresses, GUINT_TO_POINTER (address))) return FALSE; info = g_slice_new0 (AddressInfo); info->address = address; - g_hash_table_insert (priv->addresses, GUINT_TO_POINTER (address), info); + g_hash_table_insert (self->addresses, GUINT_TO_POINTER (address), info); return TRUE; } @@ -167,7 +147,6 @@ static gboolean acd_event (GIOChannel *source, GIOCondition condition, gpointer data) { NMAcdManager *self = data; - NMAcdManagerPrivate *priv = NM_ACD_MANAGER_GET_PRIVATE (self); NAcdEvent *event; AddressInfo *info; gboolean emit_probe_terminated = FALSE; @@ -175,10 +154,10 @@ acd_event (GIOChannel *source, GIOCondition condition, gpointer data) gs_free char *hwaddr_str = NULL; int r; - if (n_acd_dispatch (priv->acd)) + if (n_acd_dispatch (self->acd)) return G_SOURCE_CONTINUE; - while ( !n_acd_pop_event (priv->acd, &event) + while ( !n_acd_pop_event (self->acd, &event) && event) { gboolean check_probing_done = FALSE; @@ -186,13 +165,13 @@ acd_event (GIOChannel *source, GIOCondition condition, gpointer data) case N_ACD_EVENT_READY: n_acd_probe_get_userdata (event->ready.probe, (void **) &info); info->duplicate = FALSE; - if (priv->state == STATE_ANNOUNCING) { + if (self->state == STATE_ANNOUNCING) { /* fake probe ended, start announcing */ r = n_acd_probe_announce (info->probe, N_ACD_DEFEND_ONCE); if (r) { _LOGW ("couldn't announce address %s on interface '%s': %s", nm_utils_inet4_ntop (info->address, address_str), - nm_platform_link_get_name (NM_PLATFORM_GET, priv->ifindex), + nm_platform_link_get_name (NM_PLATFORM_GET, self->ifindex), acd_error_to_string (r)); } else { _LOGD ("announcing address %s", @@ -219,7 +198,7 @@ acd_event (GIOChannel *source, GIOCondition condition, gpointer data) nm_utils_inet4_ntop (info->address, address_str), (hwaddr_str = nm_utils_hwaddr_ntoa (event->defended.sender, event->defended.n_sender)), - nm_platform_link_get_name (NM_PLATFORM_GET, priv->ifindex)); + nm_platform_link_get_name (NM_PLATFORM_GET, self->ifindex)); break; default: _LOGD ("unhandled event '%s'", acd_event_to_string (event->event)); @@ -227,15 +206,19 @@ acd_event (GIOChannel *source, GIOCondition condition, gpointer data) } if ( check_probing_done - && priv->state == STATE_PROBING - && ++priv->completed == g_hash_table_size (priv->addresses)) { - priv->state = STATE_PROBE_DONE; + && self->state == STATE_PROBING + && ++self->completed == g_hash_table_size (self->addresses)) { + self->state = STATE_PROBE_DONE; emit_probe_terminated = TRUE; } } - if (emit_probe_terminated) - g_signal_emit (self, signals[PROBE_TERMINATED], 0); + if (emit_probe_terminated) { + if (self->callbacks.probe_terminated_callback) { + self->callbacks.probe_terminated_callback (self, + self->user_data); + } + } return G_SOURCE_CONTINUE; } @@ -245,7 +228,6 @@ acd_probe_add (NMAcdManager *self, AddressInfo *info, guint64 timeout) { - NMAcdManagerPrivate *priv = NM_ACD_MANAGER_GET_PRIVATE (self); NAcdProbeConfig *probe_config; int r; @@ -253,7 +235,7 @@ acd_probe_add (NMAcdManager *self, if (r) { _LOGW ("could not create probe config for %s on interface '%s': %s", nm_utils_inet4_ntop (info->address, NULL), - nm_platform_link_get_name (NM_PLATFORM_GET, priv->ifindex), + nm_platform_link_get_name (NM_PLATFORM_GET, self->ifindex), acd_error_to_string (r)); return FALSE; } @@ -261,11 +243,11 @@ acd_probe_add (NMAcdManager *self, n_acd_probe_config_set_ip (probe_config, (struct in_addr) { info->address }); n_acd_probe_config_set_timeout (probe_config, timeout); - r = n_acd_probe (priv->acd, &info->probe, probe_config); + r = n_acd_probe (self->acd, &info->probe, probe_config); if (r) { _LOGW ("could not start probe for %s on interface '%s': %s", nm_utils_inet4_ntop (info->address, NULL), - nm_platform_link_get_name (NM_PLATFORM_GET, priv->ifindex), + nm_platform_link_get_name (NM_PLATFORM_GET, self->ifindex), acd_error_to_string (r)); n_acd_probe_config_free (probe_config); return FALSE; @@ -280,22 +262,21 @@ acd_probe_add (NMAcdManager *self, static int acd_init (NMAcdManager *self) { - NMAcdManagerPrivate *priv = NM_ACD_MANAGER_GET_PRIVATE (self); NAcdConfig *config; int r; - if (priv->acd) + if (self->acd) return 0; r = n_acd_config_new (&config); if (r) return r; - n_acd_config_set_ifindex (config, priv->ifindex); + n_acd_config_set_ifindex (config, self->ifindex); n_acd_config_set_transport (config, N_ACD_TRANSPORT_ETHERNET); - n_acd_config_set_mac (config, priv->hwaddr, ETH_ALEN); + n_acd_config_set_mac (config, self->hwaddr, ETH_ALEN); - r = n_acd_new (&priv->acd, config); + r = n_acd_new (&self->acd, config); n_acd_config_free (config); return r; } @@ -314,74 +295,38 @@ acd_init (NMAcdManager *self) gboolean nm_acd_manager_start_probe (NMAcdManager *self, guint timeout) { - NMAcdManagerPrivate *priv; GHashTableIter iter; AddressInfo *info; gboolean success = FALSE; int fd, r; - g_return_val_if_fail (NM_IS_ACD_MANAGER (self), FALSE); - priv = NM_ACD_MANAGER_GET_PRIVATE (self); - g_return_val_if_fail (priv->state == STATE_INIT, FALSE); + g_return_val_if_fail (self, FALSE); + g_return_val_if_fail (self->state == STATE_INIT, FALSE); r = acd_init (self); if (r) { _LOGW ("couldn't init ACD for probing on interface '%s': %s", - nm_platform_link_get_name (NM_PLATFORM_GET, priv->ifindex), + nm_platform_link_get_name (NM_PLATFORM_GET, self->ifindex), acd_error_to_string (r)); return FALSE; } - priv->completed = 0; + self->completed = 0; - g_hash_table_iter_init (&iter, priv->addresses); + g_hash_table_iter_init (&iter, self->addresses); while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &info)) success |= acd_probe_add (self, info, timeout); if (success) - priv->state = STATE_PROBING; + self->state = STATE_PROBING; - n_acd_get_fd (priv->acd, &fd); - priv->channel = g_io_channel_unix_new (fd); - priv->event_id = g_io_add_watch (priv->channel, G_IO_IN, acd_event, self); + n_acd_get_fd (self->acd, &fd); + self->channel = g_io_channel_unix_new (fd); + self->event_id = g_io_add_watch (self->channel, G_IO_IN, acd_event, self); return success; } -/** - * nm_acd_manager_reset: - * @self: a #NMAcdManager - * - * Stop any operation in progress and reset @self to the initial state. - */ -void -nm_acd_manager_reset (NMAcdManager *self) -{ - NMAcdManagerPrivate *priv; - - g_return_if_fail (NM_IS_ACD_MANAGER (self)); - priv = NM_ACD_MANAGER_GET_PRIVATE (self); - - g_hash_table_remove_all (priv->addresses); - - priv->state = STATE_INIT; -} - -/** - * nm_acd_manager_destroy: - * @self: the #NMAcdManager - * - * Calls nm_acd_manager_reset() and unrefs @self. - */ -void -nm_acd_manager_destroy (NMAcdManager *self) -{ - g_return_if_fail (NM_IS_ACD_MANAGER (self)); - - nm_acd_manager_reset (self); - g_object_unref (self); -} - /** * nm_acd_manager_check_address: * @self: a #NMAcdManager @@ -395,15 +340,12 @@ nm_acd_manager_destroy (NMAcdManager *self) gboolean nm_acd_manager_check_address (NMAcdManager *self, in_addr_t address) { - NMAcdManagerPrivate *priv; AddressInfo *info; - g_return_val_if_fail (NM_IS_ACD_MANAGER (self), FALSE); - priv = NM_ACD_MANAGER_GET_PRIVATE (self); - g_return_val_if_fail ( priv->state == STATE_INIT - || priv->state == STATE_PROBE_DONE, FALSE); + g_return_val_if_fail (self, FALSE); + g_return_val_if_fail (NM_IN_SET (self->state, STATE_INIT, STATE_PROBE_DONE), FALSE); - info = g_hash_table_lookup (priv->addresses, GUINT_TO_POINTER (address)); + info = g_hash_table_lookup (self->addresses, GUINT_TO_POINTER (address)); g_return_val_if_fail (info, FALSE); return !info->duplicate; @@ -418,7 +360,6 @@ nm_acd_manager_check_address (NMAcdManager *self, in_addr_t address) void nm_acd_manager_announce_addresses (NMAcdManager *self) { - NMAcdManagerPrivate *priv = NM_ACD_MANAGER_GET_PRIVATE (self); GHashTableIter iter; AddressInfo *info; int r; @@ -426,21 +367,21 @@ nm_acd_manager_announce_addresses (NMAcdManager *self) r = acd_init (self); if (r) { _LOGW ("couldn't init ACD for announcing addresses on interface '%s': %s", - nm_platform_link_get_name (NM_PLATFORM_GET, priv->ifindex), + nm_platform_link_get_name (NM_PLATFORM_GET, self->ifindex), acd_error_to_string (r)); return; } - if (priv->state == STATE_INIT) { + if (self->state == STATE_INIT) { /* n-acd can't announce without probing, therefore let's * start a fake probe with zero timeout and then perform * the announcement. */ - g_hash_table_iter_init (&iter, priv->addresses); + g_hash_table_iter_init (&iter, self->addresses); while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &info)) acd_probe_add (self, info, 0); - priv->state = STATE_ANNOUNCING; - } else if (priv->state == STATE_ANNOUNCING) { - g_hash_table_iter_init (&iter, priv->addresses); + self->state = STATE_ANNOUNCING; + } else if (self->state == STATE_ANNOUNCING) { + g_hash_table_iter_init (&iter, self->addresses); while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &info)) { if (info->duplicate) continue; @@ -448,7 +389,7 @@ nm_acd_manager_announce_addresses (NMAcdManager *self) if (r) { _LOGW ("couldn't announce address %s on interface '%s': %s", nm_utils_inet4_ntop (info->address, NULL), - nm_platform_link_get_name (NM_PLATFORM_GET, priv->ifindex), + nm_platform_link_get_name (NM_PLATFORM_GET, self->ifindex), acd_error_to_string (r)); } else _LOGD ("announcing address %s", nm_utils_inet4_ntop (info->address, NULL)); @@ -468,59 +409,45 @@ destroy_address_info (gpointer data) /*****************************************************************************/ -static void -nm_acd_manager_init (NMAcdManager *self) -{ - NMAcdManagerPrivate *priv = NM_ACD_MANAGER_GET_PRIVATE (self); - - priv->addresses = g_hash_table_new_full (nm_direct_hash, NULL, - NULL, destroy_address_info); - priv->state = STATE_INIT; -} - NMAcdManager * -nm_acd_manager_new (int ifindex, const guint8 *hwaddr, guint hwaddr_len) +nm_acd_manager_new (int ifindex, + const guint8 *hwaddr, + guint hwaddr_len, + const NMAcdCallbacks *callbacks, + gpointer user_data) { NMAcdManager *self; - NMAcdManagerPrivate *priv; g_return_val_if_fail (ifindex > 0, NULL); g_return_val_if_fail (hwaddr, NULL); g_return_val_if_fail (hwaddr_len == ETH_ALEN, NULL); - self = g_object_new (NM_TYPE_ACD_MANAGER, NULL); - priv = NM_ACD_MANAGER_GET_PRIVATE (self); - priv->ifindex = ifindex; - memcpy (priv->hwaddr, hwaddr, ETH_ALEN); + self = g_slice_new0 (NMAcdManager); + if (callbacks) + self->callbacks = *callbacks; + self->user_data = user_data; + + self->addresses = g_hash_table_new_full (nm_direct_hash, NULL, + NULL, destroy_address_info); + self->state = STATE_INIT; + self->ifindex = ifindex; + memcpy (self->hwaddr, hwaddr, ETH_ALEN); return self; } -static void -dispose (GObject *object) +void +nm_acd_manager_free (NMAcdManager *self) { - NMAcdManager *self = NM_ACD_MANAGER (object); - NMAcdManagerPrivate *priv = NM_ACD_MANAGER_GET_PRIVATE (self); + g_return_if_fail (self); - g_clear_pointer (&priv->addresses, g_hash_table_destroy); - g_clear_pointer (&priv->channel, g_io_channel_unref); - nm_clear_g_source (&priv->event_id); - nm_clear_pointer (&priv->acd, n_acd_unref); + if (self->callbacks.user_data_destroy) + self->callbacks.user_data_destroy (self->user_data); - G_OBJECT_CLASS (nm_acd_manager_parent_class)->dispose (object); -} - -static void -nm_acd_manager_class_init (NMAcdManagerClass *klass) -{ - GObjectClass *object_class = G_OBJECT_CLASS (klass); - - object_class->dispose = dispose; - - signals[PROBE_TERMINATED] = - g_signal_new (NM_ACD_MANAGER_PROBE_TERMINATED, - G_OBJECT_CLASS_TYPE (object_class), - G_SIGNAL_RUN_FIRST, - 0, NULL, NULL, NULL, - G_TYPE_NONE, 0); + nm_clear_pointer (&self->addresses, g_hash_table_destroy); + nm_clear_pointer (&self->channel, g_io_channel_unref); + nm_clear_g_source (&self->event_id); + nm_clear_pointer (&self->acd, n_acd_unref); + + g_slice_free (NMAcdManager, self); } diff --git a/src/devices/nm-acd-manager.h b/src/devices/nm-acd-manager.h index d698c2f4be..758848460a 100644 --- a/src/devices/nm-acd-manager.h +++ b/src/devices/nm-acd-manager.h @@ -19,25 +19,25 @@ #include -#define NM_TYPE_ACD_MANAGER (nm_acd_manager_get_type ()) -#define NM_ACD_MANAGER(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_ACD_MANAGER, NMAcdManager)) -#define NM_ACD_MANAGER_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_ACD_MANAGER, NMAcdManagerClass)) -#define NM_IS_ACD_MANAGER(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), NM_TYPE_ACD_MANAGER)) -#define NM_IS_ACD_MANAGER_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_ACD_MANAGER)) -#define NM_ACD_MANAGER_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_ACD_MANAGER, NMAcdManagerClass)) +typedef struct _NMAcdManager NMAcdManager; -#define NM_ACD_MANAGER_PROBE_TERMINATED "probe-terminated" +typedef struct { + void (*probe_terminated_callback) (NMAcdManager *self, + gpointer user_data); + GDestroyNotify user_data_destroy; +} NMAcdCallbacks; -typedef struct _NMAcdManagerClass NMAcdManagerClass; +NMAcdManager *nm_acd_manager_new (int ifindex, + const guint8 *hwaddr, + guint hwaddr_len, + const NMAcdCallbacks *callbacks, + gpointer user_data); -GType nm_acd_manager_get_type (void); +void nm_acd_manager_free (NMAcdManager *self); -NMAcdManager *nm_acd_manager_new (int ifindex, const guint8 *hwaddr, guint hwaddr_len); -void nm_acd_manager_destroy (NMAcdManager *self); gboolean nm_acd_manager_add_address (NMAcdManager *self, in_addr_t address); gboolean nm_acd_manager_start_probe (NMAcdManager *self, guint timeout); gboolean nm_acd_manager_check_address (NMAcdManager *self, in_addr_t address); void nm_acd_manager_announce_addresses (NMAcdManager *self); -void nm_acd_manager_reset (NMAcdManager *self); #endif /* __NM_ACD_MANAGER__ */ diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 3fae4df152..f025574d31 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -6623,17 +6623,15 @@ get_ipv4_dad_timeout (NMDevice *self) } static void -acd_data_destroy (gpointer ptr, GClosure *closure) +acd_data_destroy (gpointer ptr) { AcdData *data = ptr; int i; - if (data) { - for (i = 0; data->configs && data->configs[i]; i++) - g_object_unref (data->configs[i]); - g_free (data->configs); - g_slice_free (AcdData, data); - } + for (i = 0; data->configs && data->configs[i]; i++) + g_object_unref (data->configs[i]); + g_free (data->configs); + g_slice_free (AcdData, data); } static void @@ -6667,8 +6665,9 @@ ipv4_manual_method_apply (NMDevice *self, NMIP4Config **configs, gboolean succes } static void -acd_manager_probe_terminated (NMAcdManager *acd_manager, AcdData *data) +acd_manager_probe_terminated (NMAcdManager *acd_manager, gpointer user_data) { + AcdData *data = user_data; NMDevice *self; NMDevicePrivate *priv; NMDedupMultiIter ipconf_iter; @@ -6696,7 +6695,7 @@ acd_manager_probe_terminated (NMAcdManager *acd_manager, AcdData *data) data->callback (self, data->configs, success); priv->acd.dad_list = g_slist_remove (priv->acd.dad_list, acd_manager); - nm_acd_manager_destroy (acd_manager); + nm_acd_manager_free (acd_manager); } /** @@ -6712,6 +6711,10 @@ acd_manager_probe_terminated (NMAcdManager *acd_manager, AcdData *data) static void ipv4_dad_start (NMDevice *self, NMIP4Config **configs, AcdCallback cb) { + static const NMAcdCallbacks acd_callbacks = { + .probe_terminated_callback = acd_manager_probe_terminated, + .user_data_destroy = acd_data_destroy, + }; NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMAcdManager *acd_manager; const NMPlatformIP4Address *address; @@ -6755,26 +6758,23 @@ ipv4_dad_start (NMDevice *self, NMIP4Config **configs, AcdCallback cb) return; } - /* don't take additional references of @acd_manager that outlive @self. - * Otherwise, the callback can be invoked on a dangling pointer as we don't - * disconnect the handler. */ - acd_manager = nm_acd_manager_new (nm_device_get_ip_ifindex (self), hwaddr_arr, length); - priv->acd.dad_list = g_slist_append (priv->acd.dad_list, acd_manager); - data = g_slice_new0 (AcdData); data->configs = configs; data->callback = cb; data->device = self; + acd_manager = nm_acd_manager_new (nm_device_get_ip_ifindex (self), + hwaddr_arr, + length, + &acd_callbacks, + data); + priv->acd.dad_list = g_slist_append (priv->acd.dad_list, acd_manager); + for (i = 0; configs[i]; i++) { nm_ip_config_iter_ip4_address_for_each (&ipconf_iter, configs[i], &address) nm_acd_manager_add_address (acd_manager, address->address); } - g_signal_connect_data (acd_manager, NM_ACD_MANAGER_PROBE_TERMINATED, - G_CALLBACK (acd_manager_probe_terminated), data, - acd_data_destroy, 0); - ret = nm_acd_manager_start_probe (acd_manager, timeout); if (!ret) { @@ -6784,7 +6784,7 @@ ipv4_dad_start (NMDevice *self, NMIP4Config **configs, AcdCallback cb) cb (self, configs, TRUE); priv->acd.dad_list = g_slist_remove (priv->acd.dad_list, acd_manager); - nm_acd_manager_destroy (acd_manager); + nm_acd_manager_free (acd_manager); } } @@ -10210,10 +10210,7 @@ arp_cleanup (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - if (priv->acd.announcing) { - nm_acd_manager_destroy (priv->acd.announcing); - priv->acd.announcing = NULL; - } + nm_clear_pointer (&priv->acd.announcing, nm_acd_manager_free); } void @@ -10248,7 +10245,11 @@ nm_device_arp_announce (NMDevice *self) if (num == 0) return; - priv->acd.announcing = nm_acd_manager_new (nm_device_get_ip_ifindex (self), hw_addr, hw_addr_len); + priv->acd.announcing = nm_acd_manager_new (nm_device_get_ip_ifindex (self), + hw_addr, + hw_addr_len, + NULL, + NULL); for (i = 0; i < num; i++) { NMIPAddress *ip = nm_setting_ip_config_get_address (s_ip4, i); @@ -10723,7 +10724,7 @@ _cleanup_ip_pre (NMDevice *self, int addr_family, CleanupType cleanup_type) arp_cleanup (self); dnsmasq_cleanup (self); ipv4ll_cleanup (self); - g_slist_free_full (priv->acd.dad_list, (GDestroyNotify) nm_acd_manager_destroy); + g_slist_free_full (priv->acd.dad_list, (GDestroyNotify) nm_acd_manager_free); priv->acd.dad_list = NULL; } else { g_slist_free_full (priv->dad6_failed_addrs, (GDestroyNotify) nmp_object_unref); diff --git a/src/devices/tests/test-acd.c b/src/devices/tests/test-acd.c index 8a2852a2d7..6f4ba8d8fd 100644 --- a/src/devices/tests/test-acd.c +++ b/src/devices/tests/test-acd.c @@ -61,20 +61,23 @@ typedef struct { } TestInfo; static void -acd_manager_probe_terminated (NMAcdManager *acd_manager, GMainLoop *loop) +acd_manager_probe_terminated (NMAcdManager *acd_manager, gpointer user_data) { - g_main_loop_quit (loop); + g_main_loop_quit (user_data); } static void test_acd_common (test_fixture *fixture, TestInfo *info) { - gs_unref_object NMAcdManager *manager = NULL; + NMAcdManager *manager; GMainLoop *loop; int i; const guint WAIT_TIME_OPTIMISTIC = 50; guint wait_time; - gulong signal_id; + static const NMAcdCallbacks callbacks = { + .probe_terminated_callback = acd_manager_probe_terminated, + .user_data_destroy = (GDestroyNotify) g_main_loop_unref, + }; /* first, try with a short waittime. We hope that this is long enough * to successfully complete the test. Only if that's not the case, we @@ -83,7 +86,13 @@ test_acd_common (test_fixture *fixture, TestInfo *info) wait_time = WAIT_TIME_OPTIMISTIC; again: - manager = nm_acd_manager_new (fixture->ifindex0, fixture->hwaddr0, fixture->hwaddr0_len); + loop = g_main_loop_new (NULL, FALSE); + + manager = nm_acd_manager_new (fixture->ifindex0, + fixture->hwaddr0, + fixture->hwaddr0_len, + &callbacks, + g_main_loop_ref (loop)); g_assert (manager != NULL); for (i = 0; info->addresses[i]; i++) @@ -94,12 +103,8 @@ again: 24, 0, 3600, 1800, 0, NULL); } - loop = g_main_loop_new (NULL, FALSE); - signal_id = g_signal_connect (manager, NM_ACD_MANAGER_PROBE_TERMINATED, - G_CALLBACK (acd_manager_probe_terminated), loop); g_assert (nm_acd_manager_start_probe (manager, wait_time)); g_assert (nmtst_main_loop_run (loop, 2000)); - g_signal_handler_disconnect (manager, signal_id); g_main_loop_unref (loop); for (i = 0; info->addresses[i]; i++) { @@ -113,7 +118,7 @@ again: /* probably we just had a glitch and the system took longer than * expected. Re-verify with a large timeout this time. */ wait_time = 1000; - g_clear_object (&manager); + nm_clear_pointer (&manager, nm_acd_manager_free); goto again; } @@ -121,6 +126,8 @@ again: i, nm_utils_inet4_ntop (info->addresses[i], NULL), info->expected_result[i] ? "detect no duplicated" : "detect a duplicate"); } + + nm_acd_manager_free (manager); } static void @@ -146,10 +153,14 @@ test_acd_probe_2 (test_fixture *fixture, gconstpointer user_data) static void test_acd_announce (test_fixture *fixture, gconstpointer user_data) { - gs_unref_object NMAcdManager *manager = NULL; + NMAcdManager *manager; GMainLoop *loop; - manager = nm_acd_manager_new (fixture->ifindex0, fixture->hwaddr0, fixture->hwaddr0_len); + manager = nm_acd_manager_new (fixture->ifindex0, + fixture->hwaddr0, + fixture->hwaddr0_len, + NULL, + NULL); g_assert (manager != NULL); g_assert (nm_acd_manager_add_address (manager, ADDR1)); @@ -159,6 +170,8 @@ test_acd_announce (test_fixture *fixture, gconstpointer user_data) nm_acd_manager_announce_addresses (manager); g_assert (!nmtst_main_loop_run (loop, 200)); g_main_loop_unref (loop); + + nm_acd_manager_free (manager); } static void diff --git a/src/nm-types.h b/src/nm-types.h index a0a7f6202e..676ca64169 100644 --- a/src/nm-types.h +++ b/src/nm-types.h @@ -37,7 +37,6 @@ typedef struct _NMAuthSubject NMAuthSubject; typedef struct _NMDBusManager NMDBusManager; typedef struct _NMConfig NMConfig; typedef struct _NMConfigData NMConfigData; -typedef struct _NMAcdManager NMAcdManager; typedef struct _NMConnectivity NMConnectivity; typedef struct _NMDevice NMDevice; typedef struct _NMDhcp4Config NMDhcp4Config;