Subject: bin/17635 printf(1) bugs
To: None <gnats-bugs@gnats.netbsd.org, netbsd-bugs@netbsd.org>
From: David Laight <david@l8s.co.uk>
List: netbsd-bugs
Date: 10/30/2002 12:46:43
The patch below fixes a variety of problems with printf(1).
Including the (possibly exploitable) buffer overrun problem:
    printf '%00000000000000000000000000000000000000000000000000000000000000000000000000000000000000d' 0

	David

Index: printf.1
===================================================================
RCS file: /cvsroot/basesrc/usr.bin/printf/printf.1,v
retrieving revision 1.16
diff -u -r1.16 printf.1
--- printf.1	2002/09/30 11:09:09	1.16
+++ printf.1	2002/10/30 12:35:45
@@ -64,6 +64,7 @@
 after the first are treated as strings if the corresponding format is
 either
 .Cm b ,
+.Cm B ,
 .Cm c
 or
 .Cm s ;
@@ -85,8 +86,7 @@
 .Pp
 Character escape sequences are in backslash notation as defined in
 .St -ansiC .
-The characters and their meanings
-are as follows:
+The characters and their meanings are as follows:
 .Bl -tag -width Ds -offset indent
 .It Cm \ee
 Write an \*[Lt]escape\*[Gt] character.
@@ -106,14 +106,20 @@
 Write a \*[Lt]vertical tab\*[Gt] character.
 .It Cm \e\'
 Write a \*[Lt]single quote\*[Gt] character.
+.It Cm \e"
+Write a \*[Lt]double quote\*[Gt] character.
 .It Cm \e\e
 Write a backslash character.
 .It Cm \e Ns Ar num
-Write an 8-bit character whose
+Write an 8\-bit character whose
 .Tn ASCII
-value is the 1-, 2-, or 3-digit
-octal number
+value is the 1\-, 2\-, or 3\-digit octal number
 .Ar num .
+.It Cm \ex Ns Ar xx
+Write an 8\-bit character whose
+.Tn ASCII
+value is the 1\- or 2\-digit hexadecimal number
+.Ar xx .
 .El
 .Pp
 Each format specification is introduced by the percent character
@@ -127,14 +133,15 @@
 A `#' character
 specifying that the value should be printed in an ``alternative form''.
 For
-.Cm c  ,
+.Cm b ,
+.Cm c ,
 .Cm d ,
 and
-.Cm s  ,
+.Cm s
 formats, this option has no effect.
 For the
 .Cm o
-formats the precision of the number is increased to force the first
+format the precision of the number is increased to force the first
 character of the output string to a zero.
 For the
 .Cm x
@@ -149,7 +156,7 @@
 .Cm f  ,
 .Cm g ,
 and
-.Cm G  ,
+.Cm G
 formats, the result will always contain a decimal point, even if no
 digits follow the point (normally, a decimal point only appears in the
 results of those formats if a digit follows the decimal point).
@@ -158,7 +165,11 @@
 and
 .Cm G
 formats, trailing zeros are not removed from the result as they
-would otherwise be;
+would otherwise be.
+.\" I turned this off - decided it isn't a valid use of '#'
+.\" For the
+.\" .Cm B
+.\" format, backslash-escape sequences are expanded first;
 .It Cm \&\-
 A minus sign `\-' which specifies
 .Em left adjustment
@@ -193,11 +204,18 @@
 and
 .Cm f
 formats, or the maximum number of characters to be printed
-from a string; if the digit string is missing, the precision is treated
+from a string
+.Sm off
+.Pf ( Cm b No ,
+.Sm on
+.Cm B
+and
+.Cm s
+formats); if the digit string is missing, the precision is treated
 as zero;
 .It Format :
 A character which indicates the type of format to use (one of
-.Cm diouxXfwEgGbcs ) .
+.Cm diouxXfwEgGbBcs ) .
 .El
 .Pp
 A field width or precision may be
