From 9cfcfefd4e1d3b0196004f9c0248565aac7d6713 Mon Sep 17 00:00:00 2001
From: "Nicholas J. Kain" <nicholas@kain.us>
Date: Sat, 11 Jun 2011 04:27:56 -0400
Subject: [PATCH] Consolidate DHCP_MAGIC check for packet.cookie field. Minor
 cosmetic cleanups for DHCP packet receipt handling.

---
 ndhc/dhcpmsg.c |  8 ++------
 ndhc/packet.c  | 43 ++++++++++++++++++++++---------------------
 2 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/ndhc/dhcpmsg.c b/ndhc/dhcpmsg.c
index 2de3514..fcaa8b6 100644
--- a/ndhc/dhcpmsg.c
+++ b/ndhc/dhcpmsg.c
@@ -1,5 +1,5 @@
 /* dhcpmsg.c - dhcp packet generation and sending functions
- * Time-stamp: <2011-05-30 10:43:28 njk>
+ * Time-stamp: <2011-06-11 04:21:26 njk>
  *
  * (c) 2004-2011 Nicholas J. Kain <njkain at gmail dot com>
  * (c) 2001 Russ Dill <Russ.Dill@asu.edu>
@@ -286,10 +286,6 @@ int get_raw_packet(struct dhcpMessage *payload, int fd)
     memcpy(payload, &packet.data,
            len - sizeof packet.ip - sizeof packet.udp);
 
-    if (ntohl(payload->cookie) != DHCP_MAGIC) {
-        log_error("Packet with bad magic number, ignoring");
-        return -2;
-    }
-    log_line("Received valid DHCP message.");
+    log_line("Received a packet via raw socket.");
     return len - sizeof packet.ip - sizeof packet.udp;
 }
diff --git a/ndhc/packet.c b/ndhc/packet.c
index 387a4b4..2479267 100644
--- a/ndhc/packet.c
+++ b/ndhc/packet.c
@@ -1,5 +1,5 @@
 /* packet.c - send and react to DHCP message packets
- * Time-stamp: <2011-06-11 04:07:56 njk>
+ * Time-stamp: <2011-06-11 04:26:22 njk>
  *
  * (c) 2004-2011 Nicholas J. Kain <njkain at gmail dot com>
  * (c) 2001 Russ Dill <Russ.Dill@asu.edu>
@@ -54,11 +54,7 @@ int get_packet(struct dhcpMessage *packet, int fd)
         return -1;
     }
 
-    if (ntohl(packet->cookie) != DHCP_MAGIC) {
-        log_error("Packet with bad magic number, ignoring.");
-        return -2;
-    }
-    log_line("Received a packet");
+    log_line("Received a packet via cooked socket.");
 
     return bytes;
 }
@@ -320,28 +316,34 @@ void handle_packet(struct client_state_t *cs)
     else /* LM_NONE */
         return;
 
-    if (len == -1 && errno != EINTR) {
-        log_error("reopening socket.");
-        change_listen_mode(cs, cs->listenMode); /* just close and reopen */
+    if (len < 0) {
+        // Transient issue handled by packet collection functions.
+        if (len == -2 || (len == -1 && errno == EINTR))
+            return;
+
+        log_error("Error when reading from listening socket: %s.  Reopening listening socket.",
+                  strerror(errno));
+        change_listen_mode(cs, cs->listenMode);
     }
 
-    if (len < 0)
-        return;
-
     if (len < DHCP_SIZE - 308) {
-        log_line("Packet is too short to contain magic cookie -- ignoring");
+        log_line("Packet is too short to contain magic cookie. Ignoring.");
+        return;
+    }
+
+    if (ntohl(packet.cookie) != DHCP_MAGIC) {
+        log_line("Packet with bad magic number. Ignoring.");
         return;
     }
 
     if (packet.xid != cs->xid) {
-        log_line("Ignoring XID %lx (our xid is %lx).",
-                 (uint32_t) packet.xid, cs->xid);
+        log_line("Packet XID %lx does not equal our XID %lx.  Ignoring.",
+                 packet.xid, cs->xid);
         return;
     }
 
-    if ((message = get_option_data(&packet, DHCP_MESSAGE_TYPE, &optlen))
-        == NULL) {
-        log_line("couldnt get option from packet -- ignoring");
+    if (!(message = get_option_data(&packet, DHCP_MESSAGE_TYPE, &optlen))) {
+        log_line("Packet does not specify a DHCP message type. Ignoring.");
         return;
     }
 
@@ -350,9 +352,8 @@ void handle_packet(struct client_state_t *cs)
             init_selecting_packet(cs, &packet, message);
             break;
         case DS_ARP_CHECK:
-            /* We ignore dhcp packets for now.  This state will
-             * be changed by the callback for arp ping.
-             */
+            // We ignore dhcp packets for now.  This state will
+            // be changed by the callback for arp ping.
             break;
         case DS_RENEW_REQUESTED:
         case DS_REQUESTING: