On 3/10/2017 8:29 PM, Bruce Evans wrote:
On Fri, 10 Mar 2017, Ngie Cooper wrote:
On Mar 10, 2017, at 03:59, Pedro Giffuni <p...@freebsd.org> wrote:
On 3/10/2017 2:45 AM, Bruce Evans wrote:
On Fri, 10 Mar 2017, Marcelo Araujo wrote:
...
Log:
Use nitems() from sys/param.h and also remove the cast.
Reviewed by: markj
MFC after: 3 weeks.
Differential Revision: https://reviews.freebsd.org/D9937
...
Modified: head/usr.bin/vmstat/vmstat.c
==============================================================================
--- head/usr.bin/vmstat/vmstat.c Fri Mar 10 04:30:31 2017
(r314988)
+++ head/usr.bin/vmstat/vmstat.c Fri Mar 10 04:49:40 2017
(r314989)
@@ -288,17 +288,13 @@ retry_nlist:
namelist[X_SUM].n_name = "_cnt";
goto retry_nlist;
}
- for (c = 0;
- c < (int)(sizeof(namelist)/sizeof(namelist[0]));
- c++)
+ for (c = 0; c < nitems(namelist); c++)
if (namelist[c].n_type == 0)
bufsize += strlen(namelist[c].n_name) + 1;
This undoes fixes to compile at WARNS=2 in r87690 and now breaks at
WARNS=3.
vmstat is still compiled at WARNS=1.
nitems suffers from the same unsigned poisoning as the sizeof()
expression
(since it reduces to the same expression. Casting to int in the
expression
to fix the warning would break exotic cases. Of course, nitems is
undocumented so no one knows when it is supposed to work).
vmstat compiles with no errors at WARNS=2. At WARNS=3, it used to
compile
with 9 excessive warnings (about the good style of omitting redundant
initializers). Now it compiles with 10 excessive warnings. 1 more
about
comparison between signed unsigned. This warning is a compiler
bug. Both
gcc-4.2.1 and clang-3.9.0 have it. It is enabled by -W, which is
put in
CFLAGS at WARNS >= 3, or by -Wsign-compare.
These compilers even complain about:
int c;
for (c = 0; c < 1U; c++)
foo();
where it is extremely clear that c never gets converted to a wrong
value
when it is promoted to unsigned for the comparison. Compilers should
only warn about sign mismatches if they can't figure out the ranges or
if they can figure out the ranges but dangerous promotiions occur.
Compilers do excessive unrolling and other optimizations of loops like
the above, and they must figure out the ranges for this.
I haven't looked at the code but it would seem like you can unsign c
and avoid the cast.
That would be much worse than the cast. The cast is at least in the
right place -- it unpoisons nitimems. "Unsigning" c would poison C.
Yeah. This might introduce a domino effect though of changes.
As I said I hadn't looked at the code; the "c" is holding the return
value of getopt() and later kvm_nlist() before getting reused as a loop
index so it must be an int.
I would think it is such reuse of the same variable which is poisonous,
although it does involve some efficiency.
Pedro.
Domino effect = fast poisoning. The dominos collapse immediately, but
poisoning takes a long time to spread. In K&R-1 size_t didn't even
exist and the type of sizeof() was unspecified. size_t was born with
unsign poisoning a little later. That is almost 40 years ago, but its
poisoning hasn't spread to many loop variables yet. Full poisoning would
spread it to the closure of all uses of the loop variables, or better
stopped at the source of the poisoning. In the above, the variable is
not used outside of the loop, so the closure is small and easy to see.
Bruce
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
