> 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.
Yeah. This might introduce a domino effect though of changes. Cheers! -Ngie _______________________________________________ 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"
