Redo the parsing of numbers to improve the error messages and

make the code more readable. And prepare for some new things
that will need to be parsed.

ok dlg@

3 files changed, 179 insertions, 181 deletions

diff --git a/sbin/dhclient/clparse.c b/sbin/dhclient/clparse.c
index 5fcba8f2380..a14109fe6ee 100644
--- a/sbin/dhclient/clparse.c
+++ b/sbin/dhclient/clparse.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: clparse.c,v 1.76 2014/01/19 04:36:04 krw Exp $	*/
+/*	$OpenBSD: clparse.c,v 1.77 2014/01/19 08:25:54 krw Exp $	*/
 
 /* Parser for dhclient config and lease files. */
 
@@ -269,25 +269,24 @@ parse_X(FILE *cfile, u_int8_t *buf, int max)
 	token = peek_token(&val, cfile);
 	if (token == TOK_NUMBER_OR_NAME || token == TOK_NUMBER) {
 		len = 0;
-		do {
-			token = next_token(&val, cfile);
-			if (token != TOK_NUMBER && token !=
-			    TOK_NUMBER_OR_NAME) {
-				parse_warn("expecting hexadecimal constant.");
-				if (token != ';')
-					skip_to_semi(cfile);
-				return (-1);
-			}
-			convert_num(&buf[len], val, 16, 8);
-			if (len++ > max) {
-				parse_warn("hexadecimal constant too long.");
-				skip_to_semi(cfile);
-				return (-1);
-			}
-			token = peek_token(&val, cfile);
-			if (token == ':')
-				token = next_token(&val, cfile);
-		} while (token == ':');
+		for (token = ':'; token == ':';
+		     token = next_token(NULL, cfile)) {
+			if (!parse_hex(cfile, &buf[len]))
+				break;
+			if (++len == max)
+				break;
+			if (peek_token(NULL, cfile) == ';')
+				return (len);
+		}
+		if (token != ':') {
+			parse_warn("expecting ':'.");
+			skip_to_semi(cfile);
+			return (-1);
+		} else {
+			parse_warn("expecting hex octet.");
+			skip_to_semi(cfile);
+			return (-1);
+		}
 	} else if (token == TOK_STRING) {
 		token = next_token(&val, cfile);
 		len = strlen(val);
@@ -298,6 +297,7 @@ parse_X(FILE *cfile, u_int8_t *buf, int max)
 		}
 		memcpy(buf, val, len + 1);
 	} else {
+		token = next_token(NULL, cfile);
 		parse_warn("expecting string or hexadecimal data");
 		if (token != ';')
 			skip_to_semi(cfile);
@@ -675,35 +675,63 @@ alloc:
 				memcpy(&hunkbuf[hunkix], dp, len);
 				hunkix += len;
 				break;
+ 			case 'l':	/* Signed 32-bit integer. */
+				if (!parse_decimal(cfile, buf, *fmt)) {
+					parse_warn("expecting signed 32-bit "
+					    "integer.");
+					skip_to_semi(cfile);
+					return (-1);
+				}
+				len = 4;
+				dp = buf;
+				goto alloc;
 			case 'L':	/* Unsigned 32-bit integer. */
-			case 'l':	/* Signed 32-bit integer. */
-				token = next_token(&val, cfile);
-				if (token != TOK_NUMBER) {
-need_number:
-					parse_warn("expecting number.");
-					if (token != ';')
-						skip_to_semi(cfile);
+				if (!parse_decimal(cfile, buf, *fmt)) {
+					parse_warn("expecting unsigned 32-bit "
+					    "integer.");
+					skip_to_semi(cfile);
 					return (-1);
 				}
-				convert_num(buf, val, 0, 32);
 				len = 4;
 				dp = buf;
 				goto alloc;
 			case 's':	/* Signed 16-bit integer. */
+				if (!parse_decimal(cfile, buf, *fmt)) {
+					parse_warn("expecting signed 16-bit "
+					    "integer.");
+					skip_to_semi(cfile);
+					return (-1);
+				}
+				len = 2;
+				dp = buf;
+				goto alloc;
 			case 'S':	/* Unsigned 16-bit integer. */
-				token = next_token(&val, cfile);
-				if (token != TOK_NUMBER)
-					goto need_number;
-				convert_num(buf, val, 0, 16);
+				if (!parse_decimal(cfile, buf, *fmt)) {
+					parse_warn("expecting unsigned 16-bit "
+					    "integer.");
+					skip_to_semi(cfile);
+					return (-1);
+				}
 				len = 2;
 				dp = buf;
 				goto alloc;
 			case 'b':	/* Signed 8-bit integer. */
+				if (!parse_decimal(cfile, buf, *fmt)) {
+					parse_warn("expecting signed 8-bit "
+					    "integer.");
+					skip_to_semi(cfile);
+					return (-1);
+				}
+				len = 1;
+				dp = buf;
+				goto alloc;
 			case 'B':	/* Unsigned 8-bit integer. */
-				token = next_token(&val, cfile);
-				if (token != TOK_NUMBER)
-					goto need_number;
-				convert_num(buf, val, 0, 8);
+				if (!parse_decimal(cfile, buf, *fmt)) {
+					parse_warn("expecting unsigned 8-bit "
+					    "integer.");
+					skip_to_semi(cfile);
+					return (-1);
+				}
 				len = 1;
 				dp = buf;
 				goto alloc;
@@ -761,7 +789,6 @@ parse_reject_statement(FILE *cfile)
 
 	do {
 		if (!parse_ip_addr(cfile, &addr)) {
-			parse_warn("expecting IP address.");
 			skip_to_semi(cfile);
 			return;
 		}
diff --git a/sbin/dhclient/dhcpd.h b/sbin/dhclient/dhcpd.h
index ed31b63cfcf..61459665e36 100644
--- a/sbin/dhclient/dhcpd.h
+++ b/sbin/dhclient/dhcpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dhcpd.h,v 1.131 2014/01/19 04:36:04 krw Exp $	*/
+/*	$OpenBSD: dhcpd.h,v 1.132 2014/01/19 08:25:54 krw Exp $	*/
 
 /*
  * Copyright (c) 2004 Henning Brauer <henning@openbsd.org>
@@ -237,7 +237,8 @@ char *parse_string(FILE *);
 int parse_ip_addr(FILE *, struct in_addr *);
 void parse_ethernet(FILE *, struct ether_addr *);
 void parse_lease_time(FILE *, time_t *);
-void convert_num(unsigned char *, char *, int, int);
+int parse_decimal(FILE *, unsigned char *, char);
+int parse_hex(FILE *, unsigned char *);
 time_t parse_date(FILE *);
 
 /* bpf.c */
diff --git a/sbin/dhclient/parse.c b/sbin/dhclient/parse.c
index 6d7d2626d70..3e0b3f4c349 100644
--- a/sbin/dhclient/parse.c
+++ b/sbin/dhclient/parse.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.c,v 1.34 2014/01/19 04:36:04 krw Exp $	*/
+/*	$OpenBSD: parse.c,v 1.35 2014/01/19 08:25:54 krw Exp $	*/
 
 /* Common parser code for dhcpd and dhclient. */
 
@@ -40,11 +40,11 @@
  * Enterprises, see ``http://www.vix.com''.
  */
 
+#include <stdint.h>
+
 #include "dhcpd.h"
 #include "dhctoken.h"
 
-int parse_numeric_aggregate(FILE *, unsigned char *, int, int, int);
-
 /*
  * Skip to the semicolon ending the current statement.   If we encounter
  * braces, the matching closing brace terminates the statement.   If we
@@ -139,7 +139,30 @@ parse_string(FILE *cfile)
 int
 parse_ip_addr(FILE *cfile, struct in_addr *addr)
 {
-	return (parse_numeric_aggregate(cfile, (char *)addr, 4, '.', 10));
+	struct in_addr buf;
+	int len, token;
+
+	token = '.';
+	len = 0;
+	for (token = '.'; token == '.'; token = next_token(NULL, cfile)) {
+		if (!parse_decimal(cfile, (unsigned char *)&buf + len, 'B'))
+			break;
+		if (++len == sizeof(buf))
+			break;
+	}
+
+	if (len == 4) {
+		memcpy(addr, &buf, sizeof(*addr));
+		return (1);
+	} else if (token != '.') {
+		parse_warn("expecting '.'.");
+		skip_to_semi(cfile);
+		return (0);
+	} else {
+		parse_warn("expecting decimal octet.");
+		skip_to_semi(cfile);
+		return (0);
+	}
 }
 
 /*
@@ -148,7 +171,8 @@ parse_ip_addr(FILE *cfile, struct in_addr *addr)
 void
 parse_ethernet(FILE *cfile, struct ether_addr *hardware)
 {
-	int token;
+	struct ether_addr buf;
+	int len, token;
 
 	token = next_token(NULL, cfile);
 	if (token != TOK_ETHERNET) {
@@ -158,11 +182,24 @@ parse_ethernet(FILE *cfile, struct ether_addr *hardware)
 		return;
 	}
 
-	if (parse_numeric_aggregate(cfile, hardware->ether_addr_octet,
-	    ETHER_ADDR_LEN, ':', 16) == 0)
-		return;
+	len = 0;
+	for (token = ':'; token == ':'; token = next_token(NULL, cfile)) {
+		if (!parse_hex(cfile, &buf.ether_addr_octet[len]))
+			break;
+		if (++len == sizeof(buf.ether_addr_octet))
+			break;
+	}
 
-	parse_semi(cfile);
+	if (len == 6) {
+		if (parse_semi(cfile))
+		    memcpy(hardware, &buf, sizeof(*hardware));
+	} else if (token != ':') {
+		parse_warn("expecting ':'.");
+		skip_to_semi(cfile);
+	} else {
+		parse_warn("expecting hex octet.");
+		skip_to_semi(cfile);
+	}
 }
 
 /*
@@ -171,160 +208,93 @@ parse_ethernet(FILE *cfile, struct ether_addr *hardware)
 void
 parse_lease_time(FILE *cfile, time_t *timep)
 {
-	char *val;
-	uint32_t value;
-	int token;
+	u_int32_t value;
 
-	token = next_token(&val, cfile);
-	if (token != TOK_NUMBER) {
-		parse_warn("expecting numeric lease time.");
-		if (token != ';')
-			skip_to_semi(cfile);
+	if (!parse_decimal(cfile, (char *)&value, 'L')) {
+		parse_warn("expecting unsigned 32-bit decimal value.");
+		skip_to_semi(cfile);
 		return;
 	}
-	convert_num((unsigned char *)&value, val, 10, 32);
-	/* Unswap the number - convert_num returns stuff in NBO. */
-	*timep = ntohl(value);
+	
+	*timep = betoh32(value);
 
 	parse_semi(cfile);
 }
 
-/*
- * Parse a sequence of numbers separated by the token specified in separator.
- * Exactly max numbers are expected.
- */
 int
-parse_numeric_aggregate(FILE *cfile, unsigned char *buf, int max, int separator,
-    int base)
+parse_decimal(FILE *cfile, unsigned char *buf, char fmt)
 {
 	char *val;
-	int token, count;
-
-	if (buf == NULL || max == 0)
-		error("no space for numeric aggregate");
-
-	for (count = 0; count < max; count++, buf++) {
-		if (count && (peek_token(&val, cfile) == separator))
-			token = next_token(&val, cfile);
+	const char *errstr;
+	int bytes, token;
+	long long numval, low, high;
 
-		token = next_token(&val, cfile);
-
-		if (token == TOK_NUMBER || (base == 16 && token ==
-			TOK_NUMBER_OR_NAME))
-			/* XXX Need to check if conversion was successful. */
-			convert_num(buf, val, base, 8);
-		else
-			break;
+	token = next_token(&val, cfile);
+	
+	switch (fmt) {
+	case 'l':	/* Signed 32-bit integer. */
+		low = INT32_MIN;
+		high = INT32_MAX;
+		bytes = 4;
+		break;
+	case 'L':	/* Unsigned 32-bit integer. */
+		low = 0;
+		high = UINT32_MAX;
+		bytes = 4;
+		break;
+	case 's':	/* Signed 16-bit integer. */
+		low = INT16_MIN;
+		high = INT16_MAX;
+		bytes = 2;
+		break;
+	case 'S':	/* Unsigned 16-bit integer. */
+		low = 0;
+		high = UINT16_MAX;
+		bytes = 2;
+		break;
+	case 'b':	/* Signed 8-bit integer. */
+		low = INT8_MIN;
+		high = INT8_MAX;
+		bytes = 1;
+		break;
+	case 'B':	/* Unsigned 8-bit integer. */
+		low = 0;
+		high = UINT8_MAX;
+		bytes = 1;
+		break;
+	default:
+		return (0);
 	}
 
-	if (count < max) {
-		parse_warn("numeric aggregate too short.");
+	numval = strtonum(val, low, high, &errstr);
+	if (errstr)
 		return (0);
-	}
+
+	numval = htobe64(numval);
+	memcpy(buf, (char *)&numval + (sizeof(numval) - bytes), bytes);
 
 	return (1);
 }
 
-void
-convert_num(unsigned char *buf, char *str, int base, int size)
+int
+parse_hex(FILE *cfile, unsigned char *buf)
 {
-	int negative = 0, tval, max;
-	u_int32_t val = 0;
-	char *ptr = str;
-
-	if (*ptr == '-') {
-		negative = 1;
-		ptr++;
-	}
-
-	/* If base wasn't specified, figure it out from the data. */
-	if (!base) {
-		if (ptr[0] == '0') {
-			if (ptr[1] == 'x') {
-				base = 16;
-				ptr += 2;
-			} else if (isascii((unsigned char)ptr[1]) &&
-			    isdigit((unsigned char)ptr[1])) {
-				base = 8;
-				ptr += 1;
-			} else
-				base = 10;
-		} else
-			base = 10;
-	}
+	char *val, *ep;
+	int token;
+	unsigned long ulval;
+	
+	token = next_token(&val, cfile);
 
-	do {
-		tval = *ptr++;
-		/* XXX assumes ASCII. */
-		if (tval >= 'a')
-			tval = tval - 'a' + 10;
-		else if (tval >= 'A')
-			tval = tval - 'A' + 10;
-		else if (tval >= '0')
-			tval -= '0';
-		else {
-			warning("Bogus number: %s.", str);
-			break;
-		}
-		if (tval >= base) {
-			warning("Bogus number: %s: digit %d not in base %d",
-			    str, tval, base);
-			break;
-		}
-		val = val * base + tval;
-	} while (*ptr);
-
-	if (negative)
-		max = (1 << (size - 1));
-	else
-		max = (1 << (size - 1)) + ((1 << (size - 1)) - 1);
-	if (val > max) {
-		switch (base) {
-		case 8:
-			warning("value %s%o exceeds max (%d) for precision.",
-			    negative ? "-" : "", val, max);
-			break;
-		case 16:
-			warning("value %s%x exceeds max (%d) for precision.",
-			    negative ? "-" : "", val, max);
-			break;
-		default:
-			warning("value %s%u exceeds max (%d) for precision.",
-			    negative ? "-" : "", val, max);
-			break;
-		}
-	}
+	errno = 0;
+	ulval = strtoul(val, &ep, 16);
+	if ((val[0] == '\0' || *ep != '\0') ||
+	    (errno == ERANGE && ulval == ULONG_MAX) ||
+	    (ulval > UINT8_MAX))
+		return (0);
 
-	if (negative)
-		switch (size) {
-		case 8:
-			*buf = -(unsigned long)val;
-			break;
-		case 16:
-			putShort(buf, -(unsigned long)val);
-			break;
-		case 32:
-			putLong(buf, -(unsigned long)val);
-			break;
-		default:
-			warning("Unexpected integer size: %d", size);
-			break;
-		}
-	else
-		switch (size) {
-		case 8:
-			*buf = (u_int8_t)val;
-			break;
-		case 16:
-			putUShort(buf, (u_int16_t)val);
-			break;
-		case 32:
-			putULong(buf, val);
-			break;
-		default:
-			warning("Unexpected integer size: %d", size);
-			break;
-		}
+	buf[0] = ulval;
+	
+	return (1);
 }
 
 /*