svn commit: r326139 - head/usr.bin/vmstat
Bruce Evans
brde at optusnet.com.au
Fri Nov 24 07:45:37 UTC 2017
On Thu, 23 Nov 2017, Konstantin Belousov wrote:
> Log:
> vmstat: use 64-bit counters from struct vmtotal.
>
> Consistently print counters using unsigned intmax type.
Not very consistently. After fixing 0.01% of libxo (just add __printflike()
to xo_emit(), the following errors are detected:
X vmstat.c:817:23: error: format specifies type 'long' but the argument has type 'int' [-Werror,-Wformat]
X total.t_rq - 1, total.t_dw + total.t_pw, total.t_sw);
X ^~~~~~~~~~~~~~~~~~~~~~~
X vmstat.c:817:48: error: format specifies type 'long' but the argument has type 'int16_t' (aka 'short') [-Werror,-Wformat]
X total.t_rq - 1, total.t_dw + total.t_pw, total.t_sw);
X ^~~~~~~~~~
Broken in r291090 (the first libxo commit to this file). All the arg types
here are still int since all the fields in 'total' still have type int16_t.
X vmstat.c:865:7: error: format specifies type 'unsigned int' but the argument has type 'unsigned long' [-Werror,-Wformat]
X (unsigned long)rate(sum.v_swtch - osum.v_swtch));
X ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Broken in r291090. Everything in the same printf is broken, but the other
bugs are newer and older:
- in 4.4BSD-Lite2, the the format was %lu for all 3 values to be printed,
but the type was MD. It was apparently long. The struct fields were all
u_int IIRC, but the rate() expression involves time_t. time_t was
_BSD_TIME_T_, which was apparently long on all arches.
- FreeBSD fixed time_t to be int on at least i386. The type error was
still non-fatal on 32-bit arches with 32-bit time_t.
- I fixed this in r37453. The fix wasn't so good. It retained the %lu
format and added casts to u_long to match the format. But the casts are
unportable and became wrong when v_swtch etc. were expanded.
- r123407 broke the style by changing all u_* to unsigned *.
- the casts became wrong, but fairly harmlessly so, on 32-bit arches when
v_swtch, etc. was changed to counter_u64_t in r317061 when v_swtch was
expanded to counter_u64_t. The problem is limit because the values are
rays of differences. Even %u format is probably enough for this.
X vmstat.c:1035:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_swtch);
X ^~~~~~~~~~~
X vmstat.c:1037:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_intr);
X ^~~~~~~~~~
X vmstat.c:1039:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_soft);
X ^~~~~~~~~~
X vmstat.c:1040:38: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X xo_emit("{:traps/%9u} {N:traps}\n", sum.v_trap);
X ~~~ ^~~~~~~~~~
X %9lu
X vmstat.c:1042:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_syscall);
X ^~~~~~~~~~~~~
X vmstat.c:1044:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_kthreads);
X ^~~~~~~~~~~~~~
X vmstat.c:1045:46: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X xo_emit("{:forks/%9u} {N: fork() calls}\n", sum.v_forks);
X ~~~ ^~~~~~~~~~~
X %9lu
X vmstat.c:1047:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_vforks);
X ^~~~~~~~~~~~
X vmstat.c:1049:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_rforks);
X ^~~~~~~~~~~~
X vmstat.c:1051:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_swapin);
X ^~~~~~~~~~~~
X vmstat.c:1053:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_swappgsin);
X ^~~~~~~~~~~~~~~
X vmstat.c:1055:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_swapout);
X ^~~~~~~~~~~~~
X vmstat.c:1057:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_swappgsout);
X ^~~~~~~~~~~~~~~~
X vmstat.c:1059:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_vnodein);
X ^~~~~~~~~~~~~
X vmstat.c:1061:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_vnodepgsin);
X ^~~~~~~~~~~~~~~~
X vmstat.c:1063:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_vnodeout);
X ^~~~~~~~~~~~~~
X vmstat.c:1065:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_vnodepgsout);
X ^~~~~~~~~~~~~~~~~
X vmstat.c:1067:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_pdwakeups);
X ^~~~~~~~~~~~~~~
X vmstat.c:1069:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_pdpages);
X ^~~~~~~~~~~~~
X vmstat.c:1071:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_pdshortfalls);
X ^~~~~~~~~~~~~~~~~~
X vmstat.c:1073:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_reactivated);
X ^~~~~~~~~~~~~~~~~
X vmstat.c:1075:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_cow_faults);
X ^~~~~~~~~~~~~~~~
X vmstat.c:1077:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_cow_optim);
X ^~~~~~~~~~~~~~~
X vmstat.c:1079:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_zfod);
X ^~~~~~~~~~
X vmstat.c:1081:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_ozfod);
X ^~~~~~~~~~~
X vmstat.c:1083:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_intrans);
X ^~~~~~~~~~~~~
X vmstat.c:1085:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_vm_faults);
X ^~~~~~~~~~~~~~~
X vmstat.c:1087:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_io_faults);
X ^~~~~~~~~~~~~~~
X vmstat.c:1089:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_kthreadpages);
X ^~~~~~~~~~~~~~~~~~
X vmstat.c:1091:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_forkpages);
X ^~~~~~~~~~~~~~~
X vmstat.c:1093:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_vforkpages);
X ^~~~~~~~~~~~~~~~
X vmstat.c:1095:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_rforkpages);
X ^~~~~~~~~~~~~~~~
X vmstat.c:1097:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_tfree);
X ^~~~~~~~~~~
X vmstat.c:1099:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_dfree);
X ^~~~~~~~~~~
X vmstat.c:1101:3: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_pfree);
X ^~~~~~~~~~~
Breakage of printing of the raw v_foo fields has a shorter history.
I think they always had type u_int and were honestly printed with %u
before counter_u64_t. But libxo prepared to break them by not using
__printflike(), and counter_u64_t broke them by not updating the format.
clang reports that uint64_t is aka u_long, but this is only for 64-bit
arches.
X vmstat.c:1127:39: error: flag ' ' results in undefined behavior with 'p' conversion specifier [-Werror,-Wformat]
X "({:positive-cache-hits/%ld}% pos + "
X ~^~
X vmstat.c:1131:6: error: format specifies type 'void *' but the argument has type 'long' [-Werror,-Wformat]
X PCT(lnchstats.ncs_neghits, nchtotal),
X ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Maybe xo_emit() is not actually printf() compatible. For printf(), the
percent sign must be printed using %%, but xo_emit() seems to print
unquoted % correct. This one is "% ".
The format checker seems to have a bug too. It treats "% p" as %p, which
gives a cascade of errors. Broken code like this probably just wants
literal %.
X vmstat.c:1024:23: note: expanded from macro 'PCT'
X #define PCT(top, bot) pct((long)(top), (long)(bot))
X ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
X vmstat.c:1128:39: error: invalid conversion specifier '{' [-Werror,-Wformat-invalid-specifier]
X "{:negative-cache-hits/%ld}% {N:neg}) "
X ~~^
X vmstat.c:1129:40: error: more '%' conversions than data arguments [-Werror,-Wformat]
X "system {:cache-hit-percent/%ld}% per-directory\n",
X ~~^
X vmstat.c:1133:51: error: invalid conversion specifier ',' [-Werror,-Wformat-invalid-specifier]
X xo_emit("{P:/%9s} {L:deletions} {:deletions/%ld}%, "
X ~^
This one is "%,". This ends the error cascade for the xo_emit() on line
1126 (it has 3 quoting errors for %).
X vmstat.c:1134:43: error: invalid conversion specifier ',' [-Werror,-Wformat-invalid-specifier]
X "{L:falsehits} {:false-hits/%ld}%, "
X ~^
X vmstat.c:1135:39: error: invalid conversion specifier '\x0a' [-Werror,-Wformat-invalid-specifier]
X "{L:toolong} {:too-long/%ld}%\n", "",
X ~^
Similarly. 3 more quoting errors for the xo_emit() on line 1133.
X vmstat.c:1149:6: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_forks, sum.v_forkpages,
X ^~~~~~~~~~~
X vmstat.c:1149:19: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_forks, sum.v_forkpages,
X ^~~~~~~~~~~~~~~
X vmstat.c:1154:6: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_vforks, sum.v_vforkpages,
X ^~~~~~~~~~~~
X vmstat.c:1154:20: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_vforks, sum.v_vforkpages,
X ^~~~~~~~~~~~~~~~
X vmstat.c:1159:6: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_rforks, sum.v_rforkpages,
X ^~~~~~~~~~~~
X vmstat.c:1159:20: error: format specifies type 'unsigned int' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X sum.v_rforks, sum.v_rforkpages,
X ^~~~~~~~~~~~~~~~
Back to misprinting counter_u64_t's.
X vmstat.c:1473:30: error: format specifies type 'char *' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X memstat_get_name(mtp), memstat_get_count(mtp),
X ^~~~~~~~~~~~~~~~~~~~~~
X vmstat.c:1474:47: error: format specifies type 'unsigned long' but the argument has type 'char *' [-Werror,-Wformat]
A very large error. The count is misprinted using %s format...
X (memstat_get_bytes(mtp) + 1023) / 1024, "-",
X ^~~
X vmstat.c:1475:7: error: format specifies type 'char *' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
X memstat_get_numallocs(mtp));
X ^~~~~~~~~~~~~~~~~~~~~~~~~~
X vmstat.c:1472:20: error: more '%' conversions than data arguments [-Werror,-Wformat]
X "{:requests/%8" PRIu64 "} ",
X ~~~~^
X /usr/include/x86/_inttypes.h:100:25: note: expanded from macro 'PRIu64'
X #define PRIu64 __PRI64"u" /* uint64_t */
X ^
There are 6 format specifiers but only 5 args. Apparently the second
specifier (%s) is missing a string arg or shouldn't be there. After
removing this, no errors are detected for this xo_emit().
The __PRI64 mistake is used in a few places including this. It helps make
the format almost unreadable.
X vmstat.c:1673:20: error: more '%' conversions than data arguments [-Werror,-Wformat]
X xo_emit("{T:RES/%5s} {T:ACT/%5s} {T:INACT/%5s} {T:REF/%3s} {T:SHD/%3s} "
X ~~^
Here it seems clear that xo_emit() is not actually printf() compatible. It
passes no args to this call. This prints nothing for vmstat -o. I don't
want libxo and don't know how to use it. The documented syntax
"vmstat --libxo followed by normal args" is a syntax error.
X 56 errors generated.
vmstat.c now has no printf()s except for 2 snprintf()s, so has essentially
no printf() format checking. A few things are printed in dehumanized
format by prthuman() = dehumanize_number() + xo_attr() with %ju format.
These get upcast enough to work, and the format of the xo_attr() for
this is easily checked manually. xo_attr() is also not declared as
__printflike().
Bruce
More information about the svn-src-all
mailing list