Cleanup some more code. Eliminate dhcp(), bootp(), parse_options() by

moving the minimal code into do_packet(). Eliminate repeated code for
checking the client hardware address and the reject list by putting
those checks into do_packet as well.

No functional change, just much easier to read.

ok stevesk@

3 files changed, 83 insertions, 134 deletions

diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c
index b6ce542cb39..b8da27e6e4f 100644
--- a/sbin/dhclient/dhclient.c
+++ b/sbin/dhclient/dhclient.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dhclient.c,v 1.104 2007/01/16 20:22:20 krw Exp $	*/
+/*	$OpenBSD: dhclient.c,v 1.105 2007/01/25 01:21:04 krw Exp $	*/
 
 /*
  * Copyright 2004 Henning Brauer <henning@openbsd.org>
@@ -576,12 +576,7 @@ dhcpack(struct iaddr client_addr, struct option_data *options)
 {
 	struct client_lease *lease;
 
-	/* If we're not receptive to an offer right now, or if the offer
-	   has an unrecognizable transaction id, then just drop it. */
-	if (client->xid != client->packet.xid ||
-	    (ifi->hw_address.hlen != client->packet.hlen) ||
-	    (memcmp(ifi->hw_address.haddr,
-	    client->packet.chaddr, client->packet.hlen)))
+	if (client->xid != client->packet.xid)
 		return;
 
 	if (client->state != S_REBOOTING &&
@@ -715,62 +710,6 @@ state_bound(void)
 }
 
 void