@@ -251,6 +269,48 @@
 Characters from the string
 .Ar argument
 are printed with backslash-escape sequences expanded.
+.br
+The following additional backslash-escape sequences are supported:
+.Bl -tag -width Ds
+.It Cm \ec
+Causes
+.Nm
+to ignore any remaining characters in the string operand containing it,
+any remaining string operands, and any additional characters in
+the format operand.
+.It Cm \e0 Ns Ar num
+Write an 8\-bit character whose
+.Tn ASCII
+value is the 1\-, 2\-, or 3\-digit
+octal number
+.Ar num .
+.It Cm \e^ Ns Ar c
+Write the control character 
+.Ar c .
+Generates characters `\e000' through `\e037`, and `\e177' (from `\e^?').
+.It Cm \eM\- Ns Ar c
+Write the character 
+.Ar c
+with the 8th bit set.
+Generates characters `\e241' through `\e376`.
+.It Cm \eM^ Ns Ar c
+Write the control character 
+.Ar c
+with the 8th bit set.
+Generates characters `\e000' through `\e037`, and `\e177' (from `\eM^?').
+.El
+.It Cm B
+Characters from the string
+.Ar argument
+are printed with unprintable characters backslash-escaped using the
+.Sm off
+.Pf ` Cm \e Ar c No ',
+.Pf ` Cm \e^ Ar c No ',
+.Pf ` Cm \eM\- Ar c No '
+or
+.Pf ` Cm \eM^ Ar c No ',
+.Sm on
+formats described above.
 .It Cm c
 The first character of
 .Ar argument
@@ -259,8 +319,8 @@
 Characters from the string
 .Ar argument
 are printed until the end is reached or until the number of characters
-indicated by the precision specification is reached; however if the
-precision is 0 or missing, all characters in the string are printed.
+indicated by the precision specification is reached; if the
+precision is omitted, all characters in the string are printed.
 .It Cm \&%
 Print a `%'; no argument is used.
 .El
@@ -275,13 +335,24 @@
 .Xr echo 1 ,
 .Xr printf 3 ,
 .Xr printf 9
+.Xr vis 3
 .Sh STANDARDS
 The
 .Nm
 utility conforms to
-.St -p1003.2-92 .
+.St -p1003.1-2001 .
+.Pp
+Support for the floating point formats and `*' as a field width and precision
+are optional in POSIX.
+.Pp
+The behaviour of the %B format and the \e', \e", \exxx, \ee and
+\e[M][\-|^]c escape sequences are undefined in POSIX.
 .Sh BUGS
 Since the floating point numbers are translated from
 .Tn ASCII
 to floating-point and
 then back again, floating-point precision may be lost.
+.Pp
+Hexadecimal character constants are restricted to, and should be specified
+as, two character constants.  This is contrary to the ISO C standard but
+does guarantee detection of the end of the constant.
Index: printf.c
===================================================================
RCS file: /cvsroot/basesrc/usr.bin/printf/printf.c,v
retrieving revision 1.24
diff -u -r1.24 printf.c
--- printf.c	2002/06/14 11:32:15	1.24
+++ printf.c	2002/10/30 12:35:48
@@ -63,76 +63,55 @@
 #include <string.h>
 #include <unistd.h>
 
-static int	 print_escape_str(const char *);
-static size_t	 print_escape(const char *);
+#ifdef __GNUC__
+#define ESCAPE '\e'
+#else
+#define ESCAPE 033
+#endif
 
+static char	*conv_escape_str(char *);
+static char	*conv_escape(char *, char *);
+static char	*conv_expand(const char *);
 static int	 getchr(void);
 static double	 getdouble(void);
-static int	 getint(void);
+static int	 getwidth(void);
 static intmax_t	 getintmax(void);
-static uintmax_t getuintmax __P ((void));
+static uintmax_t getuintmax(void);
 static char	*getstr(void);
-static char	*mklong(const char *, int); 
+static char	*mklong(const char *, int);
 static void      check_conversion(const char *, const char *);
 static void	 usage(void); 
-     
+
 static int	rval;
 static char  **gargv;
 
-#ifdef BUILTIN
-int progprintf(int, char **);
-#else
-int main(int, char **);
+#ifdef BUILTIN		/* csh builtin */
+#define main progprintf
 #endif
-
-#define isodigit(c)	((c) >= '0' && (c) <= '7')
-#define octtobin(c)	((c) - '0')
-#define hextobin(c)	((c) >= 'A' && (c) <= 'F' ? c - 'A' + 10 : (c) >= 'a' && (c) <= 'f' ? c - 'a' + 10 : c - '0')
 
-#ifdef SHELL
+#ifdef SHELL		/* sh (aka ash) builtin */
 #define main printfcmd
 #include "../../bin/sh/bltin/bltin.h"
-
-
-static void warnx(const char *fmt, ...);
-
-static void 
-warnx(const char *fmt, ...)
-{
-	
-	char buf[64];
-	va_list ap;
-
-	va_start(ap, fmt);
-	vsprintf(buf, fmt, ap);
-	va_end(ap);
-
-	error(buf);
-}
 #endif /* SHELL */
 
 #define PF(f, func) { \
-	if (fieldwidth) { \
-		if (precision) \
+	if (fieldwidth != -1) { \
+		if (precision != -1) \
 			(void)printf(f, fieldwidth, precision, func); \
 		else \
 			(void)printf(f, fieldwidth, func); \
-	} else if (precision) \
+	} else if (precision != -1) \
 		(void)printf(f, precision, func); \
 	else \
 		(void)printf(f, func); \
 }
 
-int
-#ifdef BUILTIN
-progprintf(int argc, char **argv)
-#else
-main(int argc, char **argv)
-#endif
+int main(int, char **);
+int main(int argc, char *argv[])
 {
 	char *fmt, *start;
 	int fieldwidth, precision;
-	char convch, nextch;
+	char nextch;
 	char *format;
 	int ch;
 
@@ -172,94 +151,103 @@
 		 */
 
 		/* find next format specification */
-		for (fmt = format; *fmt; fmt++) {
-			switch (*fmt) {
-			case '%':
-				start = fmt++;
-
-				if (*fmt == '%') {
-					(void)putchar('%');
-					break;
-				} else if (*fmt == 'b') {
-					char *p = getstr();
-					if (print_escape_str(p)) {
-						return (rval);
-					}
-					break;
-				}
-
-				/* skip to field width */
-				for (; strchr(SKIP1, *fmt); ++fmt) ;
-				fieldwidth = *fmt == '*' ? getint() : 0;
-
-				/* skip to possible '.', get following precision */
-				for (; strchr(SKIP2, *fmt); ++fmt) ;
-				if (*fmt == '.')
-					++fmt;
-				precision = *fmt == '*' ? getint() : 0;
-
-				for (; strchr(SKIP2, *fmt); ++fmt) ;
-				if (!*fmt) {
-					warnx ("missing format character");
-					return(1);
-				}
-
-				convch = *fmt;
-				nextch = *(fmt + 1);
-				*(fmt + 1) = '\0';
-				switch(convch) {
-				case 'c': {
-					char p = getchr();
-					PF(start, p);
-					break;
-				}
-				case 's': {
-					char *p = getstr();
-					PF(start, p);
-					break;
-				}
-				case 'd':
-				case 'i': {
-					char *f = mklong(start, convch);
-					intmax_t p = getintmax();
-					PF(f, p);
-					break;
-				}
-				case 'o':
-				case 'u':
-				case 'x':
-				case 'X': {
-					char *f = mklong(start, convch);
-					uintmax_t p = getuintmax();
-					PF(f, p);
-					break;
-				}
-				case 'e':
-				case 'E':
-				case 'f':
-				case 'g':
-				case 'G': {
-					double p = getdouble();
-					PF(start, p);
-					break;
-				}
-				default:
-					warnx ("%s: invalid directive", start);
-					return(1);
-				}
-				*(fmt + 1) = nextch;
-				break;
+		for (fmt = format; (ch = *fmt++) ;) {
+			if (ch == '\\') {
+				char c_ch;
+				fmt = conv_escape(fmt, &c_ch);
+				putchar(c_ch);
+				continue;
+			}
+			if (ch != '%' || (*fmt == '%' && ++fmt)) {
+				(void)putchar(ch);
+				continue;
+			}
 
-			case '\\':
-				fmt += print_escape(fmt);
+			/* Ok - we've found a format specification,
+			   Save its address for a later printf(). */
+			start = fmt - 1;
+
+			/* skip to field width */
+			fmt += strspn(fmt, SKIP1);
+			fieldwidth = *fmt == '*' ? getwidth() : -1;
+
+			/* skip to possible '.', get following precision */
+			fmt += strspn(fmt, SKIP2);
+			if (*fmt == '.')
+				++fmt;
+			precision = *fmt == '*' ? getwidth() : -1;
+
+			fmt += strspn(fmt, SKIP2);
+
+			ch = *fmt;
+			if (!ch) {
+				warnx("missing format character");
+				return (1);
+			}
+			/* null terminate format string to we can use it
+			   as an argument to printf. */
+			nextch = fmt[1];
+			fmt[1] = 0;
+			switch (ch) {
+
+			case 'B': {
+				const char *p = conv_expand(getstr());
+				*fmt = 's';
+				PF(start, p);
 				break;
-
-			default:
-				(void)putchar(*fmt);
+			}
+			case 'b': {
+				char *p = conv_escape_str(getstr());
+				*fmt = 's';
+				PF(start, p);
+				break;
+			}
+			case 'c': {
+				char p = getchr();
+				PF(start, p);
+				break;
+			}
+			case 's': {
+				char *p = getstr();
+				PF(start, p);
 				break;
 			}
+			case 'd':
+			case 'i': {
+				intmax_t p = getintmax();
+				char *f = mklong(start, ch);
+				PF(f, p);
+				break;
+			}
+			case 'o':
+			case 'u':
+			case 'x':
+			case 'X': {
+				uintmax_t p = getuintmax();
+				char *f = mklong(start, ch);
+				PF(f, p);
+				break;
+			}
+			case 'e':
+			case 'E':
+			case 'f':
+			case 'g':
+			case 'G': {
+				double p = getdouble();
+				PF(start, p);
+				break;
+			}
+			default:
+				warnx("%s: invalid directive", start);
+				return (1);
+			}
+			*fmt++ = ch;
+			*fmt = nextch;
+			/* escape if a \c was encountered */
+			if (rval & 0x100)
+				return (rval & ~0x100);
 		}
-	} while (gargv > argv && *gargv);
+	} while (gargv != argv && *gargv);
 
 	return (rval);
 }
@@ -267,138 +255,207 @@
 
 /*
  * Print SysV echo(1) style escape string 
- *	Halts processing string and returns 1 if a \c escape is encountered.
+ *	Halts processing string if a \c escape is encountered.
  */
-static int
-print_escape_str(const char *str)
+static char *
+conv_escape_str(char *str)
 {
 	int value;
-	int c;
+	int ch;
+	static char *conv_str;
+	char *cp;
 
-	while (*str) {
-		if (*str == '\\') {
+	/* convert string into a temporary buffer... */
+	if (conv_str)
+		free(conv_str);
+	conv_str = malloc(strlen(str) + 4);
+	if (!conv_str)
+		return "<no memory>";
+	cp = conv_str;
+
+	while ((ch = *str++)) {
+		if (ch != '\\') {
+			*cp++ = ch;
+			continue;
+		}
+
+		ch = *str++;
+		if (ch == 'c') {
+			/* \c as in SYSV echo - abort all processing.... */
+			rval |= 0x100;
+			break;
+		}
+
+		/* 
+		 * %b string octal constants are not like those in C.
+		 * They start with a \0, and are followed by 0, 1, 2, 
+		 * or 3 octal digits. 
+		 */
+		if (ch == '0') {
+			char octnum[4], *oct_end;
+			octnum[0] = str[0];
+			octnum[1] = str[1];
+			octnum[2] = str[2];
+			octnum[3] = 0;
+			*cp++ = strtoul(octnum, &oct_end, 8);
+			str += oct_end - octnum;
+			continue;
+		}
+
+		/* \[M][^|-]C as defined by vis(3) */
+		if (ch == 'M' && *str == '-') {
+			*cp++ = 0200 | str[1];
+			str += 2;
+			continue;
+		}
+		if (ch == 'M' && *str == '^') {
 			str++;
-			/* 
-			 * %b string octal constants are not like those in C.
-			 * They start with a \0, and are followed by 0, 1, 2, 
-			 * or 3 octal digits. 
-			 */
-			if (*str == '0') {
-				str++;
-				for (c = 3, value = 0; c-- && isodigit(*str); str++) {
-					value <<= 3;
-					value += octtobin(*str);
-				}
-				(void)putchar(value);
-				str--;
-			} else if (*str == 'c') {
-				return 1;
-			} else {
-				str--;			
-				str += print_escape(str);
-			}
-		} else {
-			(void)putchar(*str);
+			value = 0200;
+			ch = '^';
+		} else
+			value = 0;
+		if (ch == '^') {
+			ch = *str++;
+			if (ch == '?')
+				value |= 0177;
+			else
+				value |= ch & 037;
+			*cp++ = value;
+			continue;
 		}
-		str++;
+
+		/* Finally test for sequences valid in the format string */
+		str = conv_escape(str - 1, cp);
+		cp++;
 	}
+	*cp = 0;
 
-	return 0;
+	return conv_str;
 }
 
 /*
  * Print "standard" escape characters 
  */
-static size_t
-print_escape(const char *str)
+static char *
+conv_escape(char *str, char *conv_ch)
 {
-	const char *start = str;
 	int value;
-	int c;
+	int ch;
+	char num_buf[4], *num_end;
 
-	str++;
+	ch = *str++;
 
-	switch (*str) {
+	switch (ch) {
 	case '0': case '1': case '2': case '3':
 	case '4': case '5': case '6': case '7':
-		for (c = 3, value = 0; c-- && isodigit(*str); str++) {
-			value <<= 3;
-			value += octtobin(*str);
-		}
-		(void)putchar(value);
-		return str - start - 1;
-		/* NOTREACHED */
-
-	case 'x':
-		str++;
-		for (value = 0; isxdigit((unsigned char)*str); str++) {
-			value <<= 4;
-			value += hextobin(*str);
-		}
-		if (value > UCHAR_MAX) {
-			warnx ("escape sequence out of range for character");
-			rval = 1;
-		}
-		(void)putchar (value);
-		return str - start - 1;
-		/* NOTREACHED */
-
-	case '\\':			/* backslash */
-		(void)putchar('\\');
-		break;
-
-	case '\'':			/* single quote */
-		(void)putchar('\'');
-		break;
-
-	case '"':			/* double quote */
-		(void)putchar('"');
-		break;
-
-	case 'a':			/* alert */
-		(void)putchar('\a');
-		break;
-
-	case 'b':			/* backspace */
-		(void)putchar('\b');
+		num_buf[0] = ch;
+		ch = str[0];
+		num_buf[1] = ch;
+		num_buf[2] = ch ? str[1] : 0;
+		num_buf[3] = 0;
+		value = strtoul(num_buf, &num_end, 8);
+		str += num_end  - (num_buf + 1);
 		break;
 
-	case 'e':			/* escape */
-#ifdef __GNUC__
-		(void)putchar('\e');
-#else
-		(void)putchar(033);
-#endif
-		break;
+	case 'x':
+		/* Hexadecimal character constants are not required to be
+		   supported (by SuS v1) because there is no consistent
+		   way to detect the end of the constant.
+		   Supporting 2 byte constants is a compromise. */
+		ch = str[0];
+		num_buf[0] = ch;
+		num_buf[1] = ch ? str[1] : 0;
+		num_buf[2] = 0;
+		value = strtoul(num_buf, &num_end, 16);
+		str += num_end - num_buf;
+		break;
+
+	case '\\':	value = '\\';	break;	/* backslash */
+	case '\'':	value = '\'';	break;	/* single quote */
+	case '"':	value = '"';	break;	/* double quote */
+	case 'a':	value = '\a';	break;	/* alert */
+	case 'b':	value = '\b';	break;	/* backspace */
+	case 'e':	value = ESCAPE;	break;	/* escape */
+	case 'f':	value = '\f';	break;	/* form-feed */
+	case 'n':	value = '\n';	break;	/* newline */
+	case 'r':	value = '\r';	break;	/* carriage-return */
+	case 't':	value = '\t';	break;	/* tab */
+	case 'v':	value = '\v';	break;	/* vertical-tab */
 
-	case 'f':			/* form-feed */
-		(void)putchar('\f');
+	default:
+		warnx("unknown escape sequence `\\%c'", *str);
+		rval = 1;
+		value = *str;
 		break;
+	}
 
-	case 'n':			/* newline */
-		(void)putchar('\n');
-		break;
+	*conv_ch = value;
+	return str;
+}
 
-	case 'r':			/* carriage-return */
-		(void)putchar('\r');
-		break;
+/* expand a string so that everything is printable */
 
-	case 't':			/* tab */
-		(void)putchar('\t');
-		break;
+static char *
+conv_expand(const char *str)
+{
+	static char *conv_str;
+	char *cp;
+	int ch;
 
-	case 'v':			/* vertical-tab */
-		(void)putchar('\v');
-		break;
+	if (conv_str)
+		free(conv_str);
+	/* get a buffer that is definitely large enough.... */
+	conv_str = malloc(4 * strlen(str) + 1);
+	if (!conv_str)
+		return "<no memory>";
+	cp = conv_str;
 
-	default:
-		(void)putchar(*str);
-		warnx("unknown escape sequence `\\%c'", *str);
-		rval = 1;
-		break;
+	while ((ch = *(unsigned char *)str++)) {
+		switch (ch) {
+		/* Use C escapes for expected control characters */
+		case '\\':	ch = '\\';	break;	/* backslash */
+		case '\'':	ch = '\'';	break;	/* single quote */
+		case '"':	ch = '"';	break;	/* double quote */
+		case '\a':	ch = 'a';	break;	/* alert */
+		case '\b':	ch = 'b';	break;	/* backspace */
+		case ESCAPE:	ch = 'e';	break;	/* escape */
+		case '\f':	ch = 'f';	break;	/* form-feed */
+		case '\n':	ch = 'n';	break;	/* newline */
+		case '\r':	ch = 'r';	break;	/* carriage-return */
+		case '\t':	ch = 't';	break;	/* tab */
+		case '\v':	ch = 'v';	break;	/* vertical-tab */
+		default:
+			/* Copy anything printable */
+			if (isprint(ch)) {
+				*cp++ = ch;
+				continue;
+			}
+			/* Use vis(3) encodings for the rest */
+			*cp++ = '\\';
+			if (ch & 0200) {
+				*cp++ = 'M';
+				ch &= ~0200;
+			}
+			if (ch == 0177) {
+				*cp++ = '^';
+				*cp++ = '?';
+				continue;
+			}
+			if (ch < 040) {
+				*cp++ = '^';
+				*cp++ = ch | 0100;
+				continue;
+			}
+			*cp++ = '-';
+			*cp++ = ch;
+			continue;
+		}
+		*cp++ = '\\';
+		*cp++ = ch;
 	}
 
-	return 1;
+	*cp = 0;
+	return conv_str;
 }
 
 static char *
@@ -408,6 +465,10 @@
 	size_t len;	
 
 	len = strlen(str) + 2;
+	if (len > sizeof copy) {
+		warnx("format %s too complex\n", str);
+		len = 4;
+	}
 	(void)memmove(copy, str, len - 3);
 	copy[len - 3] = 'j';
 	copy[len - 2] = ch;
@@ -431,34 +492,47 @@
 	return (*gargv++);
 }
 
-static char *Number = "+-.0123456789";
 static int
-getint(void)
+getwidth(void)
 {
+	long val;
+	char *s, *ep;
+
+	s = *gargv;
 	if (!*gargv)
-		return(0);
+		return (0);
+	gargv++;
 
-	if (strchr(Number, **gargv))
-		return(atoi(*gargv++));
+	errno = 0;
+	val = strtoul(s, &ep, 0);
+	check_conversion(s, ep);
 
-	return 0;
+	/* Arbitrarily 'restrict' field widths to 1Mbyte */
+	if (val < 0 || val > 1 << 20) {
+		warnx("%s: invalid field width", s);
+		return 0;
+	}
+
+	return val;
 }
 
 static intmax_t
 getintmax(void)
 {
 	intmax_t val;
-	char *ep;
+	char *cp, *ep;
 
-	if (!*gargv)
-		return(INTMAX_C(0));
+	cp = *gargv;
+	if (cp == NULL)
+		return 0;
+	gargv++;
 
-	if (**gargv == '\"' || **gargv == '\'')
-		return (intmax_t) *((*gargv++)+1);
+	if (*cp == '\"' || *cp == '\'')
+		return *(cp+1);
 
 	errno = 0;
-	val = strtoimax (*gargv, &ep, 0);
-	check_conversion(*gargv++, ep);
+	val = strtoimax(cp, &ep, 0);
+	check_conversion(cp, ep);
 	return val;
 }
 
