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

[Arpi]

Hi,

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

ah. i didn't know that... but it's wrong, even if it's such everywhere :)

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

it's clear. but if the code is well written, 'iter->stack_sitze=0; (can
happen)' won't happen at all.

and anywa, if iter->stack_sitze=1; then iter->stack_size--; then
iter->stack_sitze=0; so iter->loop = iter->status_stack[iter->stack_size];
is valid but your "fix" moves it to if(iter->stack_size>0) instead of
if(iter->stack_size>=0) ...

> 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 ? ;-)

dunno. at least the comandline part works well since a while, so i want to
avoid breaking it now by commiting patch we don't really understand.

> So there should be one additional if to check if (iter->stack_size >= 0 && 
> iter->status_stack)
> To avoid unnecessary segfaults ...

yes, do it then

> But there will be one gtk still be in mplayer-code is it ok, or should it be 

why?


A'rpi / Astral & ESP-team

--
Developer of MPlayer, the Movie Player for Linux - http://www.MPlayerHQ.hu
    "However, many people beg for its inclusion in Debian. Why?" - Gabucino
  "Because having new software in Debian is good." - Josselin Mouette
"Because having good software in Debian is new." - Gabucino