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