[ntpsec commit] Eliminate more homebrew interpretation of integer strings.

Eric S. Raymond esr at ntpsec.org
Fri Oct 23 09:54:05 UTC 2015


Module:    ntpsec
Branch:    master
Commit:    5c641d644c315d7b6e8b106928da23de0fbe1098
Changeset: http://git.ntpsec.org/ntpsec/commit/?id=5c641d644c315d7b6e8b106928da23de0fbe1098

Author:    Eric S. Raymond <esr at thyrsus.com>
Date:      Fri Oct 23 05:53:19 2015 -0400

Eliminate more homebrew interpretation of integer strings.

---

 include/ntp_stdlib.h                             |  3 --
 libntp/atoint.c                                  | 51 --------------------
 libntp/atouint.c                                 | 42 -----------------
 libntp/hextoint.c                                | 42 -----------------
 libntp/wscript                                   |  3 --
 ntpd/ntp_control.c                               |  5 +-
 ntpdig/main.c                                    |  3 +-
 ntpq/ntpq-subs.c                                 |  7 +--
 ntpq/ntpq.c                                      | 12 +++--
 ports/winnt/vs2005/libntp.vcproj                 | 12 -----
 ports/winnt/vs2008/libntp/libntp.vcproj          | 12 -----
 ports/winnt/vs2013/libntp/libntp.vcxproj         |  3 --
 ports/winnt/vs2013/libntp/libntp.vcxproj.filters |  9 ----
 tests/libntp/atoint.cpp                          | 59 ------------------------
 tests/libntp/atouint.cpp                         | 51 --------------------
 tests/libntp/hextoint.cpp                        | 53 ---------------------
 16 files changed, 18 insertions(+), 349 deletions(-)

diff --git a/include/ntp_stdlib.h b/include/ntp_stdlib.h
index 7f1ab4b..f7a55ad 100644
--- a/include/ntp_stdlib.h
+++ b/include/ntp_stdlib.h
@@ -154,9 +154,6 @@ extern	char *	estrdup_impl(const char *, const char *, int);
 #endif
 
 
-extern	bool	atoint		(const char *, long *);
-extern	bool	atouint		(const char *, u_long *);
-extern	bool	hextoint	(const char *, u_long *);
 extern	const char *	humanlogtime	(void);
 extern	const char *	humantime	(time_t);
 extern	char *	mfptoa		(uint32_t, uint32_t, short);
