Differential D13228
vmstat: fix style(9) violations and bump WARNS Authored by kaktus on Nov 24 2017, 4:08 PM. Tags None Referenced Files
Subscribers
Details
This is the first version of the larger cleanup in vmstat code. I fixed many style formatting and sorting issues, removed unused variables and function, initialised struct nlist correctly. This allowed to bump WARNS to 6. I'd take a look at the PRI formatting macros and bde@ comments on xo_emit in later versions. Submited by: Pawel Biernacki <pawel.biernacki@gmail.com> No functional changes should occur.
Diff Detail
Event Timeline
Comment Actions Address @kib comments. Also more and more style(9) cleanup like moving variables from body to top of the functions, removing unneeded empty lines, PRIu64 changed to %ju with uintmax_t cast, sorting, etc... Comment Actions commit 1bdb99ca310aa8b145bdc139aa59ba5373e0c306 Author: Konstantin Belousov <kib@freebsd.org> Date: Fri Dec 1 20:20:22 2017 +0200 More style fixes. diff --git a/usr.bin/vmstat/vmstat.c b/usr.bin/vmstat/vmstat.c index f69b1d6d83b..288dc5c096f 100644 --- a/usr.bin/vmstat/vmstat.c +++ b/usr.bin/vmstat/vmstat.c @@ -171,7 +171,7 @@ static struct __vmmeter { } sum, osum; #define VMSTAT_DEFAULT_LINES 20 /* Default number of `winlines'. */ -static volatile sig_atomic_t wresized; /* Tty resized, when non-zero. */ +static volatile sig_atomic_t wresized; /* Tty resized when non-zero. */ static int winlines = VMSTAT_DEFAULT_LINES; /* Current number of tty rows. */ static int aflag; @@ -216,10 +216,10 @@ static char **getdrivedata(char **); int main(int argc, char *argv[]) { + char *bp, *buf, *memf, *nlistf; float f; int bufsize, c, len, reps, todo; unsigned int interval; - char *bp, *buf, *memf, *nlistf; char errbuf[_POSIX2_LINE_MAX]; memf = nlistf = NULL; @@ -229,7 +229,7 @@ main(int argc, char *argv[]) argc = xo_parse_args(argc, argv); if (argc < 0) - return argc; + return (argc); while ((c = getopt(argc, argv, "ac:fhHiM:mN:n:oPp:stw:z")) != -1) { switch (c) { @@ -285,7 +285,8 @@ main(int argc, char *argv[]) #ifdef notyet todo |= TIMESTAT; #else - xo_errx(EX_USAGE, "sorry, -t is not (re)implemented yet"); + xo_errx(EX_USAGE, + "sorry, -t is not (re)implemented yet"); #endif break; case 'w': @@ -318,8 +319,9 @@ retry_nlist: if (kd != NULL && (c = kvm_nlist(kd, namelist)) != 0) { if (c > 0) { bufsize = 0, len = 0; + /* - * 'cnt' was renamed to 'vm_cnt'. If 'vm_cnt' is not + * 'cnt' was renamed to 'vm_cnt'. If 'vm_cnt' is not * found try looking up older 'cnt' symbol. * */ if (namelist[X_SUM].n_type == 0 && @@ -327,6 +329,7 @@ retry_nlist: namelist[X_SUM].n_name = "_cnt"; goto retry_nlist; } + for (c = 0; c < (int)(nitems(namelist)); c++) if (namelist[c].n_type == 0) bufsize += strlen(namelist[c].n_name) @@ -428,19 +431,19 @@ getdrivedata(char **argv) num_devices = cur.dinfo->numdevs; generation = cur.dinfo->generation; - specified_devices = (char **)malloc(sizeof(char *)); + specified_devices = malloc(sizeof(char *)); for (num_devices_specified = 0; *argv; ++argv) { if (isdigit(**argv)) break; num_devices_specified++; - specified_devices = (char **)realloc(specified_devices, + specified_devices = realloc(specified_devices, sizeof(char *) * num_devices_specified); specified_devices[num_devices_specified - 1] = *argv; } dev_select = NULL; if (nflag == 0 && maxshowdevs < num_devices_specified) - maxshowdevs = num_devices_specified; + maxshowdevs = num_devices_specified; /* * People are generally only interested in disk statistics when @@ -455,7 +458,6 @@ getdrivedata(char **argv) if ((num_devices_specified == 0) && (num_matches == 0)) { if (devstat_buildmatch(da, &matches, &num_matches) != 0) xo_errx(1, "%s", devstat_errbuf); - select_mode = DS_SELECT_ADD; } else select_mode = DS_SELECT_ONLY; @@ -609,8 +611,7 @@ getcpuinfo(u_long *maskp, int *maxidp) long *times; u_long mask; size_t size; - int empty, i, j; - int maxcpu, maxid, ncpus; + int empty, i, j, maxcpu, maxid, ncpus; if (kd != NULL) xo_errx(1, "not implemented"); @@ -671,8 +672,7 @@ dovmstat(unsigned int interval, int reps) u_long cpumask; size_t size; time_t uptime, halfuptime; - int ncpus, maxid; - int rate_adj, retval; + int ncpus, maxid, rate_adj, retval; uptime = getuptime() / 1000000000LL; halfuptime = uptime / 2; @@ -755,7 +755,7 @@ dovmstat(unsigned int interval, int reps) case -1: xo_errx(1, "%s", devstat_errbuf); break; - case 1: { + case 1: num_devices = cur.dinfo->numdevs; generation = cur.dinfo->generation; @@ -776,7 +776,7 @@ dovmstat(unsigned int interval, int reps) default: break; } - } + break; default: break; } @@ -815,7 +815,8 @@ dovmstat(unsigned int interval, int reps) xo_open_container("paging-rates"); xo_emit("{:page-reactivated/%3lu} ", - (unsigned long)rate(sum.v_reactivated - osum.v_reactivated)); + (unsigned long)rate(sum.v_reactivated - + osum.v_reactivated)); xo_emit("{:paged-in/%3lu} ", (unsigned long)rate(sum.v_swapin + sum.v_vnodein - (osum.v_swapin + osum.v_vnodein))); @@ -865,13 +866,12 @@ printhdr(int maxid, u_long cpumask) int i, num_shown; num_shown = MIN(num_selected, maxshowdevs); - if (hflag) { + if (hflag) xo_emit("{T:procs} {T:memory} {T:/page%*s}", 19, ""); - } else { + else xo_emit("{T:procs} {T:memory} {T:/page%*s}", 19, ""); - } if (num_shown > 1) - xo_emit(" {T:/disks %*s}", num_shown * 4 - 7, ""); + xo_emit(" {T:/disks %*s}", num_shown * 4 - 7, ""); else if (num_shown == 1) xo_emit(" {T:disks}"); xo_emit(" {T:faults} "); @@ -885,10 +885,10 @@ printhdr(int maxid, u_long cpumask) xo_emit(" {T:cpu}\n"); if (hflag) { xo_emit("{T:r} {T:b} {T:w} {T:avm} {T:fre} {T:flt} {T:re}" - " {T:pi} {T:po} {T:fr} {T:sr} "); + " {T:pi} {T:po} {T:fr} {T:sr} "); } else { xo_emit("{T:r} {T:b} {T:w} {T:avm} {T:fre} {T:flt} " - "{T:re} {T:pi} {T:po} {T:fr} {T:sr} "); + "{T:re} {T:pi} {T:po} {T:fr} {T:sr} "); } for (i = 0; i < num_devices; i++) if ((dev_select[i].selected) && @@ -1141,8 +1141,7 @@ doforkst(void) static void devstats(void) { - long double transfers_per_second; - long double busy_seconds; + long double busy_seconds, transfers_per_second; long tmp; int di, dn, state; @@ -1156,8 +1155,8 @@ devstats(void) xo_open_list("device"); for (dn = 0; dn < num_devices; dn++) { - if ((dev_select[dn].selected == 0) || - (dev_select[dn].selected > maxshowdevs)) + if (dev_select[dn].selected == 0 || + dev_select[dn].selected > maxshowdevs) continue; di = dev_select[dn].position; @@ -1289,14 +1288,13 @@ read_intrcnts(unsigned long **intrcnts) static void print_intrcnts(unsigned long *intrcnts, unsigned long *old_intrcnts, - char *intrnames, unsigned int nintr, - size_t istrnamlen, long long period_ms) + char *intrnames, unsigned int nintr, size_t istrnamlen, long long period_ms) { - uint64_t inttotal, old_inttotal, total_count, total_rate; unsigned long *intrcnt, *old_intrcnt; + char *intrname; + uint64_t inttotal, old_inttotal, total_count, total_rate; unsigned long count, rate; unsigned int i; - char *intrname; inttotal = 0; old_inttotal = 0; @@ -1327,11 +1325,11 @@ print_intrcnts(unsigned long *intrcnts, unsigned long *old_intrcnts, static void dointr(unsigned int interval, int reps) { - long long old_uptime, uptime, period_ms; - size_t clen, inamlen, istrnamlen; unsigned long *intrcnts, *old_intrcnts; + char *intrname, *intrnames; + long long period_ms, old_uptime, uptime; + size_t clen, inamlen, istrnamlen; unsigned int nintr; - char *intrnames, *intrname; old_intrcnts = NULL; uptime = getuptime(); is my current diff over your patch. If you are fine with this, I will commit the cumulative change. Comment Actions I'm fine with it. I've one question about what is the correct way of sorting local variables? style(9) use two different versions (or I'm reading it wrong): int
main(int argc, char *argv[])
{
char *ep;
long num;
int ch;vs. struct foo one, *two; double three; int *four, five; char *six, seven, eight, nine, ten, eleven, twelve; Comment Actions Apparently tinderbox is busted. Additionally at least the following chunk is needed. commit 07e3214722f4cb9928a8ef908e7a4be4a3cb6bcd Author: Konstantin Belousov <kib@freebsd.org> Date: Fri Dec 1 20:34:44 2017 +0200 Appease gcc. diff --git a/usr.bin/vmstat/vmstat.c b/usr.bin/vmstat/vmstat.c index 288dc5c096f..74528ab3d40 100644 --- a/usr.bin/vmstat/vmstat.c +++ b/usr.bin/vmstat/vmstat.c @@ -1179,14 +1179,14 @@ devstats(void) } static void -percent(const char *name, double pct, int *over) +percent(const char *name, double pctv, int *over) { int l; char buf[10]; char fmt[128]; snprintf(fmt, sizeof(fmt), " {:%s/%%*s}", name); - l = snprintf(buf, sizeof(buf), "%.0f", pct); + l = snprintf(buf, sizeof(buf), "%.0f", pctv); if (l == 1 && *over) { xo_emit(fmt, 1, buf); (*over)--; Comment Actions It is said 'order by alignment required', for whatever arch you consider. This would explain floated order for double. Comment Actions One more issue, recording it there to keep all diffs against original submission. commit 2b9fe1c7cf023c6700f69bd25268c8ce04267872 Author: Konstantin Belousov <kib@freebsd.org> Date: Fri Dec 1 23:37:17 2017 +0200 Initialize variable to silence gcc. diff --git a/usr.bin/vmstat/vmstat.c b/usr.bin/vmstat/vmstat.c index 74528ab3d40..86f248e054d 100644 --- a/usr.bin/vmstat/vmstat.c +++ b/usr.bin/vmstat/vmstat.c @@ -679,6 +679,7 @@ dovmstat(unsigned int interval, int reps) rate_adj = 1; ncpus = 1; maxid = 0; + cpumask = 0; /* * If the user stops the program (control-Z) and then resumes it, | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||