From a114066685b6a996c3f0ae914ee32587e8f59f2f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 23 May 2016 16:27:05 +0200 Subject: sd-network: fix up assertion chaos assert_return() should only be used to validate user-facing parameters and state, assert() should be used for checking our own internal state and parameters. --- src/libsystemd-network/sd-dhcp-client.c | 41 +++++++++++++++++------------ src/libsystemd-network/sd-dhcp6-client.c | 35 +++++++++++++++++-------- src/libsystemd-network/sd-ndisc.c | 44 +++++++++++++++----------------- 3 files changed, 68 insertions(+), 52 deletions(-) (limited to 'src') diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index fab9f3f088..09e174cc01 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -151,10 +151,10 @@ int sd_dhcp_client_set_request_option(sd_dhcp_client *client, uint8_t option) { size_t i; assert_return(client, -EINVAL); - assert_return (IN_SET(client->state, DHCP_STATE_INIT, - DHCP_STATE_STOPPED), -EBUSY); + assert_return(IN_SET(client->state, DHCP_STATE_INIT, DHCP_STATE_STOPPED), -EBUSY); switch(option) { + case SD_DHCP_OPTION_PAD: case SD_DHCP_OPTION_OVERLOAD: case SD_DHCP_OPTION_MESSAGE_TYPE: @@ -182,9 +182,9 @@ int sd_dhcp_client_set_request_option(sd_dhcp_client *client, uint8_t option) { int sd_dhcp_client_set_request_address( sd_dhcp_client *client, const struct in_addr *last_addr) { + assert_return(client, -EINVAL); - assert_return (IN_SET(client->state, DHCP_STATE_INIT, - DHCP_STATE_STOPPED), -EBUSY); + assert_return(IN_SET(client->state, DHCP_STATE_INIT, DHCP_STATE_STOPPED), -EBUSY); if (last_addr) client->last_addr = last_addr->s_addr; @@ -230,8 +230,7 @@ int sd_dhcp_client_set_mac( return 0; if (!IN_SET(client->state, DHCP_STATE_INIT, DHCP_STATE_STOPPED)) { - log_dhcp_client(client, "Changing MAC address on running DHCP " - "client, restarting"); + log_dhcp_client(client, "Changing MAC address on running DHCP client, restarting"); need_restart = true; client_stop(client, SD_DHCP_CLIENT_EVENT_STOP); } @@ -283,14 +282,17 @@ int sd_dhcp_client_set_client_id( assert_return(data_len > 0 && data_len <= MAX_CLIENT_ID_LEN, -EINVAL); switch (type) { + case ARPHRD_ETHER: if (data_len != ETH_ALEN) return -EINVAL; break; + case ARPHRD_INFINIBAND: if (data_len != INFINIBAND_ALEN) return -EINVAL; break; + default: break; } @@ -434,14 +436,14 @@ int sd_dhcp_client_set_mtu(sd_dhcp_client *client, uint32_t mtu) { int sd_dhcp_client_get_lease(sd_dhcp_client *client, sd_dhcp_lease **ret) { assert_return(client, -EINVAL); - assert_return(ret, -EINVAL); if (client->state != DHCP_STATE_BOUND && client->state != DHCP_STATE_RENEWING && client->state != DHCP_STATE_REBINDING) return -EADDRNOTAVAIL; - *ret = client->lease; + if (ret) + *ret = client->lease; return 0; } @@ -454,8 +456,7 @@ static void client_notify(sd_dhcp_client *client, int event) { static int client_initialize(sd_dhcp_client *client) { assert_return(client, -EINVAL); - client->receive_message = - sd_event_source_unref(client->receive_message); + client->receive_message = sd_event_source_unref(client->receive_message); client->fd = asynchronous_close(client->fd); @@ -750,8 +751,9 @@ static int client_send_request(sd_dhcp_client *client) { size_t optoffset, optlen; int r; - r = client_message_init(client, &request, DHCP_REQUEST, - &optlen, &optoffset); + assert(client); + + r = client_message_init(client, &request, DHCP_REQUEST, &optlen, &optoffset); if (r < 0) return r; @@ -848,18 +850,23 @@ static int client_send_request(sd_dhcp_client *client) { return r; switch (client->state) { + case DHCP_STATE_REQUESTING: log_dhcp_client(client, "REQUEST (requesting)"); break; + case DHCP_STATE_INIT_REBOOT: log_dhcp_client(client, "REQUEST (init-reboot)"); break; + case DHCP_STATE_RENEWING: log_dhcp_client(client, "REQUEST (renewing)"); break; + case DHCP_STATE_REBINDING: log_dhcp_client(client, "REQUEST (rebinding)"); break; + default: log_dhcp_client(client, "REQUEST (invalid)"); break; @@ -891,6 +898,7 @@ static int client_timeout_resend( goto error; switch (client->state) { + case DHCP_STATE_RENEWING: time_left = (client->lease->t2 - client->lease->t1) / 2; @@ -1103,8 +1111,7 @@ static int client_start_delayed(sd_dhcp_client *client) { assert_return(client->ifindex > 0, -EINVAL); assert_return(client->fd < 0, -EBUSY); assert_return(client->xid == 0, -EINVAL); - assert_return(client->state == DHCP_STATE_INIT || - client->state == DHCP_STATE_INIT_REBOOT, -EBUSY); + assert_return(IN_SET(client->state, DHCP_STATE_INIT, DHCP_STATE_INIT_REBOOT), -EBUSY); client->xid = random_u32(); @@ -1150,6 +1157,8 @@ static int client_timeout_t2(sd_event_source *s, uint64_t usec, void *userdata) DHCP_CLIENT_DONT_DESTROY(client); int r; + assert(client); + client->receive_message = sd_event_source_unref(client->receive_message); client->fd = asynchronous_close(client->fd); @@ -1821,8 +1830,7 @@ int sd_dhcp_client_detach_event(sd_dhcp_client *client) { } sd_event *sd_dhcp_client_get_event(sd_dhcp_client *client) { - if (!client) - return NULL; + assert_return(client, NULL); return client->event; } @@ -1884,7 +1892,6 @@ int sd_dhcp_client_new(sd_dhcp_client **ret) { client->mtu = DHCP_DEFAULT_MIN_SIZE; client->req_opts_size = ELEMENTSOF(default_req_opts); - client->req_opts = memdup(default_req_opts, client->req_opts_size); if (!client->req_opts) return -ENOMEM; diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c index 15667c26d7..203deaa50d 100644 --- a/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/libsystemd-network/sd-dhcp6-client.c @@ -255,6 +255,7 @@ int sd_dhcp6_client_set_request_option(sd_dhcp6_client *client, uint16_t option) assert_return(client->state == DHCP6_STATE_STOPPED, -EBUSY); switch(option) { + case SD_DHCP6_OPTION_DNS_SERVERS: case SD_DHCP6_OPTION_DOMAIN_LIST: case SD_DHCP6_OPTION_SNTP_SERVERS: @@ -296,15 +297,18 @@ static void client_notify(sd_dhcp6_client *client, int event) { } static void client_set_lease(sd_dhcp6_client *client, sd_dhcp6_lease *lease) { + assert(client); + if (client->lease) { dhcp6_lease_clear_timers(&client->lease->ia); sd_dhcp6_lease_unref(client->lease); } + client->lease = lease; } static int client_reset(sd_dhcp6_client *client) { - assert_return(client, -EINVAL); + assert(client); client_set_lease(client, NULL); @@ -352,6 +356,8 @@ static int client_send_message(sd_dhcp6_client *client, usec_t time_now) { usec_t elapsed_usec; be16_t elapsed_time; + assert(client); + len = sizeof(DHCP6Message) + optlen; message = malloc0(len); @@ -453,9 +459,9 @@ static int client_send_message(sd_dhcp6_client *client, usec_t time_now) { static int client_timeout_t2(sd_event_source *s, uint64_t usec, void *userdata) { sd_dhcp6_client *client = userdata; - assert_return(s, -EINVAL); - assert_return(client, -EINVAL); - assert_return(client->lease, -EINVAL); + assert(s); + assert(client); + assert(client->lease); client->lease->ia.timeout_t2 = sd_event_source_unref(client->lease->ia.timeout_t2); @@ -470,9 +476,9 @@ 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_dhcp6_client *client = userdata; - assert_return(s, -EINVAL); - assert_return(client, -EINVAL); - assert_return(client->lease, -EINVAL); + assert(s); + assert(client); + assert(client->lease); client->lease->ia.timeout_t1 = sd_event_source_unref(client->lease->ia.timeout_t1); @@ -689,6 +695,11 @@ static int client_parse_message( bool clientid = false; be32_t iaid_lease; + assert(client); + assert(message); + assert(len >= sizeof(DHCP6Message)); + assert(lease); + option = (uint8_t *)message + sizeof(DHCP6Message); len -= sizeof(DHCP6Message); @@ -833,9 +844,12 @@ static int client_parse_message( } static int client_receive_reply(sd_dhcp6_client *client, DHCP6Message *reply, size_t len) { - int r; _cleanup_(sd_dhcp6_lease_unrefp) sd_dhcp6_lease *lease = NULL; bool rapid_commit; + int r; + + assert(client); + assert(reply); if (reply->type != DHCP6_REPLY) return 0; @@ -864,9 +878,9 @@ static int client_receive_reply(sd_dhcp6_client *client, DHCP6Message *reply, si } static int client_receive_advertise(sd_dhcp6_client *client, DHCP6Message *advertise, size_t len) { - int r; _cleanup_(sd_dhcp6_lease_unrefp) sd_dhcp6_lease *lease = NULL; uint8_t pref_advertise = 0, pref_lease = 0; + int r; if (advertise->type != DHCP6_ADVERTISE) return 0; @@ -1251,8 +1265,7 @@ int sd_dhcp6_client_detach_event(sd_dhcp6_client *client) { } sd_event *sd_dhcp6_client_get_event(sd_dhcp6_client *client) { - if (!client) - return NULL; + assert_return(client, NULL); return client->event; } diff --git a/src/libsystemd-network/sd-ndisc.c b/src/libsystemd-network/sd-ndisc.c index fe9ba43167..06afafd2c7 100644 --- a/src/libsystemd-network/sd-ndisc.c +++ b/src/libsystemd-network/sd-ndisc.c @@ -128,13 +128,15 @@ static int ndisc_prefix_new(sd_ndisc *nd, NDiscPrefix **ret) { return 0; } -int sd_ndisc_set_callback(sd_ndisc *nd, - sd_ndisc_router_callback_t router_callback, - sd_ndisc_prefix_onlink_callback_t prefix_onlink_callback, - sd_ndisc_prefix_autonomous_callback_t prefix_autonomous_callback, - sd_ndisc_callback_t callback, - void *userdata) { - assert(nd); +int sd_ndisc_set_callback( + sd_ndisc *nd, + sd_ndisc_router_callback_t router_callback, + sd_ndisc_prefix_onlink_callback_t prefix_onlink_callback, + sd_ndisc_prefix_autonomous_callback_t prefix_autonomous_callback, + sd_ndisc_callback_t callback, + void *userdata) { + + assert_return(nd, -EINVAL); nd->router_callback = router_callback; nd->prefix_onlink_callback = prefix_onlink_callback; @@ -154,7 +156,7 @@ int sd_ndisc_set_ifindex(sd_ndisc *nd, int ifindex) { } int sd_ndisc_set_mac(sd_ndisc *nd, const struct ether_addr *mac_addr) { - assert(nd); + assert_return(nd, -EINVAL); if (mac_addr) memcpy(&nd->mac_addr, mac_addr, sizeof(nd->mac_addr)); @@ -193,7 +195,7 @@ int sd_ndisc_detach_event(sd_ndisc *nd) { } sd_event *sd_ndisc_get_event(sd_ndisc *nd) { - assert(nd); + assert_return(nd, NULL); return nd->event; } @@ -245,14 +247,13 @@ sd_ndisc *sd_ndisc_unref(sd_ndisc *nd) { int sd_ndisc_new(sd_ndisc **ret) { _cleanup_(sd_ndisc_unrefp) sd_ndisc *nd = NULL; - assert(ret); + assert_return(ret, -EINVAL); nd = new0(sd_ndisc, 1); if (!nd) return -ENOMEM; nd->n_ref = 1; - nd->ifindex = -1; nd->fd = -1; @@ -272,7 +273,6 @@ int sd_ndisc_get_mtu(sd_ndisc *nd, uint32_t *mtu) { return -ENOMSG; *mtu = nd->mtu; - return 0; } @@ -281,8 +281,8 @@ static int prefix_match(const struct in6_addr *prefix, uint8_t prefixlen, uint8_t addr_prefixlen) { uint8_t bytes, mask, len; - assert_return(prefix, -EINVAL); - assert_return(addr, -EINVAL); + assert(prefix); + assert(addr); len = MIN(prefixlen, addr_prefixlen); @@ -414,8 +414,8 @@ static int ndisc_ra_parse(sd_ndisc *nd, struct nd_router_advert *ra, ssize_t len void *opt; struct nd_opt_hdr *opt_hdr; - assert_return(nd, -EINVAL); - assert_return(ra, -EINVAL); + assert(nd); + assert(ra); len -= sizeof(*ra); if (len < NDISC_OPT_LEN_UNITS) { @@ -668,14 +668,10 @@ int sd_ndisc_stop(sd_ndisc *nd) { int sd_ndisc_router_discovery_start(sd_ndisc *nd) { int r; - assert(nd); - assert(nd->event); - - if (nd->state != NDISC_STATE_IDLE) - return -EBUSY; - - if (nd->ifindex < 1) - return -EINVAL; + assert_return(nd, -EINVAL); + assert_return(nd->event, -EINVAL); + assert_return(nd->ifindex > 0, -EINVAL); + assert_return(nd->state == NDISC_STATE_IDLE, -EBUSY); r = icmp6_bind_router_solicitation(nd->ifindex); if (r < 0) -- cgit v1.2.3-54-g00ecf