[MPlayer-dev-eng] [PATCH] vf_delogo: variable rectangle

[Reimar Döffinger]

On Wed, Mar 16, 2011 at 07:49:57PM +0100, Nicolas George wrote:
> Le duodi 22 pluviôse, an CCXIX, Reimar Döffinger a écrit :
> > if (alloc_rect > INT_MAX / sizeof(*rect))
> > 
> > Always keep the value you need to validate on its own, otherwise
> > it gets really difficult to make sure you didn't miss anything
> > (in this case it also has the very minor advantage of not requiring
> > a division at runtime, it can be calculated at compile-time).
> > Since you always multiply by two other checks (like checking that
> > previous_n * sizeof() < INT_MAX/2) are possible as well.
> 
> I did not find struct_realloc, but it does not take care of dying anyway.

libmpdemux/demuxer.h

> Here is a new version that introduce *alloc functions that print an error
> message and exit if they fail (keep in mind that most current OS overcommit
> the memory, so the mallocs usually do not fail anyway). They could be more
> widely used in the rest of the code.

I don't understand what the fascination with those hard aborts are.
It won't quite properly, it might leave behind gigantic core dumps
causing issues due to the file system filling up, the terminal restoration
code is there but I think it still uses non-signal safe functions and thus
is not reliable at all, when encoding the file header won't be written thus
possibly leaving behind a completely useless file even if it happens on
e.g. the last black frame nobody cares about anyway.
Yes, some of these do not apply here, and not failing promptly is ugly here since
this is basically an encode-only feature, but still I just can't see how aborting
would be a good idea ever (except that it might exploitable errors less likely
or harder to exploit, but it doesn't seem that great for that either).