dhcp: replace NMDhcpClient's signals with "notify" and one notify data argument

NMDhcpClient communicates events via GObject signals. GObject signals in
principle could have several subscribers. In practice, a NMDhcpClient
instance has only one subscriber, because it was constructed with
certain parameters, so it's unlikely to be shared.

That one subscriber, always needs to subscribe to all signals
("state-changed" and "prefix-delegated"), Unless the subscriber only
creates a IPv4 client. In which case they won't subscribe to
"prefix-delegated", but then that signal is also no invoked.

Combine the signals in one, and pass all parameters via a new
NMDhcpClientNotfiyData payload. I feel this is nicer, to pack all
parameters together.

With l3cfg work, DHCP handling will be refactored, where this model of
having one "generic" notify signal makes more sense than here. In this
case, it is arguably pretty much the same. Also, because NMDhcpClient
subscribes two different handlers for IPv4 and IPv6. In the future,
there will be only one notify handler, and that can cover IPv4 and IPv6
and both "state-changed" and "prefix-delegated".

In the end, I find it nicer to switch on a C structure/enum, than
subscribe to multiple signals (and track that we have to unsubscribe
too).

5 files changed, 135 insertions, 100 deletions

diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c
index 97c92d5611..6ad05a95df 100644
--- a/src/core/devices/nm-device.c
+++ b/src/core/devices/nm-device.c
@@ -185,7 +185,7 @@ typedef struct {
 typedef struct {
     NMDhcpClient *client;
     NMDhcpConfig *config;
-    gulong        state_sigid;
+    gulong        notify_sigid;
     guint         grace_id;
     bool          grace_pending : 1;
     bool          was_active : 1;
@@ -579,7 +579,6 @@ typedef struct _NMDevicePrivate {
         AppliedConfig ip6_config;
         /* Event ID of the current IP6 config from DHCP */
         char *           event_id;
-        gulong           prefix_sigid;
         NMNDiscDHCPLevel mode;
         guint            needed_prefixes;
     } dhcp6;
@@ -9035,7 +9034,7 @@ dhcp4_cleanup(NMDevice *self, CleanupType cleanup_type, gboolean release)
 
     if (priv->dhcp_data_4.client) {
         /* Stop any ongoing DHCP transaction on this device */
-        nm_clear_g_signal_handler(priv->dhcp_data_4.client, &priv->dhcp_data_4.state_sigid);
+        nm_clear_g_signal_handler(priv->dhcp_data_4.client, &priv->dhcp_data_4.notify_sigid);
 
         if (cleanup_type == CLEANUP_TYPE_DECONFIGURE || cleanup_type == CLEANUP_TYPE_REMOVED)
             nm_dhcp_client_stop(priv->dhcp_data_4.client, release);
@@ -9436,21 +9435,27 @@ dhcp4_dad_cb(NMDevice *self, NMIP4Config **configs, gboolean success)
 }
 
 static void
-dhcp4_state_changed(NMDhcpClient *client,
-                    NMDhcpState   state,
-                    NMIP4Config * ip4_config,
-                    GHashTable *  options,
-                    gpointer      user_data)
+dhcp4_notify(NMDhcpClient *client, const NMDhcpClientNotifyData *notify_data, NMDevice *self)
 {
-    NMDevice *       self = NM_DEVICE(user_data);
     NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self);
-    NMIP4Config *    manual, **configs;
+    NMIP4Config *    manual;
+    NMIP4Config **   configs;
     NMConnection *   connection;
+    NMDhcpState      state;
+    NMIP4Config *    ip4_config;
+    GHashTable *     options;
+
+    nm_assert(nm_dhcp_client_get_addr_family(client) == AF_INET);
+    nm_assert(notify_data);
+    nm_assert(notify_data->notify_type == NM_DHCP_CLIENT_NOTIFY_TYPE_STATE_CHANGED);
+
+    state      = notify_data->state_changed.dhcp_state;
+    ip4_config = NM_IP4_CONFIG(notify_data->state_changed.ip_config);
+    options    = notify_data->state_changed.options;
 
-    g_return_if_fail(nm_dhcp_client_get_addr_family(client) == AF_INET);
-    g_return_if_fail(!ip4_config || NM_IS_IP4_CONFIG(ip4_config));
+    nm_assert(!ip4_config || NM_IS_IP4_CONFIG(ip4_config));
 
-    _LOGD(LOGD_DHCP4, "new DHCPv4 client state %d", state);
+    _LOGD(LOGD_DHCP4, "new DHCPv4 client state %d", (int) state);
 
     switch (state) {
     case NM_DHCP_STATE_BOUND:
@@ -9601,10 +9606,10 @@ dhcp4_start(NMDevice *self)
         return NM_ACT_STAGE_RETURN_FAILURE;
     }
 
-    priv->dhcp_data_4.state_sigid = g_signal_connect(priv->dhcp_data_4.client,
-                                                     NM_DHCP_CLIENT_SIGNAL_STATE_CHANGED,
-                                                     G_CALLBACK(dhcp4_state_changed),
-                                                     self);
+    priv->dhcp_data_4.notify_sigid = g_signal_connect(priv->dhcp_data_4.client,
+                                                      NM_DHCP_CLIENT_NOTIFY,
+                                                      G_CALLBACK(dhcp4_notify),
+                                                      self);
 
     if (nm_device_sys_iface_state_is_external_or_assume(self))
         priv->dhcp_data_4.was_active = TRUE;
@@ -9775,8 +9780,7 @@ dhcp6_cleanup(NMDevice *self, CleanupType cleanup_type, gboolean release)
     priv->dhcp_data_6.grace_pending = FALSE;
 
     if (priv->dhcp_data_6.client) {
-        nm_clear_g_signal_handler(priv->dhcp_data_6.client, &priv->dhcp_data_6.state_sigid);
-        nm_clear_g_signal_handler(priv->dhcp_data_6.client, &priv->dhcp6.prefix_sigid);
+        nm_clear_g_signal_handler(priv->dhcp_data_6.client, &priv->dhcp_data_6.notify_sigid);
 
         if (cleanup_type == CLEANUP_TYPE_DECONFIGURE || cleanup_type == CLEANUP_TYPE_REMOVED)
             nm_dhcp_client_stop(priv->dhcp_data_6.client, release);
@@ -9876,20 +9880,34 @@ clear_config:
 }
 
 static void
-dhcp6_state_changed(NMDhcpClient *client,
-                    NMDhcpState   state,
-                    NMIP6Config * ip6_config,
-                    GHashTable *  options,
-                    gpointer      user_data)
+dhcp6_notify(NMDhcpClient *client, const NMDhcpClientNotifyData *notify_data, NMDevice *self)
 {
-    NMDevice *       self     = NM_DEVICE(user_data);
     NMDevicePrivate *priv     = NM_DEVICE_GET_PRIVATE(self);
     gs_free char *   event_id = NULL;
+    NMDhcpState      state;
+    NMIP6Config *    ip6_config;
+    GHashTable *     options;
+
+    nm_assert(nm_dhcp_client_get_addr_family(client) == AF_INET6);
+    nm_assert(notify_data);
+
+    if (notify_data->notify_type == NM_DHCP_CLIENT_NOTIFY_TYPE_PREFIX_DELEGATED) {
+        /* Just re-emit. The device just contributes the prefix to the
+         * pool in NMPolicy, which decides about subnet allocation
+         * on the shared devices. */
+        g_signal_emit(self, signals[IP6_PREFIX_DELEGATED], 0, notify_data->prefix_delegated.prefix);
+        return;
+    }
+
+    nm_assert(notify_data->notify_type == NM_DHCP_CLIENT_NOTIFY_TYPE_STATE_CHANGED);
+
+    state      = notify_data->state_changed.dhcp_state;
+    ip6_config = NM_IP6_CONFIG(notify_data->state_changed.ip_config);
+    options    = notify_data->state_changed.options;
 
-    g_return_if_fail(nm_dhcp_client_get_addr_family(client) == AF_INET6);
-    g_return_if_fail(!ip6_config || NM_IS_IP6_CONFIG(ip6_config));
+    nm_assert(!ip6_config || NM_IS_IP6_CONFIG(ip6_config));
 
-    _LOGD(LOGD_DHCP6, "new DHCPv6 client state %d", state);
+    _LOGD(LOGD_DHCP6, "new DHCPv6 client state %d", (int) state);
 
     switch (state) {
     case NM_DHCP_STATE_BOUND:
@@ -9969,17 +9987,6 @@ dhcp6_state_changed(NMDhcpClient *client,
     }
 }
 
-static void
-dhcp6_prefix_delegated(NMDhcpClient *client, const NMPlatformIP6Address *prefix, gpointer user_data)
-{
-    NMDevice *self = NM_DEVICE(user_data);
-
-    /* Just re-emit. The device just contributes the prefix to the
-     * pool in NMPolicy, which decides about subnet allocation
-     * on the shared devices. */
-    g_signal_emit(self, signals[IP6_PREFIX_DELEGATED], 0, prefix);
-}
-
 /*****************************************************************************/
 
 static gboolean
@@ -10061,14 +10068,10 @@ dhcp6_start_with_link_ready(NMDevice *self, NMConnection *connection)
         return FALSE;
     }
 
-    priv->dhcp_data_6.state_sigid = g_signal_connect(priv->dhcp_data_6.client,
-                                                     NM_DHCP_CLIENT_SIGNAL_STATE_CHANGED,
-                                                     G_CALLBACK(dhcp6_state_changed),
-                                                     self);
-    priv->dhcp6.prefix_sigid      = g_signal_connect(priv->dhcp_data_6.client,
-                                                NM_DHCP_CLIENT_SIGNAL_PREFIX_DELEGATED,
-                                                G_CALLBACK(dhcp6_prefix_delegated),
-                                                self);
+    priv->dhcp_data_6.notify_sigid = g_signal_connect(priv->dhcp_data_6.client,
+                                                      NM_DHCP_CLIENT_NOTIFY,
+                                                      G_CALLBACK(dhcp6_notify),
+                                                      self);
 
     if (nm_device_sys_iface_state_is_external_or_assume(self))
         priv->dhcp_data_6.was_active = TRUE;
diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c
index 07db0da438..972bc5f583 100644
--- a/src/core/dhcp/nm-dhcp-client.c
+++ b/src/core/dhcp/nm-dhcp-client.c
@@ -28,7 +28,7 @@
 
 /*****************************************************************************/
 
-enum { SIGNAL_STATE_CHANGED, SIGNAL_PREFIX_DELEGATED, LAST_SIGNAL };
+enum { SIGNAL_NOTIFY, LAST_SIGNAL };
 
 static guint signals[LAST_SIGNAL] = {0};
 
@@ -91,6 +91,33 @@ G_STATIC_ASSERT(!(((pid_t) -1) > 0));
 
 /*****************************************************************************/
 
+static void
+_emit_notify(NMDhcpClient *self, const NMDhcpClientNotifyData *notify_data)
+{
+    g_signal_emit(G_OBJECT(self), signals[SIGNAL_NOTIFY], 0, notify_data);
+}
+
+static void
+_emit_notify_state_changed(NMDhcpClient *self,
+                           NMDhcpState   dhcp_state,
+                           NMIPConfig *  ip_config,
+                           GHashTable *  options)
+{
+    const NMDhcpClientNotifyData notify_data = {
+        .notify_type = NM_DHCP_CLIENT_NOTIFY_TYPE_STATE_CHANGED,
+        .state_changed =
+            {
+                .dhcp_state = dhcp_state,
+                .ip_config  = ip_config,
+                .options    = options,
+            },
+    };
+
+    _emit_notify(self, &notify_data);
+}
+
+/*****************************************************************************/
+
 pid_t
 nm_dhcp_client_get_pid(NMDhcpClient *self)
 {
@@ -496,12 +523,8 @@ nm_dhcp_client_set_state(NMDhcpClient *self,
     }
 
     priv->state = new_state;
-    g_signal_emit(G_OBJECT(self),
-                  signals[SIGNAL_STATE_CHANGED],
-                  0,
-                  (guint) new_state,
-                  ip_config,
-                  options);
+
+    _emit_notify_state_changed(self, new_state, ip_config, options);
 }
 
 static gboolean
@@ -873,7 +896,15 @@ maybe_add_option(NMDhcpClient *self, GHashTable *hash, const char *key, GVariant
 void
 nm_dhcp_client_emit_ipv6_prefix_delegated(NMDhcpClient *self, const NMPlatformIP6Address *prefix)
 {
-    g_signal_emit(G_OBJECT(self), signals[SIGNAL_PREFIX_DELEGATED], 0, prefix);
+    const NMDhcpClientNotifyData notify_data = {
+        .notify_type = NM_DHCP_CLIENT_NOTIFY_TYPE_PREFIX_DELEGATED,
+        .prefix_delegated =
+            {
+                .prefix = prefix,
+            },
+    };
+
+    _emit_notify(self, &notify_data);
 }
 
 gboolean
@@ -1387,28 +1418,15 @@ nm_dhcp_client_class_init(NMDhcpClientClass *client_class)
 
     g_object_class_install_properties(object_class, _PROPERTY_ENUMS_LAST, obj_properties);
 
-    signals[SIGNAL_STATE_CHANGED] = g_signal_new(NM_DHCP_CLIENT_SIGNAL_STATE_CHANGED,
-                                                 G_OBJECT_CLASS_TYPE(object_class),
-                                                 G_SIGNAL_RUN_FIRST,
-                                                 0,
-                                                 NULL,
-                                                 NULL,
-                                                 NULL,
-                                                 G_TYPE_NONE,
-                                                 3,
-                                                 G_TYPE_UINT,
-                                                 G_TYPE_OBJECT,
-                                                 G_TYPE_HASH_TABLE);
-
-    signals[SIGNAL_PREFIX_DELEGATED] =
-        g_signal_new(NM_DHCP_CLIENT_SIGNAL_PREFIX_DELEGATED,
+    signals[SIGNAL_NOTIFY] =
+        g_signal_new(NM_DHCP_CLIENT_NOTIFY,
                      G_OBJECT_CLASS_TYPE(object_class),
                      G_SIGNAL_RUN_FIRST,
                      0,
                      NULL,
                      NULL,
-                     NULL,
+                     g_cclosure_marshal_VOID__POINTER,
                      G_TYPE_NONE,
                      1,
-                     G_TYPE_POINTER /* const NMPlatformIP6Address *prefix */);
+                     G_TYPE_POINTER /* const NMDhcpClientNotifyData *notify_data */);
 }
diff --git a/src/core/dhcp/nm-dhcp-client.h b/src/core/dhcp/nm-dhcp-client.h
index 2a4a56059b..49deb3305a 100644
--- a/src/core/dhcp/nm-dhcp-client.h
+++ b/src/core/dhcp/nm-dhcp-client.h
@@ -44,8 +44,7 @@
 #define NM_DHCP_CLIENT_VENDOR_CLASS_IDENTIFIER "vendor-class-identifier"
 #define NM_DHCP_CLIENT_REJECT_SERVERS          "reject-servers"
 
-#define NM_DHCP_CLIENT_SIGNAL_STATE_CHANGED    "state-changed"
-#define NM_DHCP_CLIENT_SIGNAL_PREFIX_DELEGATED "prefix-delegated"
+#define NM_DHCP_CLIENT_NOTIFY "dhcp-notify"
 
 typedef enum {
     NM_DHCP_STATE_UNKNOWN = 0,
@@ -59,6 +58,25 @@ typedef enum {
     NM_DHCP_STATE_NOOP,       /* state is a non operation for NetworkManager */
 } NMDhcpState;
 
+typedef enum _nm_packed {
+    NM_DHCP_CLIENT_NOTIFY_TYPE_STATE_CHANGED,
+    NM_DHCP_CLIENT_NOTIFY_TYPE_PREFIX_DELEGATED,
+} NMDhcpClientNotifyType;
+
+typedef struct {
+    NMDhcpClientNotifyType notify_type;
+    union {
+        struct {
+            NMIPConfig *ip_config;
+            GHashTable *options;
+            NMDhcpState dhcp_state;
+        } state_changed;
+        struct {
+            const NMPlatformIP6Address *prefix;
+        } prefix_delegated;
+    };
+} NMDhcpClientNotifyData;
+
 const char *nm_dhcp_state_to_string(NMDhcpState state);
 
 struct _NMDhcpClientPrivate;
diff --git a/src/core/dhcp/nm-dhcp-manager.c b/src/core/dhcp/nm-dhcp-manager.c
index c3328b010b..0bcc6e1959 100644
--- a/src/core/dhcp/nm-dhcp-manager.c
+++ b/src/core/dhcp/nm-dhcp-manager.c
@@ -45,11 +45,8 @@ G_DEFINE_TYPE(NMDhcpManager, nm_dhcp_manager, G_TYPE_OBJECT)
 
 /*****************************************************************************/
 
-static void client_state_changed(NMDhcpClient * client,
-                                 NMDhcpState    state,
-                                 GObject *      ip_config,
-                                 GVariant *     options,
-                                 NMDhcpManager *self);
+static void
+client_notify(NMDhcpClient *client, const NMDhcpClientNotifyData *notify_data, NMDhcpManager *self);
 
 /*****************************************************************************/
 
@@ -161,7 +158,7 @@ get_client_for_ifindex(NMDhcpManager *manager, int addr_family, int ifindex)
 static void
 remove_client(NMDhcpManager *self, NMDhcpClient *client)
 {
-    g_signal_handlers_disconnect_by_func(client, client_state_changed, self);
+    g_signal_handlers_disconnect_by_func(client, client_notify, self);
     c_list_unlink(&client->dhcp_client_lst);
 
     /* Stopping the client is left up to the controlling device
@@ -178,13 +175,10 @@ remove_client_unref(NMDhcpManager *self, NMDhcpClient *client)
 }
 
 static void
-client_state_changed(NMDhcpClient * client,
-                     NMDhcpState    state,
-                     GObject *      ip_config,
-                     GVariant *     options,
-                     NMDhcpManager *self)
+client_notify(NMDhcpClient *client, const NMDhcpClientNotifyData *notify_data, NMDhcpManager *self)
 {
-    if (state >= NM_DHCP_STATE_TIMEOUT)
+    if (notify_data->notify_type == NM_DHCP_CLIENT_NOTIFY_TYPE_STATE_CHANGED
+        && notify_data->state_changed.dhcp_state >= NM_DHCP_STATE_TIMEOUT)
         remove_client_unref(self, client);
 }
 
@@ -334,10 +328,7 @@ client_start(NMDhcpManager *           self,
                           NULL);
     nm_assert(client && c_list_is_empty(&client->dhcp_client_lst));
     c_list_link_tail(&priv->dhcp_client_lst_head, &client->dhcp_client_lst);
-    g_signal_connect(client,
-                     NM_DHCP_CLIENT_SIGNAL_STATE_CHANGED,
-                     G_CALLBACK(client_state_changed),
-                     self);
+    g_signal_connect(client, NM_DHCP_CLIENT_NOTIFY, G_CALLBACK(client_notify), self);
 
     /* unfortunately, our implementations work differently per address-family regarding client-id/DUID.
      *
diff --git a/src/core/nm-iface-helper.c b/src/core/nm-iface-helper.c
index 66338e8e1f..73000af099 100644
--- a/src/core/nm-iface-helper.c
+++ b/src/core/nm-iface-helper.c
@@ -92,24 +92,29 @@ static struct {
 /*****************************************************************************/
 
 static void
-dhcp4_state_changed(NMDhcpClient *client,
-                    NMDhcpState   state,
-                    NMIP4Config * ip4_config,
-                    GHashTable *  options,
-                    gpointer      user_data)
+_dhcp_client_notify_cb(NMDhcpClient *                client,
+                       const NMDhcpClientNotifyData *notify_data,
+                       gpointer                      user_data)
 {
     static NMIP4Config *last_config = NULL;
     NMIP4Config *       existing;
     gs_unref_ptrarray GPtrArray *ip4_dev_route_blacklist = NULL;
     gs_free_error GError *error                          = NULL;
+    NMIP4Config *         ip4_config;
 
-    g_return_if_fail(!ip4_config || NM_IS_IP4_CONFIG(ip4_config));
+    if (!notify_data || notify_data->notify_type != NM_DHCP_CLIENT_NOTIFY_TYPE_STATE_CHANGED)
+        g_return_if_reached();
 
-    _LOGD(LOGD_DHCP4, "new DHCPv4 client state %d", state);
+    nm_assert(!notify_data->state_changed.ip_config
+              || NM_IS_IP4_CONFIG(notify_data->state_changed.ip_config));
 
-    switch (state) {
+    _LOGD(LOGD_DHCP4, "new DHCPv4 client state %d", (int) notify_data->state_changed.dhcp_state);
+
+    switch (notify_data->state_changed.dhcp_state) {
     case NM_DHCP_STATE_BOUND:
     case NM_DHCP_STATE_EXTENDED:
+        ip4_config = NM_IP4_CONFIG(notify_data->state_changed.ip_config);
+
         g_assert(ip4_config);
         g_assert(nm_ip4_config_get_ifindex(ip4_config) == gl.ifindex);
 
@@ -684,8 +689,8 @@ main(int argc, char *argv[])
             g_error("failure to start DHCP: %s", error->message);
 
         g_signal_connect(dhcp4_client,
-                         NM_DHCP_CLIENT_SIGNAL_STATE_CHANGED,
-                         G_CALLBACK(dhcp4_state_changed),
+                         NM_DHCP_CLIENT_NOTIFY,
+                         G_CALLBACK(_dhcp_client_notify_cb),
                          NULL);
     }