@@ -466,17 +540,28 @@
 getuintmax(void)
 {
 	uintmax_t val;
-	char *ep;
+	char *cp, *ep;
 
-	if (!*gargv)
-		return(UINTMAX_C(0));
-
-	if (**gargv == '\"' || **gargv == '\'')
-		return (uintmax_t) *((*gargv++)+1);
+	cp = *gargv;
+	if (cp == NULL)
+		return 0;
+	gargv++;
+
+	if (*cp == '\"' || *cp == '\'')
+		return *(cp+1);
+
+	/* strtoumax won't error -ve values */
+	while (isspace(*(unsigned char *)cp))
+		cp++;
+	if (*cp == '-') {
+		warnx("%s: expected positive numeric value", cp);
+		rval = 1;
+		return 0;
+	}
 
 	errno = 0;
-	val = strtoumax (*gargv, &ep, 0);
-	check_conversion(*gargv++, ep);
+	val = strtoumax(cp, &ep, 0);
+	check_conversion(cp, ep);
 	return val;
 }
 
@@ -487,13 +572,13 @@
 	char *ep;
 
 	if (!*gargv)
-		return(0.0);
+		return (0.0);
 
 	if (**gargv == '\"' || **gargv == '\'')
 		return (double) *((*gargv++)+1);
 
 	errno = 0;
-	val = strtod (*gargv, &ep);
+	val = strtod(*gargv, &ep);
 	check_conversion(*gargv++, ep);
 	return val;
 }
@@ -503,12 +588,12 @@
 {
 	if (*ep) {
 		if (ep == s)
-			warnx ("%s: expected numeric value", s);
+			warnx("%s: expected numeric value", s);
 		else
-			warnx ("%s: not completely converted", s);
+			warnx("%s: not completely converted", s);
 		rval = 1;
 	} else if (errno == ERANGE) {
-		warnx ("%s: %s", s, strerror(ERANGE));
+		warnx("%s: %s", s, strerror(ERANGE));
 		rval = 1;
 	}
 }

-- 
David Laight: david@l8s.co.uk