[vlc-devel] RE : RE : [PATCH] -- skins2 (more corrections)

[brezhoneg1]

> 
> Le lundi 16 mars 2009 22:53:37 brezhoneg1, vous avez écrit :
> > > I must say that...
> > >
> > >      if( !b_playlist_locked ) PL_UNLOCK;
> > >
> > > +
> > > +    if( b_playlist_locked ) PL_UNLOCK;
> > >
> > > ...that smells utmostly fishy to me.
> >
> > The function playlist_CurrentInput( ) automatically holds the
playlist.
> > Since this lock may already be held before entering the function,
this
> > results in EDEADLCK(35).
> 
> That's beside the point. The point is that this code pattern is fishy:
> 
> 	if (a) b();
> 	if (!a) b();
> 
> It might be correct, but it is obviously equivalent to:
> 	b();
> 
> And hence, I take it as a strong hint that the author has not looked
at
> the "big picture" while writing the patch. While that happens in my
code
> as
> well but it's not an excuse.
> 

Ok, sorry, I now see what is fishy. But, I did see the big picture. I
found this lock processing a real mess. Every other line in DBUS, a
command to the playlist must be inserted in this sort of tests. The
previous line was there because two lines above (not seen in the patch)
another such test was done. Mutualizing as you suggest in this situation
will still add to the difficulty to read this code.

Erwan10