[MPlayer-cvslog] r26644 - trunk/libmpdemux/demux_asf.c

[Uoti Urpala]

On Fri, 2008-05-02 at 20:46 +0400, Evgeniy Stepanov wrote:
> On Friday 02 May 2008 19:48:36 Michael Niedermayer wrote:
> > On Fri, May 02, 2008 at 03:33:14PM +0200, eugeni wrote:
> > > Check ASF packet size before calling demux_asf_read_packet. Fixes
> > > segfault with damaged ASF files.
> 
> [...]
> 
> > How does adding assert() prevent a segfault?
> 
> Huh? By leaving the program immediately instead of segfaulting later.

IMO assert should be used for cases where you "know it will be true" as
a debugging aid to guard against mistakes or later breakage. In error
cases which are expected to happen use abort() if you must make the
program stop. The compiler may ignore assert() macros if the code is
compiled with NDEBUG (the idea being that debugging stuff may be omitted
in "production" code).

Also I wouldn't say this "fixes segfault" in the commit message. A full
fix would be to make the demuxer return an error instead of terminating
the whole program. This just changes it from a segfault to another kind
of crash. I don't mean that I'd necessarily oppose doing less than a
full fix, but IMO the commit message should be something along the lines
of "demux_asf: abort() instead of random segfault".