dhcpv4: dhcpv4_lease() - simplification

Reduce indentation, check some conditions and bail early, break it into clearer
chunks and add some comments.

Signed-off-by: David Härdeman <[email protected]>
Link: https://github.com/openwrt/odhcpd/pull/286
Signed-off-by: Álvaro Fernández Rojas <[email protected]>

diff --git a/src/dhcpv4.c b/src/dhcpv4.c
index 88892e2376e19fa81555b28d93dfcc5e8d4255f2..93e756271259801c304d87fc5539823b8964f8a2 100644 (file)
--- a/src/dhcpv4.c
+++ b/src/dhcpv4.c
@@ -504,95 +504,99 @@ dhcpv4_lease(struct interface *iface, enum dhcpv4_msg msg, const uint8_t *mac,
                _fallthrough;
 
        case DHCPV4_MSG_REQUEST:
-               bool assigned = !!a;
-
-               if (!a) {
-                       if (!iface->no_dynamic_dhcp || l) {
-                               /* Create new binding */
-                               a = alloc_assignment(0);
-                               if (!a) {
-                                       warn("Failed to alloc assignment on interface %s",
-                                            iface->ifname);
-                                       return NULL;
-                               }
-                               memcpy(a->hwaddr, mac, sizeof(a->hwaddr));
-                               /* Set valid time to 0 for static lease indicating */
-                               /* infinite lifetime otherwise current time        */
-                               a->valid_until = l ? 0 : now;
-                               a->dhcp_free_cb = dhcpv4_free_assignment;
-                               a->iface = iface;
-                               a->flags = OAF_DHCPV4;
-                               a->addr = l ? l->ipaddr : INADDR_ANY;
+               if (!a && iface->no_dynamic_dhcp && !l)
+                       return NULL;
 
-                               assigned = dhcpv4_assign(iface, a, reqaddr);
+               /* Old assignment, but with an address that is out-of-scope? */
+               if (a && ((a->addr & iface->dhcpv4_mask.s_addr) !=
+                    (iface->dhcpv4_start_ip.s_addr & iface->dhcpv4_mask.s_addr)) &&
+                   !(a->flags & OAF_STATIC)) {
+                       /* Try to reassign to an address that is in-scope */
+                       list_del_init(&a->head);
+                       a->addr = INADDR_ANY;
+                       if (!dhcpv4_assign(iface, a, reqaddr)) {
+                               free_assignment(a);
+                               a = NULL;
+                               break;
+                       }
+               }
 
-                               if (l) {
-                                       a->flags |= OAF_STATIC;
+               if (!a) {
+                       /* Create new binding */
+                       a = alloc_assignment(0);
+                       if (!a) {
+                               warn("Failed to allocate memory for DHCPv4 lease on interface %s", iface->ifname);
+                               return NULL;
+                       }
 
-                                       if (l->hostname)
-                                               a->hostname = strdup(l->hostname);
+                       memcpy(a->hwaddr, mac, sizeof(a->hwaddr));
+                       a->dhcp_free_cb = dhcpv4_free_assignment;
+                       a->iface = iface;
+                       a->flags = OAF_DHCPV4;
 
-                                       if (l->leasetime)
-                                               a->leasetime = l->leasetime;
+                       /* static lease => infinite (0), else a placeholder */
+                       a->valid_until = l ? 0 : now;
+                       a->addr = l ? l->ipaddr : INADDR_ANY;
 
-                                       list_add(&a->lease_list, &l->assignments);
-                                       a->lease = l;
-                               }
+                       if (!dhcpv4_assign(iface, a, reqaddr)) {
+                               free_assignment(a);
+                               return NULL;
                        }
-               } else if (((a->addr & iface->dhcpv4_mask.s_addr) !=
-                           (iface->dhcpv4_start_ip.s_addr & iface->dhcpv4_mask.s_addr)) &&
-                           !(a->flags & OAF_STATIC)) {
-                       list_del_init(&a->head);
-                       a->addr = INADDR_ANY;
 
-                       assigned = dhcpv4_assign(iface, a, reqaddr);
-               }
+                       if (l) {
+                               a->flags |= OAF_STATIC;
 
-               if (assigned) {
-                       uint32_t my_leasetime;
+                               if (l->hostname)
+                                       a->hostname = strdup(l->hostname);
 
-                       if (a->leasetime)
-                               my_leasetime = a->leasetime;
-                       else
-                               my_leasetime = iface->dhcp_leasetime;
+                               if (l->leasetime)
+                                       a->leasetime = l->leasetime;
 
-                       if ((*leasetime == 0) || (my_leasetime < *leasetime))
-                               *leasetime = my_leasetime;
+                               list_add(&a->lease_list, &l->assignments);
+                               a->lease = l;
+                       }
+               }
 
-                       if (msg == DHCPV4_MSG_DISCOVER) {
-                               a->flags &= ~OAF_BOUND;
+               uint32_t my_leasetime;
+               if (a->leasetime)
+                       my_leasetime = a->leasetime;
+               else
+                       my_leasetime = iface->dhcp_leasetime;
 
-                               *incl_fr_opt = accept_fr_nonce;
-                               a->valid_until = now;
-                       } else {
-                               if ((!(a->flags & OAF_STATIC) || !a->hostname) && hostname_len > 0) {
-                                       a->hostname = realloc(a->hostname, hostname_len + 1);
-                                       if (a->hostname) {
-                                               memcpy(a->hostname, hostname, hostname_len);
-                                               a->hostname[hostname_len] = 0;
-
-                                               if (odhcpd_valid_hostname(a->hostname))
-                                                       a->flags &= ~OAF_BROKEN_HOSTNAME;
-                                               else
-                                                       a->flags |= OAF_BROKEN_HOSTNAME;
-                                       }
-                               }
+               if ((*leasetime == 0) || (my_leasetime < *leasetime))
+                       *leasetime = my_leasetime;
 
-                               if (!(a->flags & OAF_BOUND)) {
-                                       a->accept_fr_nonce = accept_fr_nonce;
-                                       *incl_fr_opt = accept_fr_nonce;
-                                       odhcpd_urandom(a->key, sizeof(a->key));
-                                       a->flags |= OAF_BOUND;
-                               } else
-                                       *incl_fr_opt = false;
+               if (msg == DHCPV4_MSG_DISCOVER) {
+                       a->flags &= ~OAF_BOUND;
+                       *incl_fr_opt = accept_fr_nonce;
+                       a->valid_until = now;
+                       break;
+               }
+
+               if ((!(a->flags & OAF_STATIC) || !a->hostname) && hostname_len > 0) {
+                       a->hostname = realloc(a->hostname, hostname_len + 1);
+                       if (a->hostname) {
+                               memcpy(a->hostname, hostname, hostname_len);
+                               a->hostname[hostname_len] = 0;
 
-                               a->valid_until = ((*leasetime == UINT32_MAX) ? 0 : (time_t)(now + *leasetime));
+                               if (odhcpd_valid_hostname(a->hostname))
+                                       a->flags &= ~OAF_BROKEN_HOSTNAME;
+                               else
+                                       a->flags |= OAF_BROKEN_HOSTNAME;
                        }
-               } else if (!assigned && a) {
-                       /* Cleanup failed assignment */
-                       free_assignment(a);
-                       a = NULL;
                }
+
+               if (!(a->flags & OAF_BOUND)) {
+                       /* This is the client's first request for the address */
+                       a->accept_fr_nonce = accept_fr_nonce;
+                       *incl_fr_opt = accept_fr_nonce;
+                       odhcpd_urandom(a->key, sizeof(a->key));
+                       a->flags |= OAF_BOUND;
+               } else {
+                       *incl_fr_opt = false;
+               }
+
+               a->valid_until = ((*leasetime == UINT32_MAX) ? 0 : (time_t)(now + *leasetime));
                break;
 
        default:
@@ -600,7 +604,6 @@ dhcpv4_lease(struct interface *iface, enum dhcpv4_msg msg, const uint8_t *mac,
        }
 
        dhcpv6_ia_write_statefile();
-
        return a;
 }