Re: sysctl warns6 cleanup

[Chris Pressey <cpressey@xxxxxxxxxxxxxxx>]

On Sat, 8 Jan 2005 20:26:21 +0100
Joerg Sonnenberger <joerg@xxxxxxxxxxxxxxxxx> wrote:

> On Sat, Jan 08, 2005 at 11:24:06AM -0800, Chris Pressey wrote:
> > On Sat, 8 Jan 2005 20:02:30 +0100
> > Joerg Sonnenberger <joerg@xxxxxxxxxxxxxxxxx> wrote:
> > 
> > > On Sat, Jan 08, 2005 at 05:17:17PM +0100, Peter Schuller wrote:
> > > > @@ -518,20 +519,20 @@
> > > >  	case 'A':
> > > >  		if (!nflag)
> > > >  			printf("%s%s", name, sep);
> > > > -		printf("%.*s", len, p);
> > > > +		printf("%.*s", (int)len, p);
> > > >  		return (0);
> > > >  		
> > > 
> > > There should be at least a check for the value of len, to ensure
> > > that it actually fits into an int.
> > 
> > Heh... come to think of it, isn't this kind of a silly use of
> > printf()? Couldn't we just use fwrite(p, len, 1, stdout) here
> > instead?
> 
> Yes, that's better.

OK, here is an updated patch.

I moved a little bit of code around to try to get all the functions
which should use size_t to actually use size_t and not just pass off
ints as size_t's  :)  Tell me if it looks like I've missed anything.

-Chris

Index: Makefile
===================================================================
RCS file: /home/dcvs/src/sbin/sysctl/Makefile,v
retrieving revision 1.2
diff -u -r1.2 Makefile
--- Makefile	17 Jun 2003 04:27:34 -0000	1.2
+++ Makefile	7 Jan 2005 03:06:34 -0000
@@ -4,5 +4,6 @@
 
 PROG=	sysctl
 MAN=	sysctl.8
+WARNS?=	6
 
 .include <bsd.prog.mk>
Index: sysctl.c
===================================================================
RCS file: /home/dcvs/src/sbin/sysctl/sysctl.c,v
retrieving revision 1.6
diff -u -r1.6 sysctl.c
--- sysctl.c	18 Dec 2004 21:43:46 -0000	1.6
+++ sysctl.c	8 Jan 2005 20:02:20 -0000
@@ -56,13 +56,13 @@
 
 static int	aflag, bflag, dflag, eflag, Nflag, nflag, oflag, xflag;
 
-static int	oidfmt(int *, int, char *, u_int *);
-static void	parse(char *);
-static int	show_var(int *, int);
-static int	sysctl_all (int *oid, int len);
-static int	name2oid(char *, int *);
+static int	oidfmt(int *, size_t, char *, u_int *);
+static void	parse(const char *);
+static int	show_var(int *, size_t);
+static int	sysctl_all(int *, size_t);
+static size_t	name2oid(char *, int *);
 
-static void	set_T_dev_t (char *, void **, int *);
+static void	set_T_dev_t (const char *, void **, int *);
 
 static void
 usage(void)
@@ -143,9 +143,10 @@
  * Set a new value if requested.
  */
 static void
-parse(char *string)
+parse(const char *string)
 {
-	int len, i, j;
+	size_t len;
+	int i, j;
 	void *newval = 0;
 	int intval;
 	unsigned int uintval;
@@ -169,9 +170,6 @@
 	}
 	len = name2oid(bufp, mib);
 
-	if (len < 0) 
-		errx(1, "unknown oid '%s'", bufp);
-
 	if (oidfmt(mib, len, fmt, &kind))
 		err(1, "couldn't find format of oid '%s'", bufp);
 
@@ -326,7 +324,7 @@
 }
 
 static void
-set_T_dev_t (char *path, void **val, int *size)
+set_T_dev_t(const char *path, void **val, int *size)
 {
 	static struct stat statb;
 
@@ -355,7 +353,7 @@
  * may seem, there are various conflicting requirements.
  */
 
-static int
+static size_t
 name2oid(char *name, int *oidp)
 {
 	int oid[2];
@@ -367,14 +365,14 @@
 
 	j = CTL_MAXNAME * sizeof(int);
 	i = sysctl(oid, 2, oidp, &j, name, strlen(name));
-	if (i < 0) 
-		return i;
+	if (i < 0)
+		errx(1, "unknown oid '%s'", name);
 	j /= sizeof(int);
 	return (j);
 }
 
 static int
-oidfmt(int *oid, int len, char *fmt, u_int *kind)
+oidfmt(int *oid, size_t len, char *fmt, u_int *kind)
 {
 	int qoid[CTL_MAXNAME+2];
 	u_char buf[BUFSIZ];
@@ -405,7 +403,7 @@
  */
 struct _foo {
 	int majdev;
-	char *name;
+	const char *name;
 } maj2name[] = {
 	{ 30,	"ad" },
 	{ 0,	"wd" },
@@ -456,10 +454,11 @@
  */
 
 static int
-show_var(int *oid, int nlen)
+show_var(int *oid, size_t nlen)
 {
 	u_char buf[BUFSIZ], *val, *p;
-	char name[BUFSIZ], *fmt, *sep;
+	char name[BUFSIZ], *fmt;
+	const char *sep, *spacer;
 	int qoid[CTL_MAXNAME+2];
 	int i;
 	size_t j, len;
@@ -518,20 +517,20 @@
 	case 'A':
 		if (!nflag)
 			printf("%s%s", name, sep);
-		printf("%.*s", len, p);
+		fwrite(p, len, 1, stdout);
 		return (0);
 		
 	case 'I':
 		if (!nflag)
 			printf("%s%s", name, sep);
 		fmt++;
-		val = "";
+		spacer = "";
 		while (len >= sizeof(int)) {
 			if(*fmt == 'U')
-				printf("%s%u", val, *(unsigned int *)p);
+				printf("%s%u", spacer, *(unsigned int *)p);
 			else
-				printf("%s%d", val, *(int *)p);
-			val = " ";
+				printf("%s%d", spacer, *(int *)p);
+			spacer = " ";
 			len -= sizeof(int);
 			p += sizeof(int);
 		}
@@ -545,13 +544,13 @@
 		if (!strcmp(name, "machdep.guessed_bootdev"))
 			return machdep_bootdev(*(unsigned long *)p);
 #endif
-		val = "";
+		spacer = "";
 		while (len >= sizeof(long)) {
 			if(*fmt == 'U')
-				printf("%s%lu", val, *(unsigned long *)p);
+				printf("%s%lu", spacer, *(unsigned long *)p);
 			else
-				printf("%s%ld", val, *(long *)p);
-			val = " ";
+				printf("%s%ld", spacer, *(long *)p);
+			spacer = " ";
 			len -= sizeof(long);
 			p += sizeof(long);
 		}
@@ -567,13 +566,13 @@
 		if (!nflag)
 			printf("%s%s", name, sep);
 		fmt++;
-		val = "";
+		spacer = "";
 		while (len >= sizeof(int64_t)) {
 			if(*fmt == 'U')
-				printf("%s%llu", val, (long long)*(u_int64_t *)p);
+				printf("%s%llu", spacer, (long long)*(u_int64_t *)p);
 			else
-				printf("%s%lld", val, (long long)*(int64_t *)p);
-			val = " ";
+				printf("%s%lld", spacer, (long long)*(int64_t *)p);
+			spacer = " ";
 			len -= sizeof(int64_t);
 			p += sizeof(int64_t);
 		}
@@ -616,10 +615,11 @@
 }
 
 static int
-sysctl_all (int *oid, int len)
+sysctl_all(int *oid, size_t len)
 {
 	int name1[22], name2[22];
 	int i, j;
+	unsigned int k;
 	size_t l1, l2;
 
 	name1[0] = 0;
@@ -647,8 +647,8 @@
 		if (l2 < len)
 			return 0;
 
-		for (i = 0; i < len; i++)
-			if (name2[i] != oid[i])
+		for (k = 0; k < len; k++)
+			if (name2[k] != oid[k])
 				return 0;
 
 		i = show_var(name2, l2);