Date: Fri, 10 Mar 2017 11:19:33 -0800 From: Ngie Cooper <yaneurabeya@gmail.com> To: Pedro Giffuni <pfg@FreeBSD.org> Cc: Bruce Evans <brde@optusnet.com.au>, Marcelo Araujo <araujo@freebsd.org>, 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: <DECB06D9-8136-491E-B786-0C853C70C472@gmail.com> In-Reply-To: <0906af1d-09aa-fd1e-35cc-9361a68fc160@FreeBSD.org> References: <201703100449.v2A4neTK046456@repo.freebsd.org> <20170310181404.C1416@besplex.bde.org> <0906af1d-09aa-fd1e-35cc-9361a68fc160@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> On Mar 10, 2017, at 03:59, Pedro Giffuni <pfg@FreeBSD.org> wrote: >=20 >> On 3/10/2017 2:45 AM, Bruce Evans wrote: >>> On Fri, 10 Mar 2017, Marcelo Araujo wrote: >>>=20 >>> ... >>> Log: >>> Use nitems() from sys/param.h and also remove the cast. >>>=20 >>> Reviewed by: markj >>> MFC after: 3 weeks. >>> Differential Revision: https://reviews.freebsd.org/D9937 >>> ... >>> Modified: head/usr.bin/vmstat/vmstat.c >>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=20 >>> --- 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 =3D "_cnt"; >>> goto retry_nlist; >>> } >>> - for (c =3D 0; >>> - c < (int)(sizeof(namelist)/sizeof(namelist[0])); >>> - c++) >>> + for (c =3D 0; c < nitems(namelist); c++) >>> if (namelist[c].n_type =3D=3D 0) >>> bufsize +=3D strlen(namelist[c].n_name) + 1; >>=20 >> This undoes fixes to compile at WARNS=3D2 in r87690 and now breaks at WAR= NS=3D3. >> vmstat is still compiled at WARNS=3D1. >>=20 >> nitems suffers from the same unsigned poisoning as the sizeof() expressio= n >> (since it reduces to the same expression. Casting to int in the expressi= on >> to fix the warning would break exotic cases. Of course, nitems is >> undocumented so no one knows when it is supposed to work). >>=20 >> vmstat compiles with no errors at WARNS=3D2. At WARNS=3D3, it used to co= mpile >> 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. Bot= h >> gcc-4.2.1 and clang-3.9.0 have it. It is enabled by -W, which is put in >> CFLAGS at WARNS >=3D 3, or by -Wsign-compare. >>=20 >> These compilers even complain about: >>=20 >> int c; >>=20 >> for (c =3D 0; c < 1U; c++) >> foo(); >>=20 >> 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. >>=20 >=20 > I haven't looked at the code but it would seem like you can unsign c and a= void the cast. Yeah. This might introduce a domino effect though of changes. Cheers! -Ngie=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?DECB06D9-8136-491E-B786-0C853C70C472>