From: David Härdeman <david@hardeman.nu>
Date: Wed, 8 Oct 2025 15:28:39 +0000 (+0200)
Subject: dhcpv4: dhcpv4_lease() - simplification
X-Git-Url: http://git.openwrt.org/?a=commitdiff_plain;h=2c1e6e7c3ebf7255c4c48fda75007a4ddee979eb;p=project%2Fodhcpd.git

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 <david@hardeman.nu>
Link: https://github.com/openwrt/odhcpd/pull/286
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---

diff --git a/src/dhcpv4.c b/src/dhcpv4.c
index 88892e2..93e7562 100644
--- 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;
 }