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

[Uoti Urpala]

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

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