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

[Jernej Azarija]

Hello,
On 9/24/07, Uoti Urpala <uoti.urpala at pp1.inet.fi> 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.
>
> This particular example is not that bad really. MPlayer as a whole can
> not be expected to work if any memory allocation calls fail.

I'm now wondering if there are applications that work successfully
even when memory allocation fails.

> The code could use abort() instead of generating a segfault, but IMO that
> distinction in a rare error case isn't too important (I assume that
> entry_type has a small enough offset that NULL->entry_type is guaranteed
> to trigger a segfault on any practical system).

If segfaulting vs abort()ing isn't important, then I'd consider it
important from the code consistency view. In the same file some
allocations are checked and handled upon. Having code that at some
places segfaults and aborts at others  due to malloc failure is
stupid.


> 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.

True. Actually what do you think about collecting suggestions about
the actual code and try just to clean it up?

> _______________________________________________
> MPlayer-dev-eng mailing list
> MPlayer-dev-eng at mplayerhq.hu
> http://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng
>