[MPlayer-dev-eng] [PATCH] Dohm found some Bug: Playlist wrong

[Fabian Franz]

Am Montag, 3. Februar 2003 00:27 schrieb Arpi:
> Hi,
>
> > Btw. Please comment on this patch overall ...
> > Do you think its OK ?
>
> good q.
> you know it's freezee now...
> you also know that Albeu has been kidnapped by UFOs or sth, at least no one
> can reach him.

yes 

>
> so i would split your patch to 2 parts:
> common changes, to playtree.c
> and the gui-only parts.

ok

>
> the later is simple case, if it fixes playback with gui and .so says OK
> then it may be commited now.

ok

>
> the playtree.c changes are various things:
>
> +  //DEBUG_FF: Where are the childs freed ?
> +  // Attention in using this function!
> adding comments shouldn't hurt :)

ok

>
> -  if(pt->params == NULL)
> -    printf("Can't realloc params\n");
> +  if(pt->params == NULL) {
> +      mp_msg(MSGT_PLAYTREE,MSGL_ERR,"Can't allocate %d bytes of
> memory\n",(n+2) +      return;
> +  }
>
> i don't like these changes, the error msg should be something meaningful...
> users usually aren't interested in how many bytes can't bea allocated, but
> are interested in the reason, ie which part caused memory problems.

ok, I did it that way, because it was like that in the file whereever I looked 
...

>
> and the last one:
>    iter->stack_size--;
> -  iter->loop = iter->status_stack[iter->stack_size];
>    if(iter->stack_size > 0)
> +  {
> +    iter->loop = iter->status_stack[iter->stack_size];
>      iter->status_stack =
> (int*)realloc(iter->status_stack,iter->stack_size*sizeof(int)); +  }
>
> i have no idea, but i would be happy if you could prove this is required
> and is a bug or something. or if Albeu would say: "ok, commit!".
> (ok don't run firing your fake-mail sender app yet :))

ok, it is a 10l, but my fix is not right either as I think about it ...

iter->stack_sitze=0; (can happen)

iter->stack_size--;

now iter->stack_size=-1;

iter->loop = iter->status_stack[iter->stack_size];

Now it'll segfault!

I found it while experimenting, so it can be that it was another bug that 
caused it, but code is not rock solid, is it ? ;-)

So there should be one additional if to check if (iter->stack_size >= 0 && 
iter->status_stack)

To avoid unnecessary segfaults ...

>
> what's the value/purpose of iter->loop ?
>
> > Are there parts that need to be rewritten, made different ?
>
> it would be better if the new functions you added to mplayer.c would be in
> a new file inside the Gui/ dir. i'm getting ideg by seeing the word 'gtk'
> in mplayer.c :)

ok, thats ok, I'll add it to Gui/mplayer/play.c ...

But there will be one gtk still be in mplayer-code is it ok, or should it be 
encapsulated into some function in play.c ?

Thx for your comments!

Always helps a lot!

cu

Fabian
>
>
> A'rpi / Astral & ESP-team