udhcp: dname_dec may return NULL, account for that case

Other random cleanips included...

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

1 files changed, 39 insertions, 34 deletions

diff --git a/networking/udhcp/domain_codec.c b/networking/udhcp/domain_codec.c
index 6f051c4..45354e7 100644
--- a/networking/udhcp/domain_codec.c
+++ b/networking/udhcp/domain_codec.c
@@ -25,16 +25,9 @@
  */
 char* FAST_FUNC dname_dec(const uint8_t *cstr, int clen, const char *pre)
 {
-	const uint8_t *c;
-	int crtpos, retpos, depth, plen = 0, len = 0;
+	char *ret = ret; /* for compiler */
 	char *dst = NULL;
 
-	if (!cstr)
-		return NULL;
-
-	if (pre)
-		plen = strlen(pre);
-
 	/* We make two passes over the cstr string. First, we compute
 	 * how long the resulting string would be. Then we allocate a
 	 * new buffer of the required length, and fill it in with the
@@ -42,59 +35,71 @@ char* FAST_FUNC dname_dec(const uint8_t *cstr, int clen, const char *pre)
 	 * having to deal with requiring callers to supply their own
 	 * buffer, then having to check if it's sufficiently large, etc.
 	 */
-
-	while (!dst) {
-
-		if (len > 0) {	/* second pass? allocate dst buffer and copy pre */
-			dst = xmalloc(len + plen);
-			memcpy(dst, pre, plen);
-		}
+	while (1) {
+		/* note: "return NULL" below are leak-safe since
+		 * dst isn't yet allocated */
+		const uint8_t *c;
+		unsigned crtpos, retpos, depth, len;
 
 		crtpos = retpos = depth = len = 0;
-
 		while (crtpos < clen) {
 			c = cstr + crtpos;
 
-			if ((*c & NS_CMPRSFLGS) != 0) {	/* pointer */
-				if (crtpos + 2 > clen)		/* no offset to jump to? abort */
+			if (*c & NS_CMPRSFLGS) {
+				/* pointer */
+				if (crtpos + 2 > clen) /* no offset to jump to? abort */
 					return NULL;
-				if (retpos == 0)			/* toplevel? save return spot */
+				if (retpos == 0) /* toplevel? save return spot */
 					retpos = crtpos + 2;
 				depth++;
-				crtpos = ((*c & 0x3f) << 8) | (*(c + 1) & 0xff); /* jump */
-			} else if (*c) {			/* label */
-				if (crtpos + *c + 1 > clen)		/* label too long? abort */
+				crtpos = ((c[0] & 0x3f) << 8) | (c[1] & 0xff); /* jump */
+			} else if (*c) {
+				/* label */
+				if (crtpos + *c + 1 > clen) /* label too long? abort */
 					return NULL;
 				if (dst)
-					memcpy(dst + plen + len, c + 1, *c);
+					memcpy(dst + len, c + 1, *c);
 				len += *c + 1;
 				crtpos += *c + 1;
 				if (dst)
-					*(dst + plen + len - 1) = '.';
-			} else {					/* null: end of current domain name */
-				if (retpos == 0) {			/* toplevel? keep going */
+					dst[len - 1] = '.';
+			} else {
+				/* null: end of current domain name */
+				if (retpos == 0) {
+					/* toplevel? keep going */
 					crtpos++;
-				} else {					/* return to toplevel saved spot */
+				} else {
+					/* return to toplevel saved spot */
 					crtpos = retpos;
 					retpos = depth = 0;
 				}
 				if (dst)
-					*(dst + plen + len - 1) = ' ';
+					dst[len - 1] = ' ';
 			}
 
-			if (depth > NS_MAXDNSRCH || /* too many jumps? abort, it's a loop */
-				len > NS_MAXDNAME * NS_MAXDNSRCH) /* result too long? abort */
+			if (depth > NS_MAXDNSRCH /* too many jumps? abort, it's a loop */
+			 || len > NS_MAXDNAME * NS_MAXDNSRCH /* result too long? abort */
+			) {
 				return NULL;
+			}
 		}
 
-		if (!len)			/* expanded string has 0 length? abort */
+		if (!len) /* expanded string has 0 length? abort */
 			return NULL;
 
-		if (dst)
-			*(dst + plen + len - 1) = '\0';
+		if (!dst) { /* first pass? */
+			/* allocate dst buffer and copy pre */
+			unsigned plen = strlen(pre);
+			ret = dst = xmalloc(plen + len);
+			memcpy(dst, pre, plen);
+			dst += plen;
+		} else {
+			dst[len - 1] = '\0';
+			break;
+		}
 	}
 
-	return dst;
+	return ret;
 }
 
 /* Convert a domain name (src) from human-readable "foo.blah.com" format into