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

[Jernej Azarija]

On 9/24/07, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> 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).

I agree completely.