dhcp: don't pass duid to client ip6_start() and stop()

We don't do that for ip4_start() either. The duid/client-id
is stored inside the NMDhcpClient instance, and the function can
access it from there.

Maybe, it is often preferable to have stateless objects and not
relying on ip4_start() to obtain the client ID from the client's
state. However, the purpose of the NMDhcpClient object is to
hold state about DHCP. To simplify the complexity of objects that
inherrently have state, we should be careful about mutating the state.
It adds little additional complexity of only reading the state when
needed anyway. In fact, it adds complexity, because previously
it wasn't enough to check all callers of nm_dhcp_client_get_client_id()
to see where the client-id is used. Instead, one would also need to
follow the @duid argument several layers of the call stack.
This commit is contained in:
Thomas Haller 2018-10-24 15:37:55 +02:00
parent cd9e418fbe
commit 7d55b1348b
6 changed files with 24 additions and 29 deletions

View file

@ -365,7 +365,7 @@ nm_dhcp_client_stop_pid (pid_t pid, const char *iface)
}
static void
stop (NMDhcpClient *self, gboolean release, GBytes *duid)
stop (NMDhcpClient *self, gboolean release)
{
NMDhcpClientPrivate *priv;
@ -557,14 +557,14 @@ nm_dhcp_client_start_ip6 (NMDhcpClient *self,
gs_unref_bytes GBytes *own_client_id = NULL;
g_return_val_if_fail (NM_IS_DHCP_CLIENT (self), FALSE);
g_return_val_if_fail (client_id, FALSE);
priv = NM_DHCP_CLIENT_GET_PRIVATE (self);
g_return_val_if_fail (priv->pid == -1, FALSE);
g_return_val_if_fail (priv->addr_family == AF_INET6, FALSE);
g_return_val_if_fail (priv->uuid != NULL, FALSE);
nm_assert (!priv->client_id);
nm_assert (client_id);
g_return_val_if_fail (!priv->client_id, FALSE);
if (!enforce_duid)
own_client_id = NM_DHCP_CLIENT_GET_CLASS (self)->get_duid (self);
@ -585,7 +585,6 @@ nm_dhcp_client_start_ip6 (NMDhcpClient *self,
dhcp_anycast_addr,
ll_addr,
privacy,
priv->client_id,
needed_prefixes,
error);
}
@ -652,7 +651,7 @@ nm_dhcp_client_stop (NMDhcpClient *self, gboolean release)
/* Kill the DHCP client */
old_pid = priv->pid;
NM_DHCP_CLIENT_GET_CLASS (self)->stop (self, release, priv->client_id);
NM_DHCP_CLIENT_GET_CLASS (self)->stop (self, release);
if (old_pid > 0)
_LOGI ("canceled DHCP transaction, DHCP client pid %d", old_pid);
else

View file

@ -86,13 +86,11 @@ typedef struct {
const char *anycast_addr,
const struct in6_addr *ll_addr,
NMSettingIP6ConfigPrivacy privacy,
GBytes *duid,
guint needed_prefixes,
GError **error);
void (*stop) (NMDhcpClient *self,
gboolean release,
GBytes *duid);
gboolean release);
/**
* get_duid:

View file

@ -339,7 +339,6 @@ create_dhclient_config (NMDhcpDhclient *self,
static gboolean
dhclient_start (NMDhcpClient *client,
const char *mode_opt,
GBytes *duid,
gboolean release,
pid_t *out_pid,
int prefixes,
@ -418,7 +417,9 @@ dhclient_start (NMDhcpClient *client,
/* Save the DUID to the leasefile dhclient will actually use */
if (addr_family == AF_INET6) {
if (!nm_dhcp_dhclient_save_duid (priv->lease_file, duid, &local)) {
if (!nm_dhcp_dhclient_save_duid (priv->lease_file,
nm_dhcp_client_get_client_id (client),
&local)) {
nm_utils_error_set (error,
NM_UTILS_ERROR_UNKNOWN,
"failed to save DUID to '%s': %s",
@ -537,7 +538,6 @@ ip4_start (NMDhcpClient *client,
nm_dhcp_client_set_client_id (client, new_client_id);
}
return dhclient_start (client,
NULL,
NULL,
FALSE,
NULL,
@ -550,7 +550,6 @@ ip6_start (NMDhcpClient *client,
const char *dhcp_anycast_addr,
const struct in6_addr *ll_addr,
NMSettingIP6ConfigPrivacy privacy,
GBytes *duid,
guint needed_prefixes,
GError **error)
{
@ -578,7 +577,6 @@ ip6_start (NMDhcpClient *client,
nm_dhcp_client_get_info_only (NM_DHCP_CLIENT (self))
? "-S"
: "-N",
duid,
FALSE,
NULL,
needed_prefixes,
@ -586,12 +584,12 @@ ip6_start (NMDhcpClient *client,
}
static void
stop (NMDhcpClient *client, gboolean release, GBytes *duid)
stop (NMDhcpClient *client, gboolean release)
{
NMDhcpDhclient *self = NM_DHCP_DHCLIENT (client);
NMDhcpDhclientPrivate *priv = NM_DHCP_DHCLIENT_GET_PRIVATE (self);
NM_DHCP_CLIENT_CLASS (nm_dhcp_dhclient_parent_class)->stop (client, release, duid);
NM_DHCP_CLIENT_CLASS (nm_dhcp_dhclient_parent_class)->stop (client, release);
if (priv->conf_file)
if (remove (priv->conf_file) == -1)
@ -610,7 +608,6 @@ stop (NMDhcpClient *client, gboolean release, GBytes *duid)
if (dhclient_start (client,
NULL,
duid,
TRUE,
&rpid,
0,

View file

@ -193,20 +193,20 @@ ip6_start (NMDhcpClient *client,
const char *dhcp_anycast_addr,
const struct in6_addr *ll_addr,
NMSettingIP6ConfigPrivacy privacy,
GBytes *duid,
guint needed_prefixes,
GError **error)
{
nm_utils_error_set_literal (error, NM_UTILS_ERROR_UNKNOWN, "dhcpcanon plugin does not support IPv6");
return FALSE;
}
static void
stop (NMDhcpClient *client, gboolean release, GBytes *duid)
stop (NMDhcpClient *client, gboolean release)
{
NMDhcpDhcpcanon *self = NM_DHCP_DHCPCANON (client);
NMDhcpDhcpcanonPrivate *priv = NM_DHCP_DHCPCANON_GET_PRIVATE (self);
NM_DHCP_CLIENT_CLASS (nm_dhcp_dhcpcanon_parent_class)->stop (client, release, duid);
NM_DHCP_CLIENT_CLASS (nm_dhcp_dhcpcanon_parent_class)->stop (client, release);
if (priv->pid_file) {
if (remove (priv->pid_file) == -1)

View file

@ -187,7 +187,6 @@ ip6_start (NMDhcpClient *client,
const char *dhcp_anycast_addr,
const struct in6_addr *ll_addr,
NMSettingIP6ConfigPrivacy privacy,
GBytes *duid,
guint needed_prefixes,
GError **error)
{
@ -196,12 +195,12 @@ ip6_start (NMDhcpClient *client,
}
static void
stop (NMDhcpClient *client, gboolean release, GBytes *duid)
stop (NMDhcpClient *client, gboolean release)
{
NMDhcpDhcpcd *self = NM_DHCP_DHCPCD (client);
NMDhcpDhcpcdPrivate *priv = NM_DHCP_DHCPCD_GET_PRIVATE (self);
NM_DHCP_CLIENT_CLASS (nm_dhcp_dhcpcd_parent_class)->stop (client, release, duid);
NM_DHCP_CLIENT_CLASS (nm_dhcp_dhcpcd_parent_class)->stop (client, release);
if (priv->pid_file) {
if (remove (priv->pid_file) == -1)

View file

@ -895,7 +895,6 @@ ip6_start (NMDhcpClient *client,
const char *dhcp_anycast_addr,
const struct in6_addr *ll_addr,
NMSettingIP6ConfigPrivacy privacy,
GBytes *duid,
guint needed_prefixes,
GError **error)
{
@ -907,10 +906,13 @@ ip6_start (NMDhcpClient *client,
int r, i;
const guint8 *duid_arr;
gsize duid_len;
GBytes *duid;
g_assert (priv->client4 == NULL);
g_assert (priv->client6 == NULL);
g_return_val_if_fail (duid != NULL, FALSE);
g_return_val_if_fail (!priv->client4, FALSE);
g_return_val_if_fail (!priv->client6, FALSE);
duid = nm_dhcp_client_get_client_id (client);
g_return_val_if_fail (duid, FALSE);
duid_arr = g_bytes_get_data (duid, &duid_len);
if (!duid_arr || duid_len < 2)
@ -1013,13 +1015,13 @@ errout:
}
static void
stop (NMDhcpClient *client, gboolean release, GBytes *duid)
stop (NMDhcpClient *client, gboolean release)
{
NMDhcpSystemd *self = NM_DHCP_SYSTEMD (client);
NMDhcpSystemdPrivate *priv = NM_DHCP_SYSTEMD_GET_PRIVATE (self);
int r = 0;
NM_DHCP_CLIENT_CLASS (nm_dhcp_systemd_parent_class)->stop (client, release, duid);
NM_DHCP_CLIENT_CLASS (nm_dhcp_systemd_parent_class)->stop (client, release);
_LOGT ("dhcp-client%d: stop %p",
priv->client4 ? '4' : '6',