[PATCH 06/12] parse: Add helper to parse unsigned integer values

[David Gibson <david@gibson.dropbear.id.au>]

Most places we need to parse an integer encoded as a string, we use
strtol() or strtoul().  These already work a bit like descent parser
helpers, in that they consume as much as they can, but don't require the
number parsed to be the whole of the einput string.  Make a wrapper to
parse unsigned integers as part of our parsing helper.  This wrapper
handles the mildly fiddly error checking requirements for strtoul().

We replace a number, though not all, of our existing strtoul() uses with
the new parse_unsigned().  We also replace a number of strtol() use cases,
because, despite using that instead of strtoul() they are only used for
non-negative values.  In some cases this makes the logic a little more
straightforward.  In some other cases it will catch some error cases we
previously could have missed.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 Makefile   |  1 -
 conf.c     | 51 +++++++++++++++++++++++++--------------------------
 fwd_rule.c | 42 ++++++++++++++----------------------------
 parse.c    | 27 ++++++++++++++++++++++-----
 parse.h    |  1 +
 5 files changed, 62 insertions(+), 60 deletions(-)

diff --git a/Makefile b/Makefile
index bd729c75..85d279c0 100644
--- a/Makefile
+++ b/Makefile
@@ -225,7 +225,6 @@ pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:bitmap.c
 pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:inany.h
 pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:inany.c
 pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:ip.h
-pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:parse.c
 pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:serialise.c
 pesto.cppcheck: CPPCHECK_FLAGS += --suppress=staticFunction:fwd_rule.c
 pesto.cppcheck: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h
