svn commit: r272144 - head/sbin/sysctl

Xin Li delphij at delphij.net
Fri Sep 26 00:19:43 UTC 2014


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 09/25/14 15:40, Mateusz Guzik wrote:
> On Thu, Sep 25, 2014 at 10:37:28PM +0000, Xin LI wrote:
>> Author: delphij Date: Thu Sep 25 22:37:27 2014 New Revision:
>> 272144 URL: http://svnweb.freebsd.org/changeset/base/272144
>> 
>> Log: The strtol(3) family of functions would set errno when it
>> hits one. Check errno and handle it as invalid input.
>> 
> 
> But this requires explicitely setting errno to 0 before strto*
> call, otherwise you cannot know whether errno is meaningful when
> you test it.

Fixed in r272145, good catch!

> Also it looks like the code would use some deduplications with
> macros or something.

I think the code can use some refactor.  Does the attached patch look
good to you?

Cheers,
- -- 
Xin LI <delphij at delphij.net>    https://www.delphij.net/
FreeBSD - The Power to Serve!           Live free or die
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0

iQIcBAEBCgAGBQJUJLEeAAoJEJW2GBstM+nsSD0P/3Oe4gzNc1AYy14cg8ewZ0nL
w75gPr9rMABxMXk7tGut6qS+W1vTZ32g9rCpzE+7ra1A8kDZ9l+VkJbUwkN2nr/i
udsMq+qQ1qB+dCsVJLm8Pjsh/g+AVlSxXdHrkD1u49XhANZQUGt9NBaO0ah84c5J
vaZ+Sq7cR6fF2VrZDJ0MLxW8tHLgW6hIVORvyiohvOqhzBHOFihDEDcoUOamzFZI
txqI2oxjuhGy+T1V4dVYso/PgYhhnFC4L9Lvx5E9EqUZrVzckPP1PYXiZiQR/i7H
ZEV5RgxHOGXEYNVDZXoEQthE1iP3pzFhe/xNURzlUqZIwgPeJ6Q2xz+As/C0yXlT
+LxmXdQ/zsQmRu5b2m7TvBjBp18znAR2c2mKbvHS3EX6UWEmUKwcb2CREm0AbnQL
kFrLRsksJfxD1TCkfmqysOa6WSkvRQpJ4csZdKf9y5nZFh/yjdSnzJULLKphpbzr
npc9ZYJ9m7bLj7OZF5UPwEr0JfTnRDn6VnMqKiUTgXpHk5Lpm7vm5WefdOWMyTmL
nwtmU3SyaJdLBrWARY1ZyN0ENmzTrs3sjCyWvhwlAomYIVuahT1oCuaRYIDyzito
mThiFbuRA+ufz6m8OcIXeiGLxkimdIHm68ZreNZlBHA/R/bH84vLiIN0HRNhgAOy
bgc97yetD5PL8NhRC8G/
=PWTt
-----END PGP SIGNATURE-----
-------------- next part --------------
Index: sysctl.c
===================================================================
--- sysctl.c	(revision 272145)
+++ sysctl.c	(working copy)
@@ -57,6 +57,7 @@ static const char rcsid[] =
 #include <machine/pc/bios.h>
 #endif
 
+#include <assert.h>
 #include <ctype.h>
 #include <err.h>
 #include <errno.h>
@@ -80,8 +81,32 @@ static int	show_var(int *, int);
 static int	sysctl_all(int *oid, int len);
 static int	name2oid(const char *, int *);
 
-static int	set_IK(const char *, int *);
+static int	strIKtoi(const char *, char **);
 
