From 574cc928887851269c5919123dbdf8e1b2713b23 Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Thu, 22 May 2014 15:18:28 +0200 Subject: [PATCH] sd-dhcp-client: return NULL from _unref() like the other sd-* libraries Let's keep this behavior consistent across our libraries. In order to keep the refcounting working, a DONT_DESTROY macro similar to the one in sd-bus was introduced. --- src/libsystemd-network/dhcp-internal.h | 10 +++++ src/libsystemd-network/sd-dhcp-client.c | 59 +++++++++++-------------- 2 files changed, 35 insertions(+), 34 deletions(-) diff --git a/src/libsystemd-network/dhcp-internal.h b/src/libsystemd-network/dhcp-internal.h index 03cc8243a..6f6f1218d 100644 --- a/src/libsystemd-network/dhcp-internal.h +++ b/src/libsystemd-network/dhcp-internal.h @@ -27,6 +27,7 @@ #include "socket-util.h" +#include "sd-dhcp-client.h" #include "dhcp-protocol.h" int dhcp_network_bind_raw_socket(int index, union sockaddr_union *link, uint32_t xid); @@ -56,4 +57,13 @@ void dhcp_packet_append_ip_headers(DHCPPacket *packet, be32_t source_addr, int dhcp_packet_verify_headers(DHCPPacket *packet, size_t len, bool checksum); +DEFINE_TRIVIAL_CLEANUP_FUNC(sd_dhcp_client*, sd_dhcp_client_unref); +#define _cleanup_dhcp_client_unref_ _cleanup_(sd_dhcp_client_unrefp) + +/* If we are invoking callbacks of a dhcp-client, ensure unreffing the + * client from the callback doesn't destroy the object we are working + * on */ +#define DHCP_CLIENT_DONT_DESTROY(client) \ + _cleanup_dhcp_client_unref_ _unused_ sd_dhcp_client *_dont_destroy_##client = sd_dhcp_client_ref(client) + #define log_dhcp_client(client, fmt, ...) log_meta(LOG_DEBUG, __FILE__, __LINE__, __func__, "DHCP CLIENT (0x%x): " fmt, client->xid, ##__VA_ARGS__) diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index c1af6df13..1603c4122 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -82,7 +82,7 @@ static int client_receive_message_raw(sd_event_source *s, int fd, uint32_t revents, void *userdata); static int client_receive_message_udp(sd_event_source *s, int fd, uint32_t revents, void *userdata); -static sd_dhcp_client *client_stop(sd_dhcp_client *client, int error); +static void client_stop(sd_dhcp_client *client, int error); int sd_dhcp_client_set_callback(sd_dhcp_client *client, sd_dhcp_client_cb_t cb, void *userdata) { @@ -153,6 +153,7 @@ int sd_dhcp_client_set_index(sd_dhcp_client *client, int interface_index) { int sd_dhcp_client_set_mac(sd_dhcp_client *client, const struct ether_addr *addr) { + DHCP_CLIENT_DONT_DESTROY(client); bool need_restart = false; assert_return(client, -EINVAL); @@ -165,12 +166,9 @@ int sd_dhcp_client_set_mac(sd_dhcp_client *client, log_dhcp_client(client, "Changing MAC address on running DHCP " "client, restarting"); need_restart = true; - client = client_stop(client, DHCP_EVENT_STOP); + client_stop(client, DHCP_EVENT_STOP); } - if (!client) - return 0; - memcpy(&client->client_id.mac_addr, addr, ETH_ALEN); client->client_id.type = 0x01; @@ -194,14 +192,9 @@ int sd_dhcp_client_get_lease(sd_dhcp_client *client, sd_dhcp_lease **ret) { return 0; } -static sd_dhcp_client *client_notify(sd_dhcp_client *client, int event) { - if (client->cb) { - client = sd_dhcp_client_ref(client); +static void client_notify(sd_dhcp_client *client, int event) { + if (client->cb) client->cb(client, event, client->userdata); - client = sd_dhcp_client_unref(client); - } - - return client; } static int client_initialize(sd_dhcp_client *client) { @@ -229,8 +222,8 @@ static int client_initialize(sd_dhcp_client *client) { return 0; } -static sd_dhcp_client *client_stop(sd_dhcp_client *client, int error) { - assert_return(client, NULL); +static void client_stop(sd_dhcp_client *client, int error) { + assert(client); if (error < 0) log_dhcp_client(client, "STOPPED: %s", strerror(-error)); @@ -248,12 +241,9 @@ static sd_dhcp_client *client_stop(sd_dhcp_client *client, int error) { } } - client = client_notify(client, error); - - if (client) - client_initialize(client); + client_notify(client, error); - return client; + client_initialize(client); } static int client_message_init(sd_dhcp_client *client, DHCPPacket **ret, @@ -530,6 +520,7 @@ static int client_start(sd_dhcp_client *client); static int client_timeout_resend(sd_event_source *s, uint64_t usec, void *userdata) { sd_dhcp_client *client = userdata; + DHCP_CLIENT_DONT_DESTROY(client); usec_t next_timeout = 0; uint64_t time_now; uint32_t time_left; @@ -737,13 +728,14 @@ static int client_start(sd_dhcp_client *client) { static int client_timeout_expire(sd_event_source *s, uint64_t usec, void *userdata) { sd_dhcp_client *client = userdata; + DHCP_CLIENT_DONT_DESTROY(client); log_dhcp_client(client, "EXPIRED"); - client = client_notify(client, DHCP_EVENT_EXPIRED); + client_notify(client, DHCP_EVENT_EXPIRED); /* lease was lost, start over if not freed or stopped in callback */ - if (client && client->state != DHCP_STATE_STOPPED) { + if (client->state != DHCP_STATE_STOPPED) { client_initialize(client); client_start(client); } @@ -753,6 +745,7 @@ static int client_timeout_expire(sd_event_source *s, uint64_t usec, static int client_timeout_t2(sd_event_source *s, uint64_t usec, void *userdata) { sd_dhcp_client *client = userdata; + DHCP_CLIENT_DONT_DESTROY(client); int r; client->receive_message = sd_event_source_unref(client->receive_message); @@ -774,6 +767,7 @@ static int client_timeout_t2(sd_event_source *s, uint64_t usec, void *userdata) static int client_timeout_t1(sd_event_source *s, uint64_t usec, void *userdata) { sd_dhcp_client *client = userdata; + DHCP_CLIENT_DONT_DESTROY(client); int r; client->state = DHCP_STATE_RENEWING; @@ -1045,6 +1039,7 @@ static int client_set_lease_timeouts(sd_dhcp_client *client) { static int client_handle_message(sd_dhcp_client *client, DHCPMessage *message, int len) { + DHCP_CLIENT_DONT_DESTROY(client); int r = 0, notify_event = 0; assert(client); @@ -1154,9 +1149,8 @@ static int client_handle_message(sd_dhcp_client *client, DHCPMessage *message, goto error; if (notify_event) { - client = client_notify(client, notify_event); - if (!client || - client->state == DHCP_STATE_STOPPED) + client_notify(client, notify_event); + if (client->state == DHCP_STATE_STOPPED) return 0; } @@ -1303,10 +1297,12 @@ int sd_dhcp_client_start(sd_dhcp_client *client) { } int sd_dhcp_client_stop(sd_dhcp_client *client) { + DHCP_CLIENT_DONT_DESTROY(client); + assert_return(client, -EINVAL); - if (client_stop(client, DHCP_EVENT_STOP)) - client->state = DHCP_STATE_STOPPED; + client_stop(client, DHCP_EVENT_STOP); + client->state = DHCP_STATE_STOPPED; return 0; } @@ -1355,7 +1351,7 @@ sd_dhcp_client *sd_dhcp_client_ref(sd_dhcp_client *client) { sd_dhcp_client *sd_dhcp_client_unref(sd_dhcp_client *client) { if (client && REFCNT_DEC(client->n_ref) <= 0) { - log_dhcp_client(client, "UNREF"); + log_dhcp_client(client, "FREE"); client_initialize(client); @@ -1368,18 +1364,13 @@ sd_dhcp_client *sd_dhcp_client_unref(sd_dhcp_client *client) { free(client->req_opts); free(client); - - return NULL; } - return client; + return NULL; } -DEFINE_TRIVIAL_CLEANUP_FUNC(sd_dhcp_client*, sd_dhcp_client_unref); -#define _cleanup_dhcp_client_free_ _cleanup_(sd_dhcp_client_unrefp) - int sd_dhcp_client_new(sd_dhcp_client **ret) { - _cleanup_dhcp_client_free_ sd_dhcp_client *client = NULL; + _cleanup_dhcp_client_unref_ sd_dhcp_client *client = NULL; assert_return(ret, -EINVAL); -- 2.30.2