[MPlayer-dev-eng] [PATCH] Crash when invalid file is supplied as a playlist

[Reimar Döffinger]

Hello,
On Mon, Sep 24, 2007 at 08:23:21PM +0200, Diego Biurrun wrote:
> On Mon, Sep 24, 2007 at 08:54:03PM +0300, Uoti Urpala wrote:
> > On Mon, 2007-09-24 at 17:21 +0200, Jernej Azarija wrote:
> > > I made a 3 second skim on the code to find this :
> > > 
> > >  23   play_tree_t* r = calloc(1,sizeof(play_tree_t));
> > >  24   if(r == NULL)
> > >  25     mp_msg(MSGT_PLAYTREE,MSGL_ERR,"Can't allocate %d bytes of
> > > memory\n",(int)sizeof(play_tree_t));
> > >  26   r->entry_type = PLAY_TREE_ENTRY_NODE;
> > > 
> > > If the code is full of such stuff then it's indeed time for a rewrite.
> > > I'd only need some specifications about what the code should actually
> > > do (i'm clueles about playlist stuff) and an ack that it's indeed a
> > > good thing to make a rewrite/audit.
> > 
> > I agree that the code as a whole isn't nice (it suffers from unnecessary
> > generalization and complexity among other things), but I think that
> > cleaning it up would not be an easy task for someone not familiar with
> > MPlayer internals. The playtree code is mixed with other internals such
> > as parsing and setting per-file option values.
> 
> That said, if you are unafraid of difficult tasks, I think this would
> not be unwelcome.

I'd like to state that for my part I highly prefer cleanups over
rewrites, even if the end result would be the same just with more
effort.
And as with most old MPlayer code, there probably are lots of
opportunities to simplify things without changing behaviour at all (e.g.
by just removing the message in the example above, it serves no useful
purpose).

Greetings,
Reimar Döffinger