svn commit: r314989 - head/usr.bin/vmstat
Pedro Giffuni
pfg at FreeBSD.org
Fri Mar 10 11:56:27 UTC 2017
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.
More information about the svn-src-head
mailing list