From 97137ec3fcf70ebf57b1c80dcf97ae87eef1c9f3 Mon Sep 17 00:00:00 2001
From: "Nicholas J. Kain" <nicholas@kain.us>
Date: Wed, 30 Mar 2011 10:39:11 -0400
Subject: [PATCH] Handle STRING options properly in fill_options and refactor a
 bit.

---
 ndhc/ifchange.c | 78 ++++++++++++++++++++++++++-----------------------
 ndhc/options.c  |  7 +++--
 2 files changed, 46 insertions(+), 39 deletions(-)

diff --git a/ndhc/ifchange.c b/ndhc/ifchange.c
index cdaa203..d3dc500 100644
--- a/ndhc/ifchange.c
+++ b/ndhc/ifchange.c
@@ -55,22 +55,30 @@ static int sprintip(char *dest, size_t size, char *pre, unsigned char *ip)
 					pre, ip[0], ip[1], ip[2], ip[3]);
 }
 
-/* Fill dest with the text of option 'option'. */
+/* Fill buf with the ifchd command text of option 'option'. */
 /* Returns 0 if successful, -1 if nothing was filled in. */
-static int fill_options(char *dest, unsigned char *option, ssize_t optlen,
+static int fill_options(char *buf, unsigned char *option, ssize_t optlen,
                         struct dhcp_option *type_p, unsigned int maxlen)
 {
-    char *odest;
+    char *obuf = buf;
     enum option_type type = type_p->type;
     ssize_t typelen = option_length(type);
-    uint32_t val_u32;
-    int32_t val_s32;
-    uint16_t val_u16;
-    int16_t val_s16;
+    ssize_t rem = optlen;
     uint8_t code = type_p->code;
 
     if (!option)
         return -1;
+
+    if (type == OPTION_STRING) {
+        buf += snprintf(buf, maxlen, "%s=", type_p->name);
+        if (maxlen < rem + 1)
+            return -1;
+        memcpy(buf, option, rem);
+        buf[rem] = '\0';
+        return 0;
+    }
+
+    // Length and type checking.
     if (optlen != typelen) {
         if (option_valid_list(code)) {
             if ((optlen % typelen)) {
@@ -84,55 +92,53 @@ static int fill_options(char *dest, unsigned char *option, ssize_t optlen,
             return -1;
         }
     }
-    int len = option[-1]; // XXX: WTF ugly as all hell
 
-    odest = dest;
-
-    dest += snprintf(dest, maxlen, "%s=", type_p->name);
+    buf += snprintf(buf, maxlen, "%s=", type_p->name);
 
     for(;;) {
         switch (type) {
             case OPTION_IP: /* Works regardless of host byte order. */
-                dest += sprintip(dest, maxlen - (dest - odest), "", option);
+                buf += sprintip(buf, maxlen - (buf - obuf), "", option);
                 break;
             case OPTION_U8:
-                dest += snprintf(dest, maxlen - (dest - odest),
-								 "%u ", *option);
+                buf += snprintf(buf, maxlen - (buf - obuf), "%u ", *option);
                 break;
-            case OPTION_U16:
+            case OPTION_U16: {
+                uint16_t val_u16;
                 memcpy(&val_u16, option, 2);
-                dest += snprintf(dest, maxlen - (dest - odest),
-								 "%u ", ntohs(val_u16));
+                buf += snprintf(buf, maxlen - (buf - obuf), "%u ",
+                                ntohs(val_u16));
                 break;
-            case OPTION_S16:
+            }
+            case OPTION_S16: {
+                int16_t val_s16;
                 memcpy(&val_s16, option, 2);
-                dest += snprintf(dest, maxlen - (dest - odest),
-								 "%d ", ntohs(val_s16));
+                buf += snprintf(buf, maxlen - (buf - obuf), "%d ",
+                                ntohs(val_s16));
                 break;
-            case OPTION_U32:
+            }
+            case OPTION_U32: {
+                uint32_t val_u32;
                 memcpy(&val_u32, option, 4);
-                dest += snprintf(dest, maxlen - (dest - odest),
-								 "%u ", (uint32_t) ntohl(val_u32));
+                buf += snprintf(buf, maxlen - (buf - obuf), "%u ",
+                                ntohl(val_u32));
                 break;
-            case OPTION_S32:
+            }
+            case OPTION_S32: {
+                int32_t val_s32;
                 memcpy(&val_s32, option, 4);
-                dest += snprintf(dest, maxlen - (dest - odest),
-								 "%d ", (int32_t) ntohl(val_s32));
+                buf += snprintf(buf, maxlen - (buf - obuf), "%d ",
+                                ntohl(val_s32));
                 break;
-            case OPTION_STRING:
-                if ( (maxlen - (dest - odest)) < (unsigned)len)
-                    return -1;
-                memcpy(dest, option, len);
-                dest[len] = '\0';
-                return 0;  /* Short circuit this case */
-            case OPTION_NONE:
+            }
+            default:
                 return 0;
         }
         option += typelen;
-        len -= typelen;
-        if (len <= 0)
+        rem -= typelen;
+        if (rem <= 0)
             break;
-        *(dest++) = ':';
+        *(buf++) = ':';
     }
     return 0;
 }
diff --git a/ndhc/options.c b/ndhc/options.c
index 35d85e8..9b0b715 100644
--- a/ndhc/options.c
+++ b/ndhc/options.c
@@ -81,7 +81,6 @@ uint8_t option_length(enum option_type type)
 {
 	switch (type) {
 		case OPTION_IP: return 4;
-		case OPTION_STRING: return 1; // XXX ?
 		case OPTION_U8: return 1;
 		case OPTION_U16: return 2;
 		case OPTION_S16: return 2;
@@ -150,7 +149,8 @@ unsigned char *alloc_dhcp_client_id_option(unsigned char type,
 	return alloc_option(DHCP_CLIENT_ID, data, sizeof data);
 }
 
-// Worker function for get_option().
+// Worker function for get_option().  Optlen will be set to the length
+// of the option data.
 static uint8_t *do_get_option(uint8_t *buf, ssize_t buflen, int code,
 							  char *overload, ssize_t *optlen)
 {
@@ -194,7 +194,8 @@ static uint8_t *do_get_option(uint8_t *buf, ssize_t buflen, int code,
 	return NULL;
 }
 
-/* Get an option with bounds checking (warning, result is not aligned) */
+// Get an option with bounds checking (warning, result is not aligned)
+// optlen will be equal to the length of the option data.
 uint8_t *get_option(struct dhcpMessage *packet, int code, ssize_t *optlen)
 {
 	uint8_t *option, *buf;