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.


_______________________________________________
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"

Reply via email to