dhcpc: cleanup and comments; fix buggy timeout handling in corner cases.

-25 bytes.

1 files changed, 125 insertions, 107 deletions

diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c
index b3b89459e..234ecfd13 100644
--- a/networking/udhcp/dhcpc.c
+++ b/networking/udhcp/dhcpc.c
@@ -24,7 +24,7 @@
  * in the code. Manpage says that struct in_addr has a member of type long (!)
  * which holds IPv4 address, and the struct is passed by value (!!)
  */
-static unsigned timeout;
+static int timeout; /* = 0. Must be signed */
 static uint32_t requested_ip; /* = 0 */
 static uint32_t server_addr;
 static int packet_num; /* = 0 */
@@ -41,7 +41,7 @@ static smallint state;
 
 
 /* just a little helper */
-static void change_mode(int new_mode)
+static void change_listen_mode(int new_mode)
 {
 	DEBUG("entering %s listen mode",
 		new_mode ? (new_mode == 1 ? "kernel" : "raw") : "none");
@@ -59,7 +59,7 @@ static void perform_renew(void)
 	bb_info_msg("Performing a DHCP renew");
 	switch (state) {
 	case BOUND:
-		change_mode(LISTEN_KERNEL);
+		change_listen_mode(LISTEN_KERNEL);
 	case RENEWING:
 	case REBINDING:
 		state = RENEW_REQUESTED;
@@ -68,7 +68,7 @@ static void perform_renew(void)
 		udhcp_run_script(NULL, "deconfig");
 	case REQUESTING:
 	case RELEASED:
-		change_mode(LISTEN_RAW);
+		change_listen_mode(LISTEN_RAW);
 		state = INIT_SELECTING;
 		break;
 	case INIT_SELECTING:
@@ -101,7 +101,7 @@ static void perform_release(void)
 	}
 	bb_info_msg("Entering released state");
 
-	change_mode(LISTEN_NONE);
+	change_listen_mode(LISTEN_NONE);
 	state = RELEASED;
 	timeout = INT_MAX;
 }
@@ -153,10 +153,9 @@ int udhcpc_main(int argc, char **argv)
 	char *str_W;
 #endif
 	uint32_t xid = 0;
-	uint32_t lease = 0; /* can be given as 32-bit quantity */
+	uint32_t lease_seconds = 0; /* can be given as 32-bit quantity */
 	unsigned t1 = 0, t2 = 0; /* what a wonderful names */
-	unsigned start = 0;
-	unsigned now;
+	unsigned timestamp_got_lease = 0; /* for gcc */
 	unsigned opt;
 	int max_fd;
 	int retval;
@@ -324,13 +323,14 @@ int udhcpc_main(int argc, char **argv)
 
 	state = INIT_SELECTING;
 	udhcp_run_script(NULL, "deconfig");
-	change_mode(LISTEN_RAW);
-	tv.tv_sec = 0;
-	goto jump_in;
+	change_listen_mode(LISTEN_RAW);
 
