On 16 May 2012, at 09:56, "Don Moir" <donmoir at comcast.net> wrote:
> ----- Original Message ----- From: "Reimar Döffinger" <Reimar.Doeffinger at gmx.de>
> To: "FFmpeg development discussions and patches" <ffmpeg-devel at ffmpeg.org>
> Sent: Wednesday, May 16, 2012 3:48 AM
> Subject: Re: [FFmpeg-devel] [PATCH] Fix warnings about int64 to int32conversion
>>>> On 16 May 2012, at 01:07, Michael Bradshaw <mbradshaw at sorensonmedia.com> wrote:
>>>>> Attached patch fixes the following two warnings about converting from
>>> int64 to int32:
>>>>>> 1>ffmpeg_latest\include\libavutil\common.h(174) : warning C4244:
>>> 'return' : conversion from 'int64_t' to 'int32_t', possible loss of
>>> data
>>> 1>ffmpeg_latest\include\libavutil\common.h(233) : warning C4244:
>>> 'argument' : conversion from 'uint64_t' to 'uint32_t', possible loss
>>> of data
>>>> First, if the compiler generates a warning for the >> 32 case it is just being stupid, there can be no loss of data in that case.
>> Second, that warning is generally just nonsense and I don't really see much point in cluttering the code with it, you can just disable the warning. IMHO.
>> Since this and the one in rational.h are the only warnings in the entire primary headers, they should be fixed since then noone has to deal with it. It's clear there is no loss of data and you don't want to have to disable warning C4244 in general. There is no real clutter either.
First: my main point was that there are two warnings quoted and more than two casts added, so I wanted explicit confirmation that each and every one is really needed.
Second: If really all these casts are needed the I believe a compiler bug report is in order, since there is really no excuse for printing a warning for some of those.
Concerning clutter: I generally consider any superfluous cast a rather bad kind of clutter, in C the casts are hard enough to grep for, adding a few with no purpose does not make things better.
There is also the issue of this leading to inconsistent style between these headers and all other code.
I do not mind so much about that part (though I believe the inconsistency means those casts need a comment on their purpose), but the first two I consider somewhat important.