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

Peter Kasting pkasting at google.com
Sat Aug 30 04:43:50 CEST 2014


On Fri, Aug 29, 2014 at 7:25 PM, Michael Niedermayer <michaelni at gmx.at>
wrote:

> On Fri, Aug 29, 2014 at 04:38:28PM -0700, Peter Kasting wrote:
> > I think that reinforces why a change like this is important.  These sorts
> > of value truncations shouldn't just be invisibly, implicitly happening,
> > since in many cases they can have important consequences.  If your code
> > stores a double in a float, you should be thinking about whether
> precision
> > loss is important.  If it stores a floating-point value in an integral
> one,
> > you should be wondering if you need to round rather than truncate.  If it
> > stores a 64-bit int in a 32-bit one, you should be thinking about
> potential
> > integer overflow and whether the storage type needs to be increased to
> e.g.
> > handle large files or long data streams.  Etc.
>
> but ...
>
> a = (uint8_t)b
> doesnt (just) mean "hey i know a is uint8 and b is not, dont warn me"
> it means
> "truncate b to 8bit"
>

Yes.


> now if someone changed a to be 16bit or it was that in the first place
> you might have a bug with the cast but not without and finding that
> one could be tough
>

You've mentioned two cases.  The first involves changing code later.  The
second involves insertion of these casts now.

The second one can presumably be dealt with by careful codereview + testing
to ensure we're doing the right thing now, so I don't think that's a
long-term worry.  (And, unless I made errors in the patch, I didn't
generally do things like cast a value to a type shorter than where it was
being stored, so hopefully there aren't cases like this.)

The first, I think, is not a case where the lack of a cast is a good thing.
 If you change the type of a variable, and as a result it changes the value
assigned to that variable, you had better know that that change happened,
and that it's what you want.  I claim that it's easier to find and evaluate
such cases in the presence of explicit casts than in their absence.  The
cast is only problematic if you're not actually auditing uses of the
variable whose type you're changing.  That's a recipe for disaster no
matter what.

also this patch seems to break the code
> make fate ends in
>
> make: *** [tests/data/ffprobe-test.nut] Error 134
> make: *** Waiting for unfinished jobs....


Thanks for this.  I wasn't able to run "make fate" on this machine because
my msysgit environment doesn't have make set up correctly :(.  I've been
casting about for another environment in which to run it.  In the meantime,
how can I dive deeper into what "error 134" actually means?  My local git
checkout of FFMPEG doesn't seem to have a tests/data directory.

PK


More information about the ffmpeg-devel mailing list