diff --git a/libntp/atoint.c b/libntp/atoint.c
deleted file mode 100644
index 20b77df..0000000
--- a/libntp/atoint.c
+++ /dev/null
@@ -1,51 +0,0 @@
-/*
- * atoint - convert an ascii string to a signed long, with error checking
- */
-#include <config.h>
-#include <sys/types.h>
-#include <ctype.h>
-
-#include "ntp_types.h"
-#include "ntp_stdlib.h"
-
-bool
-atoint(
-	const char *str,
-	long *ival
-	)
-{
-	register long u;
-	register const char *cp;
-	register int isneg;
-	register int oflow_digit;
-
-	cp = str;
-
-	if (*cp == '-') {
-		cp++;
-		isneg = 1;
-		oflow_digit = '8';
-	} else {
-		isneg = 0;
-		oflow_digit = '7';
-	}
-
-	if (*cp == '\0')
-	    return false;
-
-	u = 0;
-	while (*cp != '\0') {
-		if (!isdigit((unsigned char)*cp))
-		    return false;
-		if (u > 214748364 || (u == 214748364 && *cp > oflow_digit))
-		    return false;	/* overflow */
-		u = (u << 3) + (u << 1);
-		u += *cp++ - '0';	/* ascii dependent */
-	}
-
-	if (isneg)
-	    *ival = -u;
-	else 
-	    *ival = u;
-	return true;
-}
diff --git a/libntp/atouint.c b/libntp/atouint.c
deleted file mode 100644
index 5bf764d..0000000
--- a/libntp/atouint.c
+++ /dev/null
@@ -1,42 +0,0 @@
-#include <config.h>
-#include <sys/types.h>
-#include <ctype.h>
-
-#include "ntp_types.h"
-#include "ntp_stdlib.h"
-
-/*
- * atouint() - convert an ascii string representing a whole base 10
- *	       number to u_long *uval, returning true if successful.
- *	       Does not modify *uval and returns false if str is not
- *	       a positive base10 integer or is too large for a uint32_t.
- *	       this function uses u_long but should use uint32_t, and
- *	       probably be renamed.
- */
-bool
-atouint(
-	const char *str,
-	u_long *uval
-	)
-{
-	u_long u;
-	const char *cp;
-
-	cp = str;
-	if ('\0' == *cp)
-		return false;
-
-	u = 0;
-	while ('\0' != *cp) {
-		if (!isdigit((unsigned char)*cp))
-			return false;
-		if (u > 429496729 || (u == 429496729 && *cp >= '6'))
-			return false;		/* overflow */
-		/* hand-optimized u *= 10; */
-		u = (u << 3) + (u << 1);
-		u += *cp++ - '0';		/* not '\0' */
-	}
-
-	*uval = u;
-	return true;
-}
diff --git a/libntp/hextoint.c b/libntp/hextoint.c
deleted file mode 100644
index d8cade3..0000000
--- a/libntp/hextoint.c
+++ /dev/null
@@ -1,42 +0,0 @@
-/*
- * hextoint - convert an ascii string in hex to an unsigned
- *	      long, with error checking
- */
-#include <config.h>
-#include <ctype.h>
-
-#include "ntp_stdlib.h"
-
-bool
-hextoint(
-	const char *str,
-	u_long *pu
-	)
-{
-	register u_long u;
-	register const char *cp;
-
-	cp = str;
-
-	if (*cp == '\0')
-		return false;
-
-	u = 0;
-	while (*cp != '\0') {
-		if (!isxdigit((unsigned char)*cp))
-			return false;
-		if (u & 0xF0000000)
-			return false;	/* overflow */
-		u <<= 4;
-		if ('0' <= *cp && *cp <= '9')
-			u += *cp++ - '0';
-		else if ('a' <= *cp && *cp <= 'f')
-			u += *cp++ - 'a' + 10;
-		else if ('A' <= *cp && *cp <= 'F')
-			u += *cp++ - 'A' + 10;
-		else
-			return false;
-	}
-	*pu = u;
-	return true;
-}
diff --git a/libntp/wscript b/libntp/wscript
index 86a1938..4c6a1ed 100644
--- a/libntp/wscript
+++ b/libntp/wscript
@@ -3,9 +3,7 @@ def build(ctx):
 
 	libntp_source = [
 		"a_md5encrypt.c",
-		"atoint.c",
 		"atolfp.c",
-		"atouint.c",
 		"authkeys.c",
 		"authreadkeys.c",
 		"authusekey.c",
@@ -20,7 +18,6 @@ def build(ctx):
 		"dolfptoa.c",
 		"emalloc.c",
 		"getopt.c",
-		"hextoint.c",
 		"hextolfp.c",
 		"humandate.c",
 		"icom.c",
diff --git a/ntpd/ntp_control.c b/ntpd/ntp_control.c
index f0b1630..0eeee80 100644
--- a/ntpd/ntp_control.c
+++ b/ntpd/ntp_control.c
@@ -3178,8 +3178,9 @@ write_variables(
 			ctl_error(CERR_PERMISSION);
 			return;
 		}
-		if (!ext_var && (*valuep == '\0' || !atoint(valuep,
-							    &val))) {
+		errno = 0;
+		if (!ext_var && (*valuep == '\0'
+				 || (val = strtol(valuep, NULL, 10), errno != 0))) {
 			ctl_error(CERR_BADFMT);
 			return;
 		}
diff --git a/ntpdig/main.c b/ntpdig/main.c
index 78440e0..9f7ed5e 100644
--- a/ntpdig/main.c
+++ b/ntpdig/main.c
@@ -514,7 +514,8 @@ handle_lookup(
 	ctx->timeout = response_tv;
 
 	/* The following should arguably be passed in... */
-	if (opt_authkey != NULL && atoint(opt_authkey, &l)) {
+	errno = 0;
+	if (opt_authkey != NULL && (l = strtol(opt_authkey, NULL, 10), errno == 0)) {
 		ctx->key_id = l;
 		get_key(ctx->key_id, &ctx->key);
 	} else {
diff --git a/ntpq/ntpq-subs.c b/ntpq/ntpq-subs.c
index 313974f..f5a6d64 100644
--- a/ntpq/ntpq-subs.c
+++ b/ntpq/ntpq-subs.c
@@ -3713,7 +3713,8 @@ collect_display_vdc(
 						"malformed %s=%s\n",
 						pvdc->tag, val);
 			} else {	/* port */
-				if (atouint(val, &ul))
+				errno = 0;
+				if (ul = strtoul(val, NULL, 10), errno == 0)
 					SET_PORT(&pvdc->v.sau, 
 						 (u_short)ul);
 			}
@@ -3754,13 +3755,13 @@ collect_display_vdc(
 			break;
 
 		case NTP_MODE:
-			atouint(pvdc->v.str, &ul);
+			ul = strtoul(pvdc->v.str, NULL, 10);
 			fprintf(fp, "%s  %s\n", pvdc->display,
 				modetoa((int)ul));
 			break;
 
 		case NTP_2BIT:
-			atouint(pvdc->v.str, &ul);
+			ul = strtoul(pvdc->v.str, NULL, 10);
 			fprintf(fp, "%s  %s\n", pvdc->display,
 				leapbits[ul & 0x3]);
 			break;
diff --git a/ntpq/ntpq.c b/ntpq/ntpq.c
index 3b80bbc..a8eb0e6 100644
--- a/ntpq/ntpq.c
+++ b/ntpq/ntpq.c
@@ -1713,7 +1713,9 @@ getarg(
 
 	case NTP_UINT:
 		if ('&' == str[0]) {
-			if (!atouint(&str[1], &ul)) {
+			errno = 0;
+			ul = strtoul(&str[1], NULL, 10);
+			if (errno == EINVAL || errno == ERANGE) {
 				fprintf(stderr,
 					"***Association index `%s' invalid/undecodable\n",
 					str);
@@ -1732,7 +1734,9 @@ getarg(
 			argp->uval = assoc_cache[ul - 1].assid;
 			break;
 		}
-		if (!atouint(str, &argp->uval)) {
+		errno = 0;
+		argp->uval = strtoul(str, NULL, 10);
+		if (errno == EINVAL || errno == ERANGE) {
 			fprintf(stderr, "***Illegal unsigned value %s\n",
 				str);
 			return false;
@@ -1740,7 +1744,9 @@ getarg(
 		break;
 
 	case NTP_INT:
-		if (!atoint(str, &argp->ival)) {
+		errno = 0;
+		argp->ival = strtol(str, NULL, 10);
+		if (errno == EINVAL || errno == ERANGE) {
 			fprintf(stderr, "***Illegal integer value %s\n",
 				str);
 			return false;
diff --git a/ports/winnt/vs2005/libntp.vcproj b/ports/winnt/vs2005/libntp.vcproj
index 4988ba9..c0306f0 100644
--- a/ports/winnt/vs2005/libntp.vcproj
+++ b/ports/winnt/vs2005/libntp.vcproj
@@ -189,18 +189,10 @@
 				>
 			</File>
 			<File
-				RelativePath="..\..\..\libntp\atoint.c"
-				>
-			</File>
-			<File
 				RelativePath="..\..\..\libntp\atolfp.c"
 				>
 			</File>
 			<File
-				RelativePath="..\..\..\libntp\atouint.c"
-				>
-			</File>
-			<File
 				RelativePath="..\..\..\libntp\authkeys.c"
 				>
 			</File>
@@ -289,10 +281,6 @@
 				>
 			</File>
 			<File
-				RelativePath="..\..\..\libntp\hextoint.c"
-				>
-			</File>
-			<File
 				RelativePath="..\..\..\libntp\hextolfp.c"
 				>
 			</File>
diff --git a/ports/winnt/vs2008/libntp/libntp.vcproj b/ports/winnt/vs2008/libntp/libntp.vcproj
index 3033f93..d38242b 100644
--- a/ports/winnt/vs2008/libntp/libntp.vcproj
+++ b/ports/winnt/vs2008/libntp/libntp.vcproj
@@ -288,18 +288,10 @@
 				>
 			</File>
 			<File
-				RelativePath="..\..\..\..\libntp\atoint.c"
-				>
-			</File>
-			<File
 				RelativePath="..\..\..\..\libntp\atolfp.c"
 				>
 			</File>
 			<File
-				RelativePath="..\..\..\..\libntp\atouint.c"
-				>
-			</File>
-			<File
 				RelativePath="..\..\..\..\libntp\audio.c"
 				>
 			</File>
@@ -392,10 +384,6 @@
 				>
 			</File>
 			<File
-				RelativePath="..\..\..\..\libntp\hextoint.c"
-				>
-			</File>
-			<File
 				RelativePath="..\..\..\..\libntp\hextolfp.c"
 				>
 			</File>
diff --git a/ports/winnt/vs2013/libntp/libntp.vcxproj b/ports/winnt/vs2013/libntp/libntp.vcxproj
index 1b9792c..690d3e1 100644
--- a/ports/winnt/vs2013/libntp/libntp.vcxproj
+++ b/ports/winnt/vs2013/libntp/libntp.vcxproj
@@ -213,9 +213,7 @@
   </ItemDefinitionGroup>
   <ItemGroup>
     <ClCompile Include="..\..\..\..\libntp\adjtime.c" />
-    <ClCompile Include="..\..\..\..\libntp\atoint.c" />
     <ClCompile Include="..\..\..\..\libntp\atolfp.c" />
-    <ClCompile Include="..\..\..\..\libntp\atouint.c" />
     <ClCompile Include="..\..\..\..\libntp\audio.c" />
     <ClCompile Include="..\..\..\..\libntp\authkeys.c" />
     <ClCompile Include="..\..\..\..\libntp\authreadkeys.c" />
@@ -233,7 +231,6 @@
     <ClCompile Include="..\..\..\..\libntp\emalloc.c" />
     <ClCompile Include="..\..\..\..\libntp\findconfig.c" />
     <ClCompile Include="..\..\..\..\libntp\getopt.c" />
-    <ClCompile Include="..\..\..\..\libntp\hextoint.c" />
     <ClCompile Include="..\..\..\..\libntp\hextolfp.c" />
     <ClCompile Include="..\..\..\..\libntp\humandate.c" />
     <ClCompile Include="..\..\..\..\libntp\icom.c" />
diff --git a/ports/winnt/vs2013/libntp/libntp.vcxproj.filters b/ports/winnt/vs2013/libntp/libntp.vcxproj.filters
index 575a21f..c807a40 100644
--- a/ports/winnt/vs2013/libntp/libntp.vcxproj.filters
+++ b/ports/winnt/vs2013/libntp/libntp.vcxproj.filters
@@ -29,15 +29,9 @@
     <ClCompile Include="..\..\..\..\lib\isc\assertions.c">
       <Filter>Source Files</Filter>
     </ClCompile>
-    <ClCompile Include="..\..\..\..\libntp\atoint.c">
-      <Filter>Source Files</Filter>
-    </ClCompile>
     <ClCompile Include="..\..\..\..\libntp\atolfp.c">
       <Filter>Source Files</Filter>
     </ClCompile>
-    <ClCompile Include="..\..\..\..\libntp\atouint.c">
-      <Filter>Source Files</Filter>
-    </ClCompile>
     <ClCompile Include="..\..\..\..\libntp\audio.c">
       <Filter>Source Files</Filter>
     </ClCompile>
@@ -107,9 +101,6 @@
     <ClCompile Include="..\..\..\..\libntp\getopt.c">
       <Filter>Source Files</Filter>
     </ClCompile>
-    <ClCompile Include="..\..\..\..\libntp\hextoint.c">
-      <Filter>Source Files</Filter>
-    </ClCompile>
     <ClCompile Include="..\..\..\..\libntp\hextolfp.c">
       <Filter>Source Files</Filter>
     </ClCompile>
diff --git a/tests/libntp/atoint.cpp b/tests/libntp/atoint.cpp
deleted file mode 100644
index 9f31b4c..0000000
--- a/tests/libntp/atoint.cpp
+++ /dev/null
@@ -1,59 +0,0 @@
-extern "C" {
-#include "unity.h"
-#include "unity_fixture.h"
-}
-
-TEST_GROUP(atoint);
-
-TEST_SETUP(atoint) {}
-
-TEST_TEAR_DOWN(atoint) {}
-
-#include "libntptest.h"
-
-class atointTest : public libntptest {
-};
-
-TEST(atoint, RegularPositive) {
-	const char *str = "17";
-	long val;
-
-	TEST_ASSERT_TRUE(atoint(str, &val));
-	TEST_ASSERT_EQUAL(17, val);
-}
-
-TEST(atoint, RegularNegative) {
-	const char *str = "-20";
-	long val;
-
-	TEST_ASSERT_TRUE(atoint(str, &val));
-	TEST_ASSERT_EQUAL(-20, val);
-}
-
-TEST(atoint, PositiveOverflowBoundary) {
-	const char *str = "2147483648";
-	long val;
-
-	TEST_ASSERT_FALSE(atoint(str, &val));
-}
-
-TEST(atoint, NegativeOverflowBoundary) {
-	const char *str = "-2147483649";
-	long val;
-
-	TEST_ASSERT_FALSE(atoint(str, &val));
-}
-
-TEST(atoint, PositiveOverflowBig) {
-	const char *str = "2300000000";
-	long val;
-
-	TEST_ASSERT_FALSE(atoint(str, &val));
-}
-
-TEST(atoint, IllegalCharacter) {
-	const char *str = "4500l";
-	long val;
-
-	TEST_ASSERT_FALSE(atoint(str, &val));
-}
diff --git a/tests/libntp/atouint.cpp b/tests/libntp/atouint.cpp
deleted file mode 100644
index 50c62e8..0000000
--- a/tests/libntp/atouint.cpp
+++ /dev/null
@@ -1,51 +0,0 @@
-extern "C" {
-#include "unity.h"
-#include "unity_fixture.h"
-}
-
-TEST_GROUP(atouint);
-
-TEST_SETUP(atouint) {}
-
-TEST_TEAR_DOWN(atouint) {}
-
-#include "libntptest.h"
-
-class atouintTest : public libntptest {
-};
-
-TEST(atouint, RegularPositive) {
-	const char *str = "305";
-	u_long actual;
-
-	TEST_ASSERT_TRUE(atouint(str, &actual));
-	TEST_ASSERT_EQUAL(305, actual);
-}
-
-TEST(atouint, PositiveOverflowBoundary) {
-	const char *str = "4294967296";
-	u_long actual;
-
-	TEST_ASSERT_FALSE(atouint(str, &actual));
-}
-
-TEST(atouint, PositiveOverflowBig) {
-	const char *str = "8000000000";
-	u_long actual;
-
-	TEST_ASSERT_FALSE(atouint(str, &actual));
-}
-
-TEST(atouint, Negative) {
-	const char *str = "-1";
-	u_long actual;
-
-	TEST_ASSERT_FALSE(atouint(str, &actual));
-}
-
-TEST(atouint, IllegalChar) {
-	const char *str = "50c3";
-	u_long actual;
-
-	TEST_ASSERT_FALSE(atouint(str, &actual));
-}
diff --git a/tests/libntp/hextoint.cpp b/tests/libntp/hextoint.cpp
deleted file mode 100644
index cdef0b1..0000000
--- a/tests/libntp/hextoint.cpp
+++ /dev/null
@@ -1,53 +0,0 @@
-extern "C" {
-#include "unity.h"
-#include "unity_fixture.h"
-}
-
-TEST_GROUP(hextoint);
-
-TEST_SETUP(hextoint) {}
-
-TEST_TEAR_DOWN(hextoint) {}
-
-#include "libntptest.h"
-
-class hextointTest : public libntptest {
-};
-
-TEST(hextoint, SingleDigit) {
-	const char *str = "a"; // 10 decimal
-	u_long actual;
-
-	TEST_ASSERT_TRUE(hextoint(str, &actual));
-	TEST_ASSERT_EQUAL(10, actual);
-}
-
-TEST(hextoint, MultipleDigits) {
-	const char *str = "8F3"; // 2291 decimal
-	u_long actual;
-
-	TEST_ASSERT_TRUE(hextoint(str, &actual));
-	TEST_ASSERT_EQUAL(2291, actual);
-}
-
-TEST(hextoint, MaxUnsigned) {
-	const char *str = "ffffffff"; // 4294967295 decimal
-	u_long actual;
-
-	TEST_ASSERT_TRUE(hextoint(str, &actual));
-	TEST_ASSERT_EQUAL(4294967295UL, actual);
-}
-
-TEST(hextoint, Overflow) {
-	const char *str = "100000000"; // Overflow by 1
-	u_long actual;
-
-	TEST_ASSERT_FALSE(hextoint(str, &actual));
-}
-
-TEST(hextoint, IllegalChar) {
-	const char *str = "5gb"; // Illegal character g
-	u_long actual;
-
-	TEST_ASSERT_FALSE(hextoint(str, &actual));
-}



More information about the vc mailing list