avifil32: Removed sign comparison warning (sizeof expresions)
Michael Stefaniuc
mstefani at redhat.com
Mon Aug 23 10:53:56 CDT 2010
Marko Nikolic wrote:
> Andrey Turkin wrote:
>
>> On Monday 23 August 2010 13:16:07 Michael Stefaniuc wrote:
>>> IMHO gcc is *wrong* in emitting a warning there. sizeof(PCMWAVEFORMAT)
>>> is a compile time constant and gcc can see that sizeof(PCMWAVEFORMAT)
>>> falls well inside the number range expressible by a LONG. Logically
>>> there is no difference between
>>> formatsize <= sizeof(PCMWAVEFORMAT)
>>> and
>>> formatsize <= 16
>>> One gives a bogus warning and the other doesn't.
>> C99 std (para 6.5.3.4.4) states following about sizeof operator:
>> "...its type (an unsigned integer type) is size_t, defined in <stddef.h>
>> (and other headers)."
>>
>> sizeof result is a compile-time constant but, unlike numeric constants,
>> its type must always be size_t so gcc does the correct thing here.
>
> The same is with C89, paragraph 3.3.3.4 from standard:
>
> The value of the result is implementation-defined, and its type (an
> unsigned integral type) is size_t defined in the <stddef.h> header.
>
> So, the gcc is correct. In the example above, correct would be
>
> formatsize <= 16U
>
> which gives a warning, as expected.
Yeah, sorry about that. I ASSumed the C spec would preserve the value
and as sizeof(PCMWAVEFORMAT) is a constant expression it would be
promoted to a LONG as that would preserve the values of both sides. gcc
has no choice in this case then to follow the C standard. So I've read
up on it and now I blame the C standard; especially after reading the
"Coding guidelines" on page 7 of
http://c0x.coding-guidelines.com/6.5.3.4.pdf aka the commentary to the
changes proposed to sizeof() in the next C standard iteration. So much
for the "principle of the least surprise" ...
> What remains is how to correctly remove warning. In this case (and there are
> many similar in the code), signed function parameter is comparing with
> values that are natively unsigned. Changing type of the parameter is not
> possible, the same if with sizeof operator. One possiblity is to add some
> temporary variable, but in my opinioin it will just unncesary bloat the code
> and is worse than casting return value of sizeof.
>
> Maybe better solution is to make signed_sizeof macro, which will always
> return signed values and use it instead of sizeof; I will check if there is
> possiblity for that.
bye
michael
More information about the wine-devel
mailing list