[vlc-devel] commit: playlist: Proper locking when editing playlist variable in loadsave .c. Also correctly release p_ml_onelevel->p_input and p_ml_category-> p_input when changing them. (Pierre d'Herbemont )

[Enrique Osuna]

I might be speaking out of line here, but why decref potentially twice
on the same object:

+    if( p_playlist->p_ml_onelevel->p_input )
+        vlc_gc_decref( p_playlist->p_ml_onelevel->p_input );
+    if( p_playlist->p_ml_category->p_input )
+        vlc_gc_decref( p_playlist->p_ml_category->p_input );

when you only incref once:

    p_playlist->p_ml_onelevel->p_input =
    p_playlist->p_ml_category->p_input = p_input;
    vlc_gc_incref( p_input );

Shouldn't you incref for every place that you store p_input for example:
    p_playlist->p_ml_onelevel->p_input = p_input;
    vlc_gc_incref( p_input );
    p_playlist->p_ml_category->p_input = p_input;
    vlc_gc_incref( p_input );

Enrique

On Tue, Apr 1, 2008 at 12:28 AM, git version control <git at videolan.org> wrote:
> vlc | branch: master | Pierre d'Herbemont <pdherbemont at videolan.org> | Tue Apr  1 01:25:54 2008 +0200| [00dbcd22a3ad188806a35c56f75b75b86cdab8a6]
>
>  playlist: Proper locking when editing playlist variable in loadsave.c. Also correctly release p_ml_onelevel->p_input and p_ml_category->p_input when changing them.
>
>  This fixes partly make check and input items leaks.
>
>  > http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=00dbcd22a3ad188806a35c56f75b75b86cdab8a6
>  ---
>
>   src/playlist/loadsave.c |   14 +++++++++++++-
>   1 files changed, 13 insertions(+), 1 deletions(-)
>
>  diff --git a/src/playlist/loadsave.c b/src/playlist/loadsave.c
>  index 55b2a27..28b163e 100644
>  --- a/src/playlist/loadsave.c
>  +++ b/src/playlist/loadsave.c
>  @@ -148,19 +148,31 @@ int playlist_MLLoad( playlist_t *p_playlist )
>      if( p_input == NULL )
>          goto error;
>
>  +    PL_LOCK;
>  +    if( p_playlist->p_ml_onelevel->p_input )
>  +        vlc_gc_decref( p_playlist->p_ml_onelevel->p_input );
>  +    if( p_playlist->p_ml_category->p_input )
>  +        vlc_gc_decref( p_playlist->p_ml_category->p_input );
>  +
>      p_playlist->p_ml_onelevel->p_input =
>      p_playlist->p_ml_category->p_input = p_input;
>  -
>      vlc_gc_incref( p_input );
>  +    PL_UNLOCK;
>
>      vlc_event_attach( &p_input->event_manager, vlc_InputItemSubItemAdded,
>                          input_item_subitem_added, p_playlist );
>
>  +    PL_LOCK;
>      p_playlist->b_doing_ml = VLC_TRUE;
>  +    PL_UNLOCK;
>  +
>      stats_TimerStart( p_playlist, "ML Load", STATS_TIMER_ML_LOAD );
>      input_Read( p_playlist, p_input, VLC_TRUE );
>      stats_TimerStop( p_playlist,STATS_TIMER_ML_LOAD );
>  +
>  +    PL_LOCK;
>      p_playlist->b_doing_ml = VLC_FALSE;
>  +    PL_UNLOCK;
>
>      vlc_event_detach( &p_input->event_manager, vlc_InputItemSubItemAdded,
>                          input_item_subitem_added, p_playlist );
>
>  _______________________________________________
>  vlc-devel mailing list
>  To unsubscribe or modify your subscription options:
>  http://mailman.videolan.org/listinfo/vlc-devel
>