dhcpc: fix the case where we might add extra space at the end of envvar.

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

2 files changed, 13 insertions, 9 deletions

diff --git a/networking/udhcp/common.c b/networking/udhcp/common.c
index ba41905..2e61136 100644
--- a/networking/udhcp/common.c
+++ b/networking/udhcp/common.c
@@ -36,6 +36,9 @@ const struct dhcp_optflag dhcp_optflags[] = {
 	{ OPTION_STRING                           , 0x11 }, /* DHCP_ROOT_PATH     */
 	{ OPTION_U8                               , 0x17 }, /* DHCP_IP_TTL        */
 	{ OPTION_U16                              , 0x1a }, /* DHCP_MTU           */
+//TODO: why do we request DHCP_BROADCAST? Can't we assume that
+//in the unlikely case it is different from typical N.N.255.255,
+//server would let us know anyway?
 	{ OPTION_IP                   | OPTION_REQ, 0x1c }, /* DHCP_BROADCAST     */
 	{ OPTION_IP_PAIR | OPTION_LIST            , 0x21 }, /* DHCP_ROUTES        */
 	{ OPTION_STRING                           , 0x28 }, /* DHCP_NIS_DOMAIN    */
diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c
index d9269f2..d67769e 100644
--- a/networking/udhcp/dhcpc.c
+++ b/networking/udhcp/dhcpc.c
@@ -173,16 +173,13 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_
 	dest += sprintf(ret, "%s=", opt_name);
 
 	while (len >= optlen) {
-		unsigned ip_ofs = 0;
-
 		switch (type) {
+		case OPTION_IP:
 		case OPTION_IP_PAIR:
 			dest += sprint_nip(dest, "", option);
-			*dest++ = '/';
-			ip_ofs = 4;
-			/* fall through */
-		case OPTION_IP:
-			dest += sprint_nip(dest, "", option + ip_ofs);
+			if (type == OPTION_IP)
+				break;
+			dest += sprint_nip(dest, "/", option + 4);
 			break;
 //		case OPTION_BOOLEAN:
 //			dest += sprintf(dest, *option ? "yes" : "no");
@@ -204,10 +201,14 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_
 			dest += sprintf(dest, type == OPTION_U32 ? "%lu" : "%ld", (unsigned long) ntohl(val_u32));
 			break;
 		}
+		/* Note: options which use 'return' instead of 'break'
+		 * (for example, OPTION_STRING) skip the code which handles
+		 * the case of list of options.
+		 */
 		case OPTION_STRING:
 			memcpy(dest, option, len);
 			dest[len] = '\0';
-			return ret;	 /* Short circuit this case */
+			return ret;
 		case OPTION_STATIC_ROUTES: {
 			/* Option binary format:
 			 * mask [one byte, 0..32]
@@ -347,7 +348,7 @@ static NOINLINE char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_
 // TODO: it can be a list only if (optflag->flags & OPTION_LIST).
 // Should we bail out/warn if we see multi-ip option which is
 // not allowed to be such (for example, DHCP_BROADCAST)? -
-		if (len <= 0 /* || !(optflag->flags & OPTION_LIST) */)
+		if (len < optlen /* || !(optflag->flags & OPTION_LIST) */)
 			break;
 		*dest++ = ' ';
 		*dest = '\0';