Bring in some iostat fixes that bde reminded me about. These fixes were

originally written in January, 2000, but have been substantially updated.

- No longer use hz/stathz and the CPU times in computing the TTY stats,
  but rather use etime, like the disk stats.
- Clean up malloc/realloc failure tests.
- Use a new integrated routine to fetch devstat information via sysctl or
  KVM.
- Get rid of the X() macro for calculating CPU stats
- Use rint() on the CPU state display to avoid truncation errors.  (this
  requires libm)
- Clean up flag usage somewhat.

Reviewed by:	bde

2 files changed, 125 insertions, 95 deletions

diff --git a/usr.sbin/iostat/Makefile b/usr.sbin/iostat/Makefile
index c6903b6..002a250 100644
--- a/usr.sbin/iostat/Makefile
+++ b/usr.sbin/iostat/Makefile
@@ -4,9 +4,8 @@
 MAINTAINER=	ken@FreeBSD.ORG
 
 PROG=	iostat
+DPADD=	${LIBKVM} ${LIBDEVSTAT} ${LIBM}
+LDADD=	-lkvm -ldevstat -lm
 MAN=	iostat.8
 
-DPADD=	${LIBKVM} ${LIBDEVSTAT}
-LDADD=	-lkvm -ldevstat
-
 .include <bsd.prog.mk>
diff --git a/usr.sbin/iostat/iostat.c b/usr.sbin/iostat/iostat.c
index 8a8cdc8..3611968 100644
--- a/usr.sbin/iostat/iostat.c
+++ b/usr.sbin/iostat/iostat.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 1998  Kenneth D. Merry.
+ * Copyright (c) 1997, 1998, 2000, 2001  Kenneth D. Merry
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -115,6 +115,7 @@
 #include <unistd.h>
 #include <limits.h>
 #include <devstat.h>
+#include <math.h>
 
 struct nlist namelist[] = {
 #define X_TK_NIN	0
@@ -123,11 +124,9 @@ struct nlist namelist[] = {
 	{ "_tk_nout" },
 #define X_CP_TIME	2
 	{ "_cp_time" },
-#define X_HZ		3
-	{ "_hz" },
-#define X_STATHZ	4
-	{ "_stathz" },
-#define X_END		4
+#define X_BOOTTIME	3
+	{ "_boottime" },
+#define X_END		3
 	{ NULL },
 };
 
@@ -137,15 +136,13 @@ struct device_selection *dev_select;
 int maxshowdevs;
 int dflag = 0, Iflag = 0, Cflag = 0, Tflag = 0, oflag = 0, Kflag = 0;
 
-#define nlread(x, v) \
-	kvm_read(kd, namelist[x].n_value, &(v), sizeof(v))
-
 /* local function declarations */
 static void usage(void);
 static void phdr(int signo);
-static void devstats(int perf_select,double etime, int havelast);
+static void devstats(int perf_select, long double etime, int havelast);
 static void cpustats(void);
-static void getsysctl(const char *, void *, size_t);
+static int readvar(kvm_t *kd, const char *name, int nlid, void *ptr,
+		   size_t len);
 
 static void
 usage(void)
@@ -173,7 +170,6 @@ main(int argc, char **argv)
 	int num_matches = 0;
         char errbuf[_POSIX2_LINE_MAX];
 	kvm_t *kd = NULL;
-	int hz, stathz;
 	int headercount;
 	long generation;
 	int num_devices_specified;
@@ -181,8 +177,7 @@ main(int argc, char **argv)
 	long select_generation;
 	char **specified_devices;
 	devstat_select_mode select_mode;
-	int use_kvm, havelast = 0;
-	struct clockinfo clkinfo;
+	int havelast = 0;
 
 	matches = NULL;
 	maxshowdevs = 3;
@@ -251,24 +246,14 @@ main(int argc, char **argv)
 	argc -= optind;
 	argv += optind;
 
-	if (nlistf == NULL && memf == NULL) {
-		use_kvm = 0;
-		getsysctl("kern.clockrate", &clkinfo, sizeof(clkinfo));
-		hz = clkinfo.hz;
-	} else {
-		use_kvm = 1;
+	if (nlistf != NULL || memf != NULL) {
 		kd = kvm_openfiles(nlistf, memf, NULL, O_RDONLY, errbuf);
 		
-		if (kd == 0)
+		if (kd == NULL)
 			errx(1, "kvm_openfiles: %s", errbuf);
 		
 		if (kvm_nlist(kd, namelist) == -1)
 			errx(1, "kvm_nlist: %s", kvm_geterr(kd));
-
-		(void)nlread(X_HZ, hz);
-		(void)nlread(X_STATHZ, stathz);
-		if (stathz)
-			hz = stathz;
 	}
 
 	/*
@@ -280,6 +265,15 @@ main(int argc, char **argv)
 		errx(1, "%s", devstat_errbuf);
 
 	/*
+	 * Make sure Tflag and/or Cflag are set if dflag == 0.  If dflag is
+	 * greater than 0, they may be 0 or non-zero.
+	 */
+	if (dflag == 0) {
+		Cflag = 1;
+		Tflag = 1;
+	}
+
+	/*
 	 * Figure out how many devices we should display.
 	 */
 	if (nflag == 0) {
@@ -302,12 +296,14 @@ main(int argc, char **argv)
 	if ((num_devices = devstat_getnumdevs(kd)) < 0)
 		err(1, "can't get number of devices");
 
-	if ((cur.dinfo = (struct devinfo *)malloc(sizeof(struct devinfo))) ==
-	     NULL)
-		err(1, "devinfo malloc failed");
-	if ((last.dinfo = (struct devinfo *)malloc(sizeof(struct devinfo))) ==
-	     NULL)
-		err(1, "devinfo malloc failed");
+	cur.dinfo = (struct devinfo *)malloc(sizeof(struct devinfo));
+	if (cur.dinfo == NULL)
+		err(1, "malloc failed");
+
+	last.dinfo = (struct devinfo *)malloc(sizeof(struct devinfo));
+	if (last.dinfo == NULL)
+		err(1, "malloc failed");
+
 	bzero(cur.dinfo, sizeof(struct devinfo));
 	bzero(last.dinfo, sizeof(struct devinfo));
 
@@ -326,8 +322,10 @@ main(int argc, char **argv)
 	 * If the user specified any devices on the command line, see if
 	 * they are in the list of devices we have now.
 	 */
-	if ((specified_devices = (char **)malloc(sizeof(char *))) == NULL)
-		err(1, "specified_devices malloc failed");
+	specified_devices = (char **)malloc(sizeof(char *));
+	if (specified_devices == NULL)
+		err(1, "malloc failed");
+
 	for (num_devices_specified = 0; *argv; ++argv) {
 		if (isdigit(**argv))
 			break;
@@ -335,6 +333,9 @@ main(int argc, char **argv)
 		specified_devices = (char **)realloc(specified_devices,
 						     sizeof(char *) *
 						     num_devices_specified);
+		if (specified_devices == NULL)
+			err(1, "realloc failed");
+
 		specified_devices[num_devices_specified - 1] = *argv;
 
 	}
@@ -399,6 +400,18 @@ main(int argc, char **argv)
 	if ((wflag > 0) && (cflag == 0))
 		count = -1;
 
+	bzero(&cur.cp_time, sizeof(cur.cp_time));
+	cur.tk_nout = 0;
+	cur.tk_nin = 0;
+
+	/*
+	 * Set the busy time to the system boot time, so the stats are
+	 * calculated since system boot.
+	 */
+	if (readvar(kd, "kern.boottime", X_BOOTTIME, &cur.busy_time,
+		    sizeof(cur.busy_time)) != 0)
+		exit(1);
+
 	/*
 	 * If the user stops the program (control-Z) and then resumes it,
 	 * print out the header again.
@@ -408,27 +421,30 @@ main(int argc, char **argv)
 	for (headercount = 1;;) {
 		struct devinfo *tmp_dinfo;
 		long tmp;
-		double etime;
+		long double etime;
+
+		if (Tflag > 0) {
+			if ((readvar(kd, "kern.tty_nin", X_TK_NIN, &cur.tk_nin,
+			     sizeof(cur.tk_nin)) != 0)
+			 || (readvar(kd, "kern.tty_nout", X_TK_NOUT,
+			     &cur.tk_nout, sizeof(cur.tk_nout))!= 0)) {
+				Tflag = 0;
+				warnx("disabling TTY statistics");
+			}
+		 }
+
+		if (Cflag > 0) {
+			if (readvar(kd, "kern.cp_time", X_CP_TIME,
+			    &cur.cp_time, sizeof(cur.cp_time)) != 0) {
+				Cflag = 0;
+				warnx("disabling CPU time statistics");
+			}
+		}
 
 		if (!--headercount) {
 			phdr(0);
 			headercount = 20;
 		}
-		if (use_kvm) {
-			(void)kvm_read(kd, namelist[X_TK_NIN].n_value,
-			    &cur.tk_nin, sizeof(cur.tk_nin));
-			(void)kvm_read(kd, namelist[X_TK_NOUT].n_value,
-			    &cur.tk_nout, sizeof(cur.tk_nout));
-			(void)kvm_read(kd, namelist[X_CP_TIME].n_value,
-			    cur.cp_time, sizeof(cur.cp_time));
-		} else {
-			getsysctl("kern.tty_nin", &cur.tk_nin,
-			    sizeof(cur.tk_nin));
-			getsysctl("kern.tty_nout", &cur.tk_nout,
-			    sizeof(cur.tk_nout));
-			getsysctl("kern.cp_time", &cur.cp_time,
-			    sizeof(cur.cp_time));
-		}
 
 		tmp_dinfo = last.dinfo;
 		last.dinfo = cur.dinfo;
@@ -511,30 +527,35 @@ main(int argc, char **argv)
 			}
 		}
 
-		tmp = cur.tk_nin;
-		cur.tk_nin -= last.tk_nin;
-		last.tk_nin = tmp;
-		tmp = cur.tk_nout;
-		cur.tk_nout -= last.tk_nout;
-		last.tk_nout = tmp;
+		if (Tflag > 0) {
+			tmp = cur.tk_nin;
+			cur.tk_nin -= last.tk_nin;
+			last.tk_nin = tmp;
+			tmp = cur.tk_nout;
+			cur.tk_nout -= last.tk_nout;
+			last.tk_nout = tmp;
+		}
 
-		etime = 0.0;
+		etime = devstat_compute_etime(cur.busy_time, last.busy_time);
 
-#define X(fld)	tmp = cur.fld[i]; cur.fld[i] -= last.fld[i]; last.fld[i] = tmp
+		if (etime == 0.0)
+			etime = 1.0;
 
 		for (i = 0; i < CPUSTATES; i++) {
-			X(cp_time);
-			etime += cur.cp_time[i];
+			tmp = cur.cp_time[i];
+			cur.cp_time[i] -= last.cp_time[i];
+			last.cp_time[i] = tmp;
 		}
-		etime /= (float)hz;
-		if (etime == 0.0)
-			etime = 1.0;
-		if ((dflag == 0) || (Tflag > 0))
-			printf("%4.0f%5.0f", cur.tk_nin / etime, 
+
+		if (Tflag > 0)
+			printf("%4.0Lf%5.0Lf", cur.tk_nin / etime, 
 				cur.tk_nout/etime);
+
 		devstats(hflag, etime, havelast);
-		if ((dflag == 0) || (Cflag > 0))
+
+		if (Cflag > 0)
 			cpustats();
+
 		printf("\n");
 		fflush(stdout);
 
@@ -554,7 +575,7 @@ phdr(int signo)
 	register int i;
 	int printed;
 
-	if ((dflag == 0) || (Tflag > 0))
+	if (Tflag > 0)
 		(void)printf("      tty");
 	for (i = 0, printed=0;(i < num_devices) && (printed < maxshowdevs);i++){
 		int di;
@@ -572,12 +593,12 @@ phdr(int signo)
 			printed++;
 		}
 	}
-	if ((dflag == 0) || (Cflag > 0))
+	if (Cflag > 0)
 		(void)printf("            cpu\n");
 	else
 		(void)printf("\n");
 
-	if ((dflag == 0) || (Tflag > 0))
+	if (Tflag > 0)
 		(void)printf(" tin tout");
 
 	for (i=0, printed = 0;(i < num_devices) && (printed < maxshowdevs);i++){
@@ -597,7 +618,7 @@ phdr(int signo)
 			printed++;
 		}
 	}
-	if ((dflag == 0) || (Cflag > 0))
+	if (Cflag > 0)
 		(void)printf(" us ni sy in id\n");
 	else
 		printf("\n");
@@ -605,26 +626,15 @@ phdr(int signo)
 }
 
 static void
-devstats(int perf_select, double etime, int havelast)
+devstats(int perf_select, long double etime, int havelast)
 {
 	register int dn;
 	long double transfers_per_second;
 	long double kb_per_transfer, mb_per_second;
 	u_int64_t total_bytes, total_transfers, total_blocks;
-	long double busy_seconds;
 	long double total_mb;
 	long double blocks_per_second, ms_per_transaction;
 	
-	/*
-	 * Calculate elapsed time up front, since it's the same for all
-	 * devices.
-	 */
-	if (havelast)
-		busy_seconds = devstat_compute_etime(cur.busy_time,
-						     last.busy_time);
-	else
-		busy_seconds = etime;
-
 	for (dn = 0; dn < num_devices; dn++) {
 		int di;
 
@@ -635,7 +645,7 @@ devstats(int perf_select, double etime, int havelast)
 		di = dev_select[dn].position;
 
 		if (devstat_compute_statistics(&cur.dinfo->devices[di],
-		    havelast ? &last.dinfo->devices[di] : NULL, busy_seconds,
+		    havelast ? &last.dinfo->devices[di] : NULL, etime,
 		    DSM_TOTAL_BYTES, &total_bytes,
 		    DSM_TOTAL_TRANSFERS, &total_transfers,
 		    DSM_TOTAL_BLOCKS, &total_blocks,
@@ -706,17 +716,38 @@ cpustats(void)
 		time += cur.cp_time[state];
 	for (state = 0; state < CPUSTATES; ++state)
 		printf("%3.0f",
-		       100. * cur.cp_time[state] / (time ? time : 1));
+		       rint(100. * cur.cp_time[state] / (time ? time : 1)));
 }
 
-static void
-getsysctl(const char *name, void *ptr, size_t len)
+static int
+readvar(kvm_t *kd, const char *name, int nlid, void *ptr, size_t len)
 {
-	size_t nlen = len;
+	if (kd != NULL) {
+		ssize_t nbytes;
+
+		nbytes = kvm_read(kd, nlid, ptr, len);
+
+		if (nbytes == 0) {
+			warnx("kvm_read(%s): %s", name, kvm_geterr(kd));
+			return (1);
+		}
+		if (nbytes != len) {
+			warnx("kvm_read(%s): expected %lu bytes, got %ld bytes",
+			      name, (unsigned long)len, (long)nbytes);
+			return (1);
+		}
+	} else {
+		size_t nlen = len;
 	
-	if (sysctlbyname(name, ptr, &nlen, NULL, 0) == -1)
-		err(1, "sysctl(%s...) failed", name);
-	if (nlen != len)
-		errx(1, "sysctl(%s...): expected %lu, got %lu", name,
-		     (unsigned long)len, (unsigned long)nlen);
+		if (sysctlbyname(name, ptr, &nlen, NULL, 0) == -1) {
+			warn("sysctl(%s...) failed", name);
+			return (1);
+		}
+		if (nlen != len) {
+			warnx("sysctl(%s...): expected %lu, got %lu", name,
+			      (unsigned long)len, (unsigned long)nlen);
+			return (1);
+		}
+	}
+	return (0);
 }