Date: Fri, 10 Mar 2017 06:59:23 -0500 From: Pedro Giffuni <pfg@FreeBSD.org> To: Bruce Evans <brde@optusnet.com.au>, 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: <0906af1d-09aa-fd1e-35cc-9361a68fc160@FreeBSD.org> In-Reply-To: <20170310181404.C1416@besplex.bde.org> References: <201703100449.v2A4neTK046456@repo.freebsd.org> <20170310181404.C1416@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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. Pedro.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?0906af1d-09aa-fd1e-35cc-9361a68fc160>