Date: Fri, 10 Mar 2017 18:45:49 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Marcelo Araujo <araujo@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r314989 - head/usr.bin/vmstat Message-ID: <20170310181404.C1416@besplex.bde.org> In-Reply-To: <201703100449.v2A4neTK046456@repo.freebsd.org> References: <201703100449.v2A4neTK046456@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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. Compilers mostly err in the other direction by not warning when they should, since average code has many sign mismatches and warning about them all would give too many warnings. Compilers have more excuses for not being able to figure out the ranges in complicated expressions, and until they are better at this so that they can avoid generating spurious warnings, they are correct to never warn. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170310181404.C1416>