[FFmpeg-devel] [PATCH] Fix MSVC warnings about possible value truncation.

[Peter Kasting]

On Tue, Sep 2, 2014 at 3:19 PM, wm4 <nfxjfg at googlemail.com> wrote:

> Here's why patches like these are bad: they're
> HUGE. Reviewing them takes a lot of time, and the result is
> questionable. How are we going to do v2 of this patch? And v3 etc.?
> Probably not at all. It's just too monolithic. It's like reviewing a
> phone book.
>
> For the next time, I'd suggest putting every real fix (or attempt of a
> fix) into a separate patch - even if it results in dozens of patches.
> At least this makes it easier to see what patch got rejected, what got
> accepted, and what needs further work.


I'm perfectly happy to do that -- indeed, I tried to say in my very first
email that every file in this patch is basically independent, and I could
break the patch down into chunks of whatever size people deemed reasonable,
even one per file (or, potentially, even smaller).  I just wasn't sure what
would make sense.  Telling me "please come back with a series of extremely
tiny patches" is fine feedback.  What I'll do with that feedback, going
forward, is try and split this patch into small pieces that account for the
various issues you raise separately.  Hopefully that's reasonable.

Another problem: this mixes hiding compiler warnings and trying to fix
> real problems. And in many cases, hiding real problems. Just plastering
> everything with "(type)expr" won't help much, especially not if "type"
> is signed, because it doesn't change anything about the legality of an
> overflow.


I tried not to "just plaster things with casts", but as I said, I was
counting on review feedback to help me understand where changes would be
fixing real problems or hiding real problems, as opposed to just silencing
warnings.  The ultimate goal, of both this patch and of enabling these
sorts of warnings in general, is to find and fix the real problems, hide no
problems, and hopefully not cost too much in terms of additional code noise
to "silence" non-problem cases.

Thanks for the review comments, to both of you who provided some.  I will
go try to address them and return with smaller patches.

PK