+	/* Main event loop. select() waits on signal pipe and possibly
+	 * on sockfd.
+	 * "continue" statements in code below jump to the top of the loop.
+	 */
 	for (;;) {
-		tv.tv_sec = timeout - monotonic_sec();
- jump_in:
+		tv.tv_sec = timeout;
 		tv.tv_usec = 0;
 
 		if (listen_mode != LISTEN_NONE && sockfd < 0) {
@@ -347,15 +347,19 @@ int udhcpc_main(int argc, char **argv)
 			retval = select(max_fd + 1, &rfds, NULL, NULL, &tv);
 		}
 
-		now = monotonic_sec();
 		if (retval < 0) {
 			/* EINTR? signal was caught, don't panic */
 			if (errno != EINTR) {
 				/* Else: an error occured, panic! */
 				bb_perror_msg_and_die("select");
 			}
-		} else if (retval == 0) {
-			/* timeout dropped to zero */
+			continue;
+		}
+
+		/* If timeout dropped to zero, time to become active:
+		 * resend discover/renew/whatever
+		 */
+		if (retval == 0) {
 			switch (state) {
 			case INIT_SELECTING:
 				if (packet_num < discover_retries) {
@@ -365,23 +369,23 @@ int udhcpc_main(int argc, char **argv)
 					/* send discover packet */
 					send_discover(xid, requested_ip); /* broadcast */
 
-					timeout = now + discover_timeout;
+					timeout = discover_timeout;
 					packet_num++;
-				} else {
-					udhcp_run_script(NULL, "leasefail");
-					if (client_config.background_if_no_lease) {
-						bb_info_msg("No lease, forking to background");
-						client_background();
-					} else if (client_config.abort_if_no_lease) {
-						bb_info_msg("No lease, failing");
-						retval = 1;
-						goto ret;
-					}
-					/* wait to try again */
-					packet_num = 0;
-					timeout = now + tryagain_timeout;
+					continue;
 				}
-				break;
+				udhcp_run_script(NULL, "leasefail");
+				if (client_config.background_if_no_lease) {
+					bb_info_msg("No lease, forking to background");
+					client_background();
+				} else if (client_config.abort_if_no_lease) {
+					bb_info_msg("No lease, failing");
+					retval = 1;
+					goto ret;
+				}
+				/* wait to try again */
+				timeout = tryagain_timeout;
+				packet_num = 0;
+				continue;
 			case RENEW_REQUESTED:
 			case REQUESTING:
 				if (packet_num < discover_retries) {
@@ -390,62 +394,67 @@ int udhcpc_main(int argc, char **argv)
 						send_renew(xid, server_addr, requested_ip); /* unicast */
 					else send_selecting(xid, server_addr, requested_ip); /* broadcast */
 
-					timeout = now + ((packet_num == 2) ? 10 : 2);
+					timeout = ((packet_num == 2) ? 10 : 2);
 					packet_num++;
-				} else {
-					/* timed out, go back to init state */
-					if (state == RENEW_REQUESTED)
-						udhcp_run_script(NULL, "deconfig");
-					state = INIT_SELECTING;
-					timeout = now;
-					packet_num = 0;
-					change_mode(LISTEN_RAW);
+					continue;
 				}
-				break;
+				/* timed out, go back to init state */
+				if (state == RENEW_REQUESTED)
+					udhcp_run_script(NULL, "deconfig");
+				change_listen_mode(LISTEN_RAW);
+				state = INIT_SELECTING;
+				timeout = 0;
+				packet_num = 0;
+				continue;
 			case BOUND:
 				/* Lease is starting to run out, time to enter renewing state */
-				state = RENEWING;
-				change_mode(LISTEN_KERNEL);
+				change_listen_mode(LISTEN_KERNEL);
 				DEBUG("Entering renew state");
+				state = RENEWING;
 				/* fall right through */
 			case RENEWING:
 				/* Either set a new T1, or enter REBINDING state */
-				if ((t2 - t1) <= (lease / 14400 + 1)) {
-					/* timed out, enter rebinding state */
-					state = REBINDING;
-					timeout = now + (t2 - t1);
-					DEBUG("Entering rebinding state");
-				} else {
+				if ((t2 - t1) > (lease_seconds / (4*60*60) + 1)) {
 					/* send a request packet */
 					send_renew(xid, server_addr, requested_ip); /* unicast */
-					t1 = (t2 - t1) / 2 + t1;
-					timeout = start + t1;
+					t1 += (t2 - t1) / 2;
+					timeout = t1 - ((int)monotonic_sec() - timestamp_got_lease);
+					continue;
 				}
-				break;
+				/* Timed out, enter rebinding state */
+				DEBUG("Entering rebinding state");
+				state = REBINDING;
+				timeout = (t2 - t1);
+				continue;
 			case REBINDING:
-				/* Either set a new T2, or enter INIT state */
-				if ((lease - t2) <= (lease / 14400 + 1)) {
-					/* timed out, enter init state */
-					state = INIT_SELECTING;
-					bb_info_msg("Lease lost, entering init state");
-					udhcp_run_script(NULL, "deconfig");
-					timeout = now;
-					packet_num = 0;
-					change_mode(LISTEN_RAW);
-				} else {
+				/* Lease is *really* about to run out,
+				 * try to find DHCP server using broadcast */
+				if ((lease_seconds - t2) > (lease_seconds / (4*60*60) + 1)) {
 					/* send a request packet */
 					send_renew(xid, 0, requested_ip); /* broadcast */
-					t2 = (lease - t2) / 2 + t2;
-					timeout = start + t2;
+					t2 += (lease_seconds - t2) / 2;
+					timeout = t2 - ((int)monotonic_sec() - timestamp_got_lease);
+					continue;
 				}
-				break;
-			case RELEASED:
-				/* yah, I know, *you* say it would never happen */
-				timeout = INT_MAX;
-				break;
+				/* Timed out, enter init state */
+				bb_info_msg("Lease lost, entering init state");
+				udhcp_run_script(NULL, "deconfig");
+				change_listen_mode(LISTEN_RAW);
+				state = INIT_SELECTING;
+				timeout = 0;
+				packet_num = 0;
+				continue;
+			/* case RELEASED: */
 			}
-		} else if (listen_mode != LISTEN_NONE && FD_ISSET(sockfd, &rfds)) {
-			/* a packet is ready, read it */
+			/* yah, I know, *you* say it would never happen */
+			timeout = INT_MAX;
+			continue; /* back to main loop */
+		}
+
+		/* select() didn't timeout, something did happen. */
+		/* Is is a packet? */
+		if (listen_mode != LISTEN_NONE && FD_ISSET(sockfd, &rfds)) {
+			/* A packet is ready, read it */
 
 			if (listen_mode == LISTEN_KERNEL)
 				len = udhcp_get_packet(&packet, sockfd);
@@ -453,7 +462,7 @@ int udhcpc_main(int argc, char **argv)
 
 			if (len == -1 && errno != EINTR) {
 				DEBUG("error on read, %s, reopening socket", strerror(errno));
-				change_mode(listen_mode); /* just close and reopen */
+				change_listen_mode(listen_mode); /* just close and reopen */
 			}
 			if (len < 0) continue;
 
@@ -471,7 +480,7 @@ int udhcpc_main(int argc, char **argv)
 
 			message = get_option(&packet, DHCP_MESSAGE_TYPE);
 			if (message == NULL) {
-				bb_error_msg("cannot get option from packet - ignoring");
+				bb_error_msg("cannot get message type from packet - ignoring");
 				continue;
 			}
 
@@ -479,22 +488,24 @@ int udhcpc_main(int argc, char **argv)
 			case INIT_SELECTING:
 				/* Must be a DHCPOFFER to one of our xid's */
 				if (*message == DHCPOFFER) {
+			/* TODO: why we don't just fetch server's IP from IP header? */
 					temp = get_option(&packet, DHCP_SERVER_ID);
-					if (temp) {
-						/* can be misaligned, thus memcpy */
-						memcpy(&server_addr, temp, 4);
-						xid = packet.xid;
-						requested_ip = packet.yiaddr;
-
-						/* enter requesting state */
-						state = REQUESTING;
-						timeout = now;
-						packet_num = 0;
-					} else {
+					if (!temp) {
 						bb_error_msg("no server ID in message");
+						continue;
+						/* still selecting - this server looks bad */
 					}
+					/* can be misaligned, thus memcpy */
+					memcpy(&server_addr, temp, 4);
+					xid = packet.xid;
+					requested_ip = packet.yiaddr;
+
+					/* enter requesting state */
+					state = REQUESTING;
+					timeout = 0;
+					packet_num = 0;
 				}
-				break;
+				continue;
 			case RENEW_REQUESTED:
 			case REQUESTING:
 			case RENEWING:
@@ -503,13 +514,12 @@ int udhcpc_main(int argc, char **argv)
 					temp = get_option(&packet, DHCP_LEASE_TIME);
 					if (!temp) {
 						bb_error_msg("no lease time with ACK, using 1 hour lease");
-						lease = 60 * 60;
+						lease_seconds = 60 * 60;
 					} else {
 						/* can be misaligned, thus memcpy */
-						memcpy(&lease, temp, 4);
-						lease = ntohl(lease);
+						memcpy(&lease_seconds, temp, 4);
+						lease_seconds = ntohl(lease_seconds);
 					}
-
 #if ENABLE_FEATURE_UDHCPC_ARPING
 					if (opt & OPT_a) {
 						if (!arpping(packet.yiaddr,
@@ -517,37 +527,37 @@ int udhcpc_main(int argc, char **argv)
 							    client_config.arp,
 							    client_config.interface)
 						) {
-							bb_info_msg("offered address is in use,"
-								" declining");
+							bb_info_msg("offered address is in use "
+								"(got ARP reply), declining");
 							send_decline(xid, server_addr);
 
 							if (state != REQUESTING)
 								udhcp_run_script(NULL, "deconfig");
+							change_listen_mode(LISTEN_RAW);
 							state = INIT_SELECTING;
 							requested_ip = 0;
+							timeout = decline_wait;
 							packet_num = 0;
-							change_mode(LISTEN_RAW);
-							timeout = now + decline_wait;
-							break;
+							continue; /* back to main loop */
 						}
 					}
 #endif
 					/* enter bound state */
-					t1 = lease / 2;
+					t1 = lease_seconds / 2;
 
 					/* little fixed point for n * .875 */
-					t2 = (lease * 7) >> 3;
+					t2 = (lease_seconds * 7) >> 3;
 					temp_addr.s_addr = packet.yiaddr;
 					bb_info_msg("Lease of %s obtained, lease time %u",
-						inet_ntoa(temp_addr), (unsigned)lease);
-					start = now;
-					timeout = start + t1;
+						inet_ntoa(temp_addr), (unsigned)lease_seconds);
+					timestamp_got_lease = monotonic_sec();
+					timeout = t1;
 					requested_ip = packet.yiaddr;
 					udhcp_run_script(&packet,
 						   ((state == RENEWING || state == REBINDING) ? "renew" : "bound"));
 
 					state = BOUND;
-					change_mode(LISTEN_NONE);
+					change_listen_mode(LISTEN_NONE);
 					if (client_config.quit_after_lease) {
 						if (client_config.release_on_quit)
 							perform_release();
@@ -556,23 +566,30 @@ int udhcpc_main(int argc, char **argv)
 					if (!client_config.foreground)
 						client_background();
 
-				} else if (*message == DHCPNAK) {
+					continue; /* back to main loop */
+				}
+				if (*message == DHCPNAK) {
 					/* return to init state */
 					bb_info_msg("Received DHCP NAK");
 					udhcp_run_script(&packet, "nak");
 					if (state != REQUESTING)
 						udhcp_run_script(NULL, "deconfig");
+					change_listen_mode(LISTEN_RAW);
+					sleep(3); /* avoid excessive network traffic */
 					state = INIT_SELECTING;
-					timeout = now;
 					requested_ip = 0;
+					timeout = 0;
 					packet_num = 0;
-					change_mode(LISTEN_RAW);
-					sleep(3); /* avoid excessive network traffic */
 				}
-				break;
+				continue;
 			/* case BOUND, RELEASED: - ignore all packets */
 			}
-		} else {
+			continue; /* back to main loop */
+		}
+
+		/* select() didn't timeout, something did happen.
+		 * But it wasn't a packet. It's a signal pipe then. */
+		{
 			int signo = udhcp_sp_read(&rfds);
 			switch (signo) {
 			case SIGUSR1:
@@ -588,7 +605,8 @@ int udhcpc_main(int argc, char **argv)
 				goto ret0;
 			}
 		}
-	} /* for (;;) */
+	} /* for (;;) - main loop ends */
+
  ret0:
 	retval = 0;
  ret: