DragonFly BSD
DragonFly submit List (threaded) for 2005-01
[Date Prev][Date Next]  [Thread Prev][Thread Next]  [Date Index][Thread Index]

vmstat WARNS6 cleanup


From: Peter Schuller <peter.schuller@xxxxxxxxxxxx>
Date: Sat, 8 Jan 2005 19:31:45 +0100

Note #1: This patch depends on previous dmesg patch due to the use
         of NLIST_NAME macro,

Note #2: Apart from vmstat itself, this patch also modifies
         lib/devstat/devstat.(h|c|3). buildmatch() was actually
         destructive w.r.t. the match string given due to the
         use of strsep internally.

         Since use of strsep is about as much of an irrelevant
         implementation detail as anything, I fixed it rather
         than just documenting the previous behavior. The "fix"
         in this case is to malloc an internal buffer.

         Unfortunately this uglifies the function by complicating
         subsequent exit paths; but even if one chose to use
         alloca() (e.g. for performance) one would have to fallback
         to malloc for large strings anyway.

-- 
/ Peter Schuller, InfiDyne Technologies HB

PGP userID: 0xE9758B7D or 'Peter Schuller <peter.schuller@xxxxxxxxxxxx>'
Key retrieval: Send an E-Mail to getpgpkey@xxxxxxxxx
E-Mail: peter.schuller@xxxxxxxxxxxx Web: http://www.scode.org

--- usr.bin/vmstat/Makefile.orig	2005-01-08 08:51:43.000000000 +0000
+++ usr.bin/vmstat/Makefile	2005-01-08 08:51:24.000000000 +0000
@@ -9,5 +9,6 @@
 BINMODE=2555
 DPADD=	${LIBKINFO} ${LIBKVM} ${LIBDEVSTAT}
 LDADD=	-lkinfo -lkvm -ldevstat
+WARNS?= 6
 
 .include <bsd.prog.mk>
--- usr.bin/vmstat/vmstat.c.orig	2005-01-08 08:51:52.000000000 +0000
+++ usr.bin/vmstat/vmstat.c	2005-01-08 10:05:22.000000000 +0000
@@ -70,37 +70,37 @@
 
 static struct nlist namelist[] = {
 #define	X_BOOTTIME	0
-	{ "_boottime" },
+	NLIST_NAME("_boottime"),  
 #define X_NCHSTATS	1
-	{ "_nchstats" },
+	NLIST_NAME("_nchstats"),
 #define	X_INTRNAMES	2
-	{ "_intrnames" },
+	NLIST_NAME("_intrnames"),
 #define	X_EINTRNAMES	3
-	{ "_eintrnames" },
+	NLIST_NAME("_eintrnames"),
 #define	X_INTRCNT	4
-	{ "_intrcnt" },
+	NLIST_NAME("_intrcnt"),
 #define	X_EINTRCNT	5
-	{ "_eintrcnt" },
+	NLIST_NAME("_eintrcnt"),
 #define	X_KMEMSTATISTICS	6
-	{ "_kmemstatistics" },
+	NLIST_NAME("_kmemstatistics"),
 #define	X_ZLIST		7
-	{ "_zlist" },
+	NLIST_NAME("_zlist"),
 #ifdef notyet
 #define	X_DEFICIT	8
-	{ "_deficit" },
+	NLIST_NAME("_deficit"),
 #define	X_FORKSTAT	9
-	{ "_forkstat" },
+	NLIST_NAME("_forkstat"),
 #define X_REC		10
-	{ "_rectime" },
+	NLIST_NAME("_rectime"),
 #define X_PGIN		11
-	{ "_pgintime" },
+	NLIST_NAME("_pgintime"),
 #define	X_XSTATS	12
-	{ "_xstats" },
+	NLIST_NAME("_xstats"),
 #define X_END		13
 #else
 #define X_END		8
 #endif
-	{ "" },
+	NLIST_NULL,
 };
 
 struct statinfo cur, last;
@@ -133,13 +133,25 @@
 #define	VMSTAT		0x20
 #define ZMEMSTAT	0x40
 
-void	cpustats(), dointr(), domem(), dosum(), dozmem();
-void	dovmstat(), kread(), usage();
+static void cpustats(void);
+static void dointr(void);
+static void domem(void);
+static void dosum(void);
+static void dozmem(void);
+static void dovmstat(u_int interval, int reps);
+static void kread(int nlx, void *addr, size_t size);
+static void usage(void);
+static char **getdrivedata(char **argv);
+static long getuptime(void);
+static void needhdr(int signo);
+static long pct(long top, long bot);
+
 #ifdef notyet
-void	dotimes(), doforkst();
+static void dotimes(void); /* Not implemented */
+static void doforkst(void);
 #endif
-void printhdr(void);
-static void devstats();
+static void printhdr(void);
+static void devstats(void);
 
 int
 main(int argc, char **argv)
@@ -230,7 +242,8 @@
 		if (c > 0) {
 			warnx("undefined symbols:");
 			for (c = 0;
-			    c < sizeof(namelist)/sizeof(namelist[0]); c++)
+			     c < (int)(sizeof(namelist)/sizeof(namelist[0]));
+			     c++)
 				if (namelist[c].n_type == 0)
 					fprintf(stderr, " %s",
 					    namelist[c].n_name);
@@ -241,7 +254,6 @@
 	}
 
 	if (todo & VMSTAT) {
-		char **getdrivedata();
 		struct winsize winsize;
 
 		/*
@@ -297,7 +309,7 @@
 	exit(0);
 }
 
-char **
+static char **
 getdrivedata(char **argv)
 {
 	if ((num_devices = getnumdevs()) < 0)
@@ -362,7 +374,7 @@
 	return(argv);
 }
 
-long
+static long
 getuptime(void)
 {
 	static time_t now, boottime;
@@ -379,13 +391,12 @@
 
 int	hdrcnt;
 
-void
+static void
 dovmstat(u_int interval, int reps)
 {
 	struct vmtotal total;
 	time_t uptime, halfuptime;
 	struct devinfo *tmp_dinfo;
-	void needhdr();
 	int vmm_size = sizeof(vmm);
 	int vms_size = sizeof(vms);
 	int vmt_size = sizeof(total);
@@ -500,7 +511,7 @@
 	}
 }
 
-void
+static void
 printhdr(void)
 {
 	int i, num_shown;
@@ -526,14 +537,14 @@
 /*
  * Force a header to be prepended to the next output.
  */
-void
-needhdr(void)
+static void
+needhdr(__unused int signo)
 {
 
 	hdrcnt = 1;
 }
 
-long
+static long
 pct(long top, long bot)
 {
 	long ans;
@@ -546,7 +557,7 @@
 
 #define	PCT(top, bot) pct((long)(top), (long)(bot))
 
-void
+static void
 dosum(void)
 {
 	struct nchstats *nch_tmp, nchstats;
@@ -657,10 +668,9 @@
 static void
 devstats(void)
 {
-	int dn, state;
+	int dn;
 	long double transfers_per_second;
 	long double busy_seconds;
-	long tmp;
 
 	diff_cp_time.cp_user = cp_time.cp_user - old_cp_time.cp_user;
 	diff_cp_time.cp_nice = cp_time.cp_nice - old_cp_time.cp_nice;
@@ -691,25 +701,28 @@
 	}
 }
 
-void
+static void
 cpustats(void)
 {
 	uint64_t total;
-	double pct;
+	double totusage;
 
 	total = diff_cp_time.cp_user + diff_cp_time.cp_nice +
 	    diff_cp_time.cp_sys + diff_cp_time.cp_intr + diff_cp_time.cp_idle;
 
 	if (total)
-		pct = 100.0 / total;
+		totusage = 100.0 / total;
 	else
-		pct = 0;
-	printf("%2.0f ", (diff_cp_time.cp_user + diff_cp_time.cp_nice) * pct);
-	printf("%2.0f ", (diff_cp_time.cp_sys + diff_cp_time.cp_intr) * pct);
-	printf("%2.0f", diff_cp_time.cp_idle * pct);
+		totusage = 0;
+	printf("%2.0f ",
+	       (diff_cp_time.cp_user + diff_cp_time.cp_nice) * totusage);
+	printf("%2.0f ",
+	       (diff_cp_time.cp_sys + diff_cp_time.cp_intr) * totusage);
+	printf("%2.0f",
+	       diff_cp_time.cp_idle * totusage);
 }
 
-void
+static void
 dointr(void)
 {
 	u_long *intrcnt, uptime;
@@ -756,7 +769,7 @@
     return(ttl);
 }
 
-void
+static void
 domem(void)
 {
 	struct malloc_type *ks;
@@ -774,7 +787,7 @@
 		if (sizeof(buf) !=  kvm_read(kd, 
 	            (u_long)kmemstats[nkms].ks_shortdesc, buf, sizeof(buf)))
 			err(1, "kvm_read(%p)", 
-			    (void *)kmemstats[nkms].ks_shortdesc);
+			    kmemstats[nkms].ks_shortdesc);
 		buf[sizeof(buf) - 1] = '\0';
 		kmemstats[nkms].ks_shortdesc = strdup(buf);
 		kmsp = kmemstats[nkms].ks_next;
@@ -818,7 +831,7 @@
 	     (totuse + 1023) / 1024, (totfree + 1023) / 1024, totreq);
 }
 
-void
+static void
 dozmem(void)
 {
 	char *buf;
@@ -843,7 +856,7 @@
 /*
  * kread reads something from the kernel, given its nlist index.
  */
-void
+static void
 kread(int nlx, void *addr, size_t size)
 {
 	const char *sym;
@@ -854,7 +867,7 @@
 			++sym;
 		errx(1, "symbol %s not defined", sym);
 	}
-	if (kvm_read(kd, namelist[nlx].n_value, addr, size) != size) {
+	if (kvm_read(kd, namelist[nlx].n_value, addr, size) != (ssize_t)size) {
 		sym = namelist[nlx].n_name;
 		if (*sym == '_')
 			++sym;
@@ -862,7 +875,7 @@
 	}
 }
 
-void
+static void
 usage(void)
 {
 	fprintf(stderr, "%s%s",
--- lib/libdevstat/devstat.h.orig	2005-01-08 09:44:51.000000000 +0000
+++ lib/libdevstat/devstat.h	2005-01-08 09:45:00.000000000 +0000
@@ -98,7 +98,7 @@
 	       char **dev_selections, int num_dev_selections,
 	       devstat_select_mode select_mode, int maxshowdevs,
 	       int perf_select);
-int buildmatch(char *match_str, struct devstat_match **matches,
+int buildmatch(const char *match_str, struct devstat_match **matches,
 	       int *num_matches);
 int compute_stats(struct devstat *current, struct devstat *previous,
 		  long double etime, u_int64_t *total_bytes,
--- lib/libdevstat/devstat.c.orig	2005-01-08 09:44:46.000000000 +0000
+++ lib/libdevstat/devstat.c	2005-01-08 09:57:33.000000000 +0000
@@ -879,12 +879,17 @@
  * device matching expression from it.
  */
 int
-buildmatch(char *match_str, struct devstat_match **matches, int *num_matches)
+buildmatch(const char *match_str,
+	   struct devstat_match **matches,
+	   int *num_matches)
 {
 	char *tstr[5];
 	char **tempstr;
+	char *matchbuf = NULL; /* copy of match_str */
+	char *matchbuf_tmp;    /* allow strsep to clobber */
 	int num_args;
 	int i, j;
+	int rc = 0;
 	char *func_name = "buildmatch";
 
 	/* We can't do much without a string to parse */
@@ -895,9 +900,17 @@
 
 	/*
 	 * Break the (comma delimited) input string out into separate strings.
+	 * strsep is destructive, so copy the string first.
 	 */
+	matchbuf = malloc(strlen(match_str) + 1);
+	if(matchbuf == NULL) {
+	  sprintf(devstat_errbuf, "%s: out of memory", func_name);
+	  return(-1);
+	}
+	matchbuf_tmp = matchbuf;
+	strcpy(matchbuf, match_str);
 	for (tempstr = tstr, num_args  = 0; 
-	     (*tempstr = strsep(&match_str, ",")) != NULL && (num_args < 5); 
+	     (*tempstr = strsep(&matchbuf_tmp, ",")) != NULL && (num_args < 5); 
 	     num_args++)
 		if (**tempstr != '\0')
 			if (++tempstr >= &tstr[5])
@@ -907,7 +920,7 @@
 	if (num_args > 3) {
 		sprintf(devstat_errbuf, "%s: too many type arguments",
 			func_name);
-		return(-1);
+		goto err;
 	}
 
 	/*
@@ -975,7 +988,7 @@
 						"%s: cannot have more than "
 						"one match item in a single "
 						"category", func_name);
-					return(-1);
+					goto err;
 				}
 				/*
 				 * If we've gotten this far, we have a
@@ -999,13 +1012,17 @@
 			snprintf(devstat_errbuf, sizeof(devstat_errbuf),
 				"%s: unknown match item \"%s\"", func_name,
 				tstr[i]);
-			return(-1);
+			goto err;
 		}
 	}
 
 	(*num_matches)++;
-
-	return(0);
+	goto success;
+ err:
+	rc = -1;
+ success:
+	free(matchbuf);
+	return(rc);
 }
 
 /*
--- lib/libdevstat/devstat.3.orig	2005-01-08 10:12:47.000000000 +0000
+++ lib/libdevstat/devstat.3	2005-01-08 10:13:23.000000000 +0000
@@ -76,7 +76,7 @@
 .Fc
 .Ft int
 .Fo buildmatch
-.Fa "char *match_str"
+.Fa "const char *match_str"
 .Fa "struct devstat_match **matches"
 .Fa "int *num_matches"
 .Fc


[Date Prev][Date Next]  [Thread Prev][Thread Next]  [Date Index][Thread Index]