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.

3 files changed, 68 insertions, 52 deletions

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)