svn commit: r228668 - head/usr.bin/netstat

[Dimitry Andric]

On 2011-12-18 02:39, David Schultz wrote:
...
>>> Log:
>>>   Revert r228650, and work around the clang false positive with printf
>>>   formats in usr.bin/netstat/atalk.c by conditionally adding NO_WFORMAT to
>>>   the Makefile instead.
... 
> Have you been keeping track of the other hacks you've been
> sprinkling throughout the tree to work around clang bugs, e.g.,
> the one in fsdb?  It would be unfortunate if someone else has to
> waste their time later on figuring out what you did, when we could
> just as easily have waited a month for the clang bug to be fixed.

Yes, I will revert the fsdb change too, and add a workaround in the
Makefile.  Though after talking with a language lawyer type person, it
seems that clang was actually right with its original warning.

Apparently, if you use ?: with two shorts, the end result always gets
promoted to an int, due to the Usual Arithmetic Conversions, so using
the "%hu" format is not correct.  The following small program
demonstrates this:

#include <stdio.h>

int main(void)
{
        printf("%zu\n", sizeof(1 > 2 ? (short)1 : (short)2));
        return 0;
}

It will always print sizeof(int), e.g. 4 on most arches.  This is not
what most programmers expect, I guess, at least I didn't. :)

Since htons() and ntohs() are implemented in our tree with the __bswap
macros, which use ?: operators (at least when GNU inline asm and
__builtin_constant_p are supported), they will in fact return int, not
uint16_t.

A better fix is to add explicit casts to the __bswap macros, as per
attached patch, which I'm running through a make universe now.  (Note
that some arches, such as arm and mips already add the explicit casts
for the expected __bswap return types.)  Would that be OK to commit?


> Incidentally, the "bug" you fixed in telnet/utilities.c is also a
> false positive; clang doesn't understand that an index into a
> string constant is also a string constant.

Yes, that is indeed a false positive, although this way of offsetting a
format string is a bit too clever, to the point of being unreadable. :)

I will create a PR for the clang guys, and revert this, adding
NO_WFORMAT to the Makefile as a workaround.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bswap-ternary-1.diff
Type: text/x-diff
Size: 3812 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-head/attachments/20111218/5f900fe7/bswap-ternary-1.bin