diff --git a/conf.c b/conf.c
index ea299e18..bd357117 100644
--- a/conf.c
+++ b/conf.c
@@ -321,17 +321,16 @@ static void conf_pasta_ns(int *netns_only, char *userns, char *netns,
 		die("Both --netns and PID or command given");
 
 	if (optind + 1 == argc) {
-		char *endptr;
-		long pidval;
+		const char *p = argv[optind];
+		unsigned long pidval;
 
-		pidval = strtol(argv[optind], &endptr, 10);
-		if (!*endptr) {
+		if (parse_unsigned(&p, 10, &pidval) && parse_eoi(p)) {
 			/* Looks like a pid */
-			if (pidval < 0 || pidval > INT_MAX)
+			if (pidval > INT_MAX)
 				die("Invalid PID %s", argv[optind]);
 
 			if (snprintf_check(netns, PATH_MAX,
-					   "/proc/%ld/ns/net", pidval))
+					   "/proc/%lu/ns/net", pidval))
 				die_perror("Can't build netns path");
 
 			if (!*userns) {
@@ -354,6 +353,7 @@ static void conf_pasta_ns(int *netns_only, char *userns, char *netns,
  */
 static uint8_t conf_ip4_prefix(const char *arg)
 {
+	const char *p = arg;
 	struct in_addr mask;
 	unsigned long len;
 
@@ -362,11 +362,9 @@ static uint8_t conf_ip4_prefix(const char *arg)
 		len = __builtin_popcount(hmask);
 		if ((hmask << len) == 0)
 			return len;
-	} else {
-		errno = 0;
-		len = strtoul(arg, NULL, 0);
-		if (!errno && len <= 32)
-			return len;
+	} else if (parse_unsigned(&p, 0, &len) && parse_eoi(p) &&
+		   len <= 32) {
+		return len;
 	}
 
 	die("Invalid prefix length: %s", arg);
@@ -1159,13 +1157,12 @@ static void conf_sock_listen(const struct ctx *c)
  */
 int conf_tap_fd(const char *arg)
 {
-	long val;
-
-	errno = 0;
-	val = strtol(arg, NULL, 0);
+	const char *p = arg;
+	unsigned long val;
 
-	if (errno || (val != STDIN_FILENO && val <= STDERR_FILENO) ||
-	    val > INT_MAX)
+	if (!parse_unsigned(&p, 0, &val) || !parse_eoi(p)	||
+	    val > INT_MAX					||
+	    (val != STDIN_FILENO && val <= STDERR_FILENO))
 		die("Invalid --fd: %s", arg);
 
 	return val;
@@ -1288,6 +1285,8 @@ void conf(struct ctx *c, int argc, char **argv)
 
 	optind = 0;
 	do {
+		const char *p;
+
 		name = getopt_long(argc, argv, optstring, options, NULL);
 
 		switch (name) {
@@ -1368,14 +1367,17 @@ void conf(struct ctx *c, int argc, char **argv)
 		case 12:
 			runas = optarg;
 			break;
-		case 13:
-			errno = 0;
-			logsize = strtol(optarg, NULL, 0);
+		case 13: {
+			unsigned long val;
 
-			if (logsize < LOGFILE_SIZE_MIN || errno)
+			p = optarg;
+			if (!parse_unsigned(&p, 0, &val) || !parse_eoi(p) ||
+			    val < LOGFILE_SIZE_MIN)
 				die("Invalid --log-size: %s", optarg);
 
+			logsize = val;
 			break;
+		}
 		case 14:
 			FPRINTF(stdout,
 				c->mode == MODE_PASTA ? "pasta " : "passt ");
@@ -1552,12 +1554,9 @@ void conf(struct ctx *c, int argc, char **argv)
 			break;
 		case 'm': {
 			unsigned long mtu;
-			char *e;
-
-			errno = 0;
-			mtu = strtoul(optarg, &e, 0);
 
-			if (errno || *e)
+			p = optarg;
+			if (!parse_unsigned(&p, 0, &mtu) || !parse_eoi(p))
 				die("Invalid MTU: %s", optarg);
 
 			if (mtu > max_mtu) {
diff --git a/fwd_rule.c b/fwd_rule.c
index d0e90402..d4b422d7 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -41,10 +41,11 @@ static in_port_t fwd_ephemeral_max = NUM_PORTS - 1;
  */
 void fwd_probe_ephemeral(void)
 {
-	char *line, *tab, *end;
+	unsigned long min, max;
 	struct lineread lr;
-	long min, max;
+	const char *p;
 	ssize_t len;
+	char *line;
 	int fd;
 
 	fd = open(PORT_RANGE_SYSCTL, O_RDONLY | O_CLOEXEC);
@@ -57,35 +58,20 @@ void fwd_probe_ephemeral(void)
 	len = lineread_get(&lr, &line);
 	close(fd);
 
-	if (len < 0)
-		goto parse_err;
-
-	tab = strchr(line, '\t');
-	if (!tab)
-		goto parse_err;
-	*tab = '\0';
-
-	errno = 0;
-	min = strtol(line, &end, 10);
-	if (*end || errno)
-		goto parse_err;
-
-	errno = 0;
-	max = strtol(tab + 1, &end, 10);
-	if (*end || errno)
-		goto parse_err;
-
-	if (min < 0 || min >= (long)NUM_PORTS ||
-	    max < 0 || max >= (long)NUM_PORTS)
-		goto parse_err;
+	p = line;
+	if (len < 0				||
+	    !parse_unsigned(&p, 10, &min)	||
+	    !parse_literal(&p, "\t")		||
+	    !parse_unsigned(&p, 10, &max)	||
+	    !parse_eoi(p)			||
+	    min >= NUM_PORTS			||
+	    max >= NUM_PORTS) {
+		warn("Unable to parse %s", PORT_RANGE_SYSCTL);
+		return;
+	}
 
 	fwd_ephemeral_min = min;
 	fwd_ephemeral_max = max;
-
-	return;
-
-parse_err:
-	warn("Unable to parse %s", PORT_RANGE_SYSCTL);
 }
 
 /**
diff --git a/parse.c b/parse.c
index c2a92422..8abfd4dd 100644
--- a/parse.c
+++ b/parse.c
@@ -12,14 +12,12 @@
  * Author: David Gibson <david@gibson.dropbear.id.au>
  */
 
-#include <assert.h>
+#include <errno.h>
 #include <stddef.h>
-#include <fcntl.h>
+#include <stdlib.h>
 #include <string.h>
-#include <stdbool.h>
-#include <unistd.h>
 
-#include "common.h"
+#include "parse.h"
 
 /**
  * DOC: Theory of Operation
@@ -65,3 +63,22 @@ bool parse_eoi(const char *cursor)
 {
 	return !(*cursor);
 }
+
+/*
+ * parse_unsigned() - Parse an unsigned integer
+ * @base:	Numeric base for string as strtoul(3)
+ * @valp:	On success, updated with parsed value
+ */
+bool parse_unsigned(const char **cursor, int base, unsigned long *valp)
+{
+	const char *p = *cursor;
+	unsigned long val;
+
+	errno = 0;
+	val = strtoul(p, (char **)&p, base);
+	if (errno || p == *cursor)
+		return false;
+	*valp = val;
+	*cursor = p;
+	return true;
+}
diff --git a/parse.h b/parse.h
index eb51a469..4f38c28b 100644
--- a/parse.h
+++ b/parse.h
@@ -10,5 +10,6 @@
 
 bool parse_literal(const char **cursor, const char *lit);
 bool parse_eoi(const char *cursor);
+bool parse_unsigned(const char **cursor, int base, unsigned long *valp);
 
 #endif /* _PARSE_H */
-- 
2.54.0