-bootp(struct iaddr client_addr, struct option_data *options)
-{
-	struct iaddrlist *ap;
-
-	if (client->packet.op != BOOTREPLY)
-		return;
-
-	/* If there's a reject list, make sure this packet's sender isn't
-	   on it. */
-	for (ap = config->reject_list;
-	    ap; ap = ap->next) {
-		if (addr_eq(client_addr, ap->addr)) {
-			note("BOOTREPLY from %s rejected.", piaddr(ap->addr));
-			return;
-		}
-	}
-	dhcpoffer(client_addr, options);
-}
-
-void
-dhcp(struct iaddr client_addr, struct option_data *options)
-{
-	struct iaddrlist *ap;
-	void (*handler)(struct iaddr, struct option_data *);
-	char *type;
-
-	switch (options[DHO_DHCP_MESSAGE_TYPE].data[0]) {
-	case DHCPOFFER:
-		handler = dhcpoffer;
-		type = "DHCPOFFER";
-		break;
-	case DHCPNAK:
-		handler = dhcpnak;
-		type = "DHCPNACK";
-		break;
-	case DHCPACK:
-		handler = dhcpack;
-		type = "DHCPACK";
-		break;
-	default:
-		return;
-	}
-
-	/* If there's a reject list, make sure this packet's sender isn't
-	   on it. */
-	for (ap = config->reject_list;
-	    ap; ap = ap->next) {
-		if (addr_eq(client_addr, ap->addr)) {
-			note("%s from %s rejected.", type, piaddr(ap->addr));
-			return;
-		}
-	}
-	(*handler)(client_addr, options);
-}
-
-void
 dhcpoffer(struct iaddr client_addr, struct option_data *options)
 {
 	struct client_lease *lease, *lp;
@@ -779,17 +718,13 @@ dhcpoffer(struct iaddr client_addr, struct option_data *options)
 	char *name = options[DHO_DHCP_MESSAGE_TYPE].len ? "DHCPOFFER" :
 	    "BOOTREPLY";
 
-	/* If we're not receptive to an offer right now, or if the offer
-	   has an unrecognizable transaction id, then just drop it. */
-	if (client->state != S_SELECTING ||
-	    client->xid != client->packet.xid ||
-	    (ifi->hw_address.hlen != client->packet.hlen) ||
-	    (memcmp(ifi->hw_address.haddr,
-	    client->packet.chaddr, client->packet.hlen)))
+	if (client->xid != client->packet.xid)
 		return;
 
-	note("%s from %s", name, piaddr(client_addr));
+	if (client->state != S_SELECTING)
+		return;
 
+	note("%s from %s", name, piaddr(client_addr));
 
 	/* If this lease doesn't supply the minimum required parameters,
 	   blow it off. */
@@ -971,12 +906,7 @@ packet_to_lease(struct iaddr client_addr, struct option_data *options)
 void
 dhcpnak(struct iaddr client_addr, struct option_data *options)
 {
-	/* If we're not receptive to an offer right now, or if the offer
-	   has an unrecognizable transaction id, then just drop it. */
-	if (client->xid != client->packet.xid ||
-	    (ifi->hw_address.hlen != client->packet.hlen) ||
-	    (memcmp(ifi->hw_address.haddr,
-	    client->packet.chaddr, client->packet.hlen)))
+	if (client->xid != client->packet.xid)
 		return;
 
 	if (client->state != S_REBOOTING &&
diff --git a/sbin/dhclient/dhcpd.h b/sbin/dhclient/dhcpd.h
index dc1a412e93c..0feca0fcbe5 100644
--- a/sbin/dhclient/dhcpd.h
+++ b/sbin/dhclient/dhcpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dhcpd.h,v 1.62 2007/01/16 20:22:20 krw Exp $	*/
+/*	$OpenBSD: dhcpd.h,v 1.63 2007/01/25 01:21:04 krw Exp $	*/
 
 /*
  * Copyright (c) 2004 Henning Brauer <henning@openbsd.org>
@@ -169,7 +169,6 @@ struct client_state {
 	struct string_list	 *medium;
 	struct dhcp_packet	  packet;
 	int			  packet_length;
-	int			  options_valid;
 	struct iaddr		  requested_address;
 	char			**scriptEnv;
 	int			  scriptEnvsize;
@@ -333,9 +332,6 @@ struct client_lease *packet_to_lease(struct iaddr, struct option_data *);
 void go_daemon(void);
 void client_location_changed(void);
 
-void bootp(struct iaddr, struct option_data *);
-void dhcp(struct iaddr, struct option_data *);
-
 /* packet.c */
 void assemble_hw_header(unsigned char *, int *, struct hardware *);
 void assemble_udp_ip_header(unsigned char *, int *, u_int32_t, u_int32_t,
diff --git a/sbin/dhclient/options.c b/sbin/dhclient/options.c
index 28320e3e67c..b6b3094ded2 100644
--- a/sbin/dhclient/options.c
+++ b/sbin/dhclient/options.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: options.c,v 1.32 2007/01/16 20:22:20 krw Exp $	*/
+/*	$OpenBSD: options.c,v 1.33 2007/01/25 01:21:04 krw Exp $	*/
 
 /* DHCP options parsing and reassembly. */
 
@@ -44,52 +44,14 @@
 
 #include "dhcpd.h"
 
-void	parse_options(struct option_data *);
-void	parse_option_buffer(struct option_data *, unsigned char *, int);
-
-/*
- * Parse all available options out of the specified packet.
- */
-void
-parse_options(struct option_data *options)
-{
-	/* If we don't see the magic cookie, there's nothing to parse. */
-	if (memcmp(&client->packet.options, DHCP_OPTIONS_COOKIE, 4)) {
-		client->options_valid = 1;
-		return;
-	}
-
-	/*
-	 * Go through the options field, up to the end of the packet or
-	 * the End field.
-	 */
-	parse_option_buffer(options, &client->packet.options[4],
-	    sizeof(client->packet.options) - 4);
-
-	/*
-	 * If we parsed a DHCP Option Overload option, parse more
-	 * options out of the buffer(s) containing them.
-	 */
-	if (client->options_valid && options[DHO_DHCP_MESSAGE_TYPE].data &&
-	    options[DHO_DHCP_OPTION_OVERLOAD].data) {
-		if (options[DHO_DHCP_OPTION_OVERLOAD].data[0] & 1)
-			parse_option_buffer(options,
-			    (unsigned char *)client->packet.file,
-			    sizeof(client->packet.file));
-		if (client->options_valid &&
-		    options[DHO_DHCP_OPTION_OVERLOAD].data[0] & 2)
-			parse_option_buffer(options,
-			    (unsigned char *)client->packet.sname,
-			    sizeof(client->packet.sname));
-	}
-}
+int parse_option_buffer(struct option_data *, unsigned char *, int);
 
 /*
  * Parse options out of the specified buffer, storing addresses of
  * option values in options and setting client->options_valid if
  * no errors are encountered.
  */
-void
+int
 parse_option_buffer(struct option_data *options, unsigned char *buffer,
     int length)
 {
@@ -122,8 +84,7 @@ parse_option_buffer(struct option_data *options, unsigned char *buffer,
 			warning("option %s (%d) larger than buffer.",
 			    dhcp_options[code].name, len);
 			warning("rejecting bogus offer.");
-			client->options_valid = 0;
-			return;
+			return (0);
 		}
 		/*
 		 * If we haven't seen this option before, just make
@@ -160,7 +121,8 @@ parse_option_buffer(struct option_data *options, unsigned char *buffer,
 		}
 		s += len + 2;
 	}
-	client->options_valid = 1;
+
+	return (1);
 }
 
 /*
@@ -455,23 +417,84 @@ void
 do_packet(int len, unsigned int from_port, struct iaddr from,
     struct hardware *hfrom)
 {
+	struct dhcp_packet *packet = &client->packet;
 	struct option_data options[256];
-	int i;
+	struct iaddrlist *ap;
+	void (*handler)(struct iaddr, struct option_data *);
+	char *type;
+	int i, options_valid = 1;
 
-	if (client->packet.hlen > sizeof(client->packet.chaddr)) {
+	if (packet->hlen > sizeof(packet->chaddr)) {
 		note("Discarding packet with invalid hlen.");
 		return;
 	}
 
-	memset(&options, 0, sizeof(options));
+	/*
+	 * Silently drop the packet if the client hardware address in the
+	 * packet is not the hardware address of the interface being managed.
+	 */
+	if ((ifi->hw_address.hlen != packet->hlen) ||
+	    (memcmp(ifi->hw_address.haddr, packet->chaddr, packet->hlen)))
+		return;
+
+	memset(options, 0, sizeof(options));
+
+	if (memcmp(&packet->options, DHCP_OPTIONS_COOKIE, 4) == 0) {
+		/* Parse the BOOTP/DHCP options field. */
+		options_valid = parse_option_buffer(options,
+		    &packet->options[4], sizeof(packet->options) - 4);
+
+		/* Only DHCP packets have overload areas for options. */ 
+		if (options_valid &&
+		    options[DHO_DHCP_MESSAGE_TYPE].data &&
+		    options[DHO_DHCP_OPTION_OVERLOAD].data) {
+			if (options[DHO_DHCP_OPTION_OVERLOAD].data[0] & 1)
+				options_valid = parse_option_buffer(options,
+				    (unsigned char *)packet->file,
+				    sizeof(packet->file));
+			if (options_valid &&
+			    options[DHO_DHCP_OPTION_OVERLOAD].data[0] & 2)
+				options_valid = parse_option_buffer(options,
+				    (unsigned char *)packet->sname,
+				    sizeof(packet->sname));
+		}
+	}
+
+	type = "";
+	handler = NULL;
 
-	parse_options(options);
 	if (options[DHO_DHCP_MESSAGE_TYPE].data) {
-		dhcp(from, options);
-	} else if (client->options_valid)
-		bootp(from, options);
+		/* Always try a DHCP packet, even if a bad option was seen. */
+		switch (options[DHO_DHCP_MESSAGE_TYPE].data[0]) {
+		case DHCPOFFER:
+			handler = dhcpoffer;
+			type = "DHCPOFFER";
+			break;
+		case DHCPNAK:
+			handler = dhcpnak;
+			type = "DHCPNACK";
+			break;
+		case DHCPACK:
+			handler = dhcpack;
+			type = "DHCPACK";
+			break;
+		default:
+			break;
+		}
+	} else if (options_valid && packet->op == BOOTREPLY) {
+		handler = dhcpoffer;
+		type = "BOOTREPLY";
+	}
+
+	for (ap = config->reject_list; ap && handler; ap = ap->next)
+		if (addr_eq(from, ap->addr)) {
+			note("%s from %s rejected.", type, piaddr(from));
+			handler = NULL;
+		}
+
+	if (handler)	
+		(*handler)(from, options);
 
-	/* Free the data associated with the options. */
 	for (i = 0; i < 256; i++)
 		if (options[i].len && options[i].data)
 			free(options[i].data);