+static int ctl_sign[CTLTYPE+1] = {
+	[CTLTYPE_INT] = 1,
+	[CTLTYPE_LONG] = 1,
+	[CTLTYPE_S64] = 1,
+};
+
+static int ctl_size[CTLTYPE+1] = {
+	[CTLTYPE_INT] = sizeof(int),
+	[CTLTYPE_UINT] = sizeof(u_int),
+	[CTLTYPE_LONG] = sizeof(long),
+	[CTLTYPE_ULONG] = sizeof(u_long),
+	[CTLTYPE_S64] = sizeof(int64_t),
+	[CTLTYPE_U64] = sizeof(uint64_t),
+};
+
+static const char *ctl_typename[CTLTYPE+1] = {
+	[CTLTYPE_INT] = "integer",
+	[CTLTYPE_UINT] = "unsigned integer",
+	[CTLTYPE_LONG] = "long integer",
+	[CTLTYPE_ULONG] = "unsigned long",
+	[CTLTYPE_S64] = "int64_t",
+	[CTLTYPE_U64] = "uint64_t",
+};
+
 static void
 usage(void)
 {
@@ -191,7 +216,8 @@ static int
 parse(const char *string, int lineno)
 {
 	int len, i, j;
-	void *newval = 0;
+	const void *newval = NULL;
+	const char *newvalstr = NULL;
 	int intval;
 	unsigned int uintval;
 	long longval;
@@ -230,7 +256,7 @@ parse(const char *string, int lineno)
 				cp[strlen(cp) - 1] = '\0';
 			cp++;
 		}
-		newval = cp;
+		newvalstr = cp;
 		newsize = strlen(cp);
 	}
 	len = name2oid(bufp, mib);
@@ -254,7 +280,7 @@ parse(const char *string, int lineno)
 			exit(1);
 	}
 
-	if (newval == NULL || dflag) {
+	if (newvalstr == NULL || dflag) {
 		if ((kind & CTLTYPE) == CTLTYPE_NODE) {
 			if (dflag) {
 				i = show_var(mib, len);
@@ -288,10 +314,22 @@ parse(const char *string, int lineno)
 		    (kind & CTLTYPE) == CTLTYPE_ULONG ||
 		    (kind & CTLTYPE) == CTLTYPE_S64 ||
 		    (kind & CTLTYPE) == CTLTYPE_U64) {
-			if (strlen(newval) == 0) {
+			if (strlen(newvalstr) == 0) {
 				warnx("empty numeric value");
 				return (1);
 			}
+		} else if ((kind & CTLTYPE) != CTLTYPE_STRING) {
+			/*
+			 * The only acceptable types are:
+			 * CTLTYPE_INT, CTLTYPE_UINT,
+			 * CTLTYPE_LONG, CTLTYPE_ULONG,
+			 * CTLTYPE_S64, CTLTYPE_U64 and
+			 * CTLTYPE_STRING.
+			 */
+			warnx("oid '%s' is type %d,"
+				" cannot set that%s", bufp,
+				kind & CTLTYPE, line);
+			return (1);
 		}
 
 		errno = 0;
@@ -298,91 +336,53 @@ parse(const char *string, int lineno)
 
 		switch (kind & CTLTYPE) {
 			case CTLTYPE_INT:
-				if (strcmp(fmt, "IK") == 0) {
-					if (!set_IK(newval, &intval)) {
-						warnx("invalid value '%s'%s",
-						    (char *)newval, line);
-						return (1);
-					}
- 				} else {
-					intval = (int)strtol(newval, &endptr,
+				if (strcmp(fmt, "IK") == 0)
+					intval = strIKtoi(newvalstr, &endptr);
+ 				else
+					intval = (int)strtol(newvalstr, &endptr,
 					    0);
-					if (errno != 0 || endptr == newval ||
-						*endptr != '\0') {
-						warnx("invalid integer '%s'%s",
-						    (char *)newval, line);
-						return (1);
-					}
-				}
 				newval = &intval;
 				newsize = sizeof(intval);
 				break;
 			case CTLTYPE_UINT:
-				uintval = (int) strtoul(newval, &endptr, 0);
-				if (errno != 0 || endptr == newval ||
-					*endptr != '\0') {
-					warnx("invalid unsigned integer '%s'%s",
-					    (char *)newval, line);
-					return (1);
-				}
+				uintval = (int) strtoul(newvalstr, &endptr, 0);
 				newval = &uintval;
 				newsize = sizeof(uintval);
 				break;
 			case CTLTYPE_LONG:
-				longval = strtol(newval, &endptr, 0);
-				if (errno != 0 || endptr == newval ||
-					*endptr != '\0') {
-					warnx("invalid long integer '%s'%s",
-					    (char *)newval, line);
-					return (1);
-				}
+				longval = strtol(newvalstr, &endptr, 0);
 				newval = &longval;
 				newsize = sizeof(longval);
 				break;
 			case CTLTYPE_ULONG:
-				ulongval = strtoul(newval, &endptr, 0);
-				if (errno != 0 || endptr == newval ||
-					*endptr != '\0') {
-					warnx("invalid unsigned long integer"
-					    " '%s'%s", (char *)newval, line);
-					return (1);
-				}
+				ulongval = strtoul(newvalstr, &endptr, 0);
 				newval = &ulongval;
 				newsize = sizeof(ulongval);
 				break;
 			case CTLTYPE_STRING:
+				newval = newvalstr;
 				break;
 			case CTLTYPE_S64:
-				i64val = strtoimax(newval, &endptr, 0);
-				if (errno != 0 || endptr == newval ||
-					*endptr != '\0') {
-					warnx("invalid int64_t '%s'%s",
-					    (char *)newval, line);
-					return (1);
-				}
+				i64val = strtoimax(newvalstr, &endptr, 0);
 				newval = &i64val;
 				newsize = sizeof(i64val);
 				break;
 			case CTLTYPE_U64:
-				u64val = strtoumax(newval, &endptr, 0);
-				if (errno != 0 || endptr == newval ||
-					*endptr != '\0') {
-					warnx("invalid uint64_t '%s'%s",
-					    (char *)newval, line);
-					return (1);
-				}
+				u64val = strtoumax(newvalstr, &endptr, 0);
 				newval = &u64val;
 				newsize = sizeof(u64val);
 				break;
-			case CTLTYPE_OPAQUE:
-				/* FALLTHROUGH */
 			default:
-				warnx("oid '%s' is type %d,"
-					" cannot set that%s", bufp,
-					kind & CTLTYPE, line);
+				/* NOTREACHED */
 				return (1);
 		}
 
+		if (errno != 0 || endptr == newvalstr || *endptr != '\0') {
+			warnx("invalid %s '%s'%s", ctl_typename[kind & CTLTYPE],
+			    newvalstr, line);
+			return (1);
+		}
+
 		i = show_var(mib, len);
 		if (sysctl(mib, len, 0, 0, newval, newsize) == -1) {
 			if (!i && !bflag)
@@ -665,33 +665,35 @@ S_bios_smap_xattr(size_t l2, void *p)
 #endif
 
 static int
-set_IK(const char *str, int *val)
+strIKtoi(const char *str, char **endptrp)
 {
+	int kelv;
 	float temp;
-	int len, kelv;
+	size_t len;
 	const char *p;
-	char *endptr;
 
-	if ((len = strlen(str)) == 0)
-		return (0);
+	assert(errno == 0);
+
+	len = strlen(str);
+	/* caller already checked this */
+	assert(len > 0);
+
 	p = &str[len - 1];
-	errno = 0;
 	if (*p == 'C' || *p == 'F') {
-		temp = strtof(str, &endptr);
-		if (errno != 0 || endptr == str ||
-			endptr != p)
-			return (0);
-		if (*p == 'F')
-			temp = (temp - 32) * 5 / 9;
-		kelv = temp * 10 + 2732;
+		temp = strtof(str, endptrp);
+		if (*endptrp != str && *endptrp == p && errno != 0) {
+			if (*p == 'F')
+				temp = (temp - 32) * 5 / 9;
+			return (temp * 10 + 2732);
+		}
 	} else {
-		kelv = (int)strtol(str, &endptr, 10);
-		if (errno != 0 || endptr == str ||
-			*endptr != '\0')
-			return (0);
+		kelv = (int)strtol(str, endptrp, 10);
+		if (*endptrp != str && *endptrp == p && errno != 0)
+			return (kelv);
 	}
-	*val = kelv;
-	return (1);
+
+	errno = ERANGE;
+	return (0);
 }
 
 /*
@@ -746,21 +748,6 @@ oidfmt(int *oid, int len, char *fmt, u_int *kind)
 	return (0);
 }
 
-static int ctl_sign[CTLTYPE+1] = {
-	[CTLTYPE_INT] = 1,
-	[CTLTYPE_LONG] = 1,
-	[CTLTYPE_S64] = 1,
-};
-
-static int ctl_size[CTLTYPE+1] = {
-	[CTLTYPE_INT] = sizeof(int),
-	[CTLTYPE_UINT] = sizeof(u_int),
-	[CTLTYPE_LONG] = sizeof(long),
-	[CTLTYPE_ULONG] = sizeof(u_long),
-	[CTLTYPE_S64] = sizeof(int64_t),
-	[CTLTYPE_U64] = sizeof(int64_t),
-};
-
 /*
  * This formats and outputs the value of one variable
  *
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sysctl.diff.sig
Type: application/octet-stream
Size: 543 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20140925/2cedcf0d/attachment.obj>


More information about the svn-src-all mailing list