vmstat WARNS6 cleanup
Peter Schuller
peter.schuller at infidyne.com
Sat Jan 8 10:22:26 PST 2005
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 at xxxxxxxxxxxx>'
Key retrieval: Send an E-Mail to getpgpkey at xxxxxxxxx
E-Mail: peter.schuller at 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
More information about the Submit
mailing list