From ace44bc9f59e2ced98bb8534aa225fce9a479d8b Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Tue, 15 Jul 2008 09:40:39 +0200 Subject: [PATCH] playlist: Use PL_LOCK_IF and PL_UNLOCK_IF to perform some more checks on lock state. And fix a unlocked usage of get_current_status_item(). --- src/playlist/control.c | 4 ++-- src/playlist/engine.c | 1 - src/playlist/item.c | 34 ++++++++++++++++---------------- src/playlist/playlist_internal.h | 13 +++++++++++- src/playlist/search.c | 19 ++++++++++-------- 5 files changed, 42 insertions(+), 29 deletions(-) diff --git a/src/playlist/control.c b/src/playlist/control.c index fbbf9412e8..2cb00e99cc 100644 --- a/src/playlist/control.c +++ b/src/playlist/control.c @@ -65,10 +65,10 @@ int playlist_Control( playlist_t * p_playlist, int i_query, va_list args; int i_result; va_start( args, b_locked ); - if( !b_locked ) PL_LOCK; + PL_LOCK_IF( !b_locked ); i_result = PlaylistVAControl( p_playlist, i_query, args ); va_end( args ); - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); return i_result; } diff --git a/src/playlist/engine.c b/src/playlist/engine.c index c01450e1e2..ba516c86a4 100644 --- a/src/playlist/engine.c +++ b/src/playlist/engine.c @@ -272,7 +272,6 @@ input_thread_t * playlist_CurrentInput( playlist_t * p_playlist ) return p_input; } - /** * @} */ diff --git a/src/playlist/item.c b/src/playlist/item.c index 66573c98c6..5d09181b22 100644 --- a/src/playlist/item.c +++ b/src/playlist/item.c @@ -256,10 +256,10 @@ int playlist_DeleteInputInParent( playlist_t *p_playlist, int i_input_id, playlist_item_t *p_root, bool b_locked ) { int i_ret; - if( !b_locked ) PL_LOCK; + PL_LOCK_IF( !b_locked ); i_ret = DeleteFromInput( p_playlist, i_input_id, p_root, true ); - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); return i_ret; } @@ -276,12 +276,12 @@ int playlist_DeleteFromInput( playlist_t *p_playlist, int i_input_id, bool b_locked ) { int i_ret1, i_ret2; - if( !b_locked ) PL_LOCK; + PL_LOCK_IF( !b_locked ); i_ret1 = DeleteFromInput( p_playlist, i_input_id, p_playlist->p_root_category, true ); i_ret2 = DeleteFromInput( p_playlist, i_input_id, p_playlist->p_root_onelevel, true ); - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); return ( i_ret1 == VLC_SUCCESS || i_ret2 == VLC_SUCCESS ) ? VLC_SUCCESS : VLC_ENOITEM; } @@ -295,10 +295,10 @@ int playlist_DeleteFromInput( playlist_t *p_playlist, int i_input_id, */ void playlist_Clear( playlist_t * p_playlist, bool b_locked ) { - if( !b_locked ) PL_LOCK; + PL_LOCK_IF( !b_locked ); playlist_NodeEmpty( p_playlist, p_playlist->p_local_category, true ); playlist_NodeEmpty( p_playlist, p_playlist->p_local_onelevel, true ); - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); } /** @@ -403,7 +403,7 @@ int playlist_AddInput( playlist_t* p_playlist, input_item_t *p_input, PL_DEBUG( "adding item `%s' ( %s )", p_input->psz_name, p_input->psz_uri ); - if( !b_locked ) PL_LOCK; + PL_LOCK_IF( !b_locked ); /* Add to ONELEVEL */ p_item_one = playlist_ItemNewFromInput( p_playlist, p_input ); @@ -421,7 +421,7 @@ int playlist_AddInput( playlist_t* p_playlist, input_item_t *p_input, GoAndPreparse( p_playlist, i_mode, p_item_cat, p_item_one ); - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); return VLC_SUCCESS; } @@ -452,11 +452,11 @@ int playlist_BothAddInput( playlist_t *p_playlist, int i_top; assert( p_input ); - if( !b_locked ) PL_LOCK; + PL_LOCK_IF( !b_locked ); if( !vlc_object_alive( p_playlist ) ) { - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); return VLC_EGENERIC; } @@ -491,7 +491,7 @@ int playlist_BothAddInput( playlist_t *p_playlist, if( i_cat ) *i_cat = p_item_cat->i_id; if( i_one ) *i_one = p_item_one->i_id; - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); return VLC_SUCCESS; } @@ -520,13 +520,13 @@ playlist_item_t * playlist_NodeAddInput( playlist_t *p_playlist, if( p_playlist->b_die ) return NULL; - if( !b_locked ) PL_LOCK; + PL_LOCK_IF( !b_locked ); p_item = playlist_ItemNewFromInput( p_playlist, p_input ); if( p_item == NULL ) return NULL; AddItem( p_playlist, p_item, p_parent, i_mode, i_pos ); - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); return p_item; } @@ -566,11 +566,11 @@ playlist_item_t *playlist_ItemToNode( playlist_t *p_playlist, * useful for later BothAddInput ) */ - if( !b_locked ) PL_LOCK; + PL_LOCK_IF( !b_locked ); /* Fast track the media library, no time to loose */ if( p_item == p_playlist->p_ml_category ) { - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); return p_item; } @@ -603,13 +603,13 @@ playlist_item_t *playlist_ItemToNode( playlist_t *p_playlist, vlc_object_signal_unlocked( p_playlist ); var_SetInteger( p_playlist, "item-change", p_item_in_category-> p_input->i_id ); - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); return p_item_in_category; } else { ChangeToNode( p_playlist, p_item ); - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); return NULL; } } diff --git a/src/playlist/playlist_internal.h b/src/playlist/playlist_internal.h index 155dae0851..e4369e370b 100644 --- a/src/playlist/playlist_internal.h +++ b/src/playlist/playlist_internal.h @@ -143,7 +143,18 @@ void playlist_set_current_input( #define PLI_NAME( p ) p && p->p_input ? p->p_input->psz_name : "null" -#define PL_ASSERT_LOCKED vlc_assert_locked( &(vlc_internals(p_playlist)->lock) ); +#define PL_ASSERT_LOCKED vlc_assert_locked( &(vlc_internals(p_playlist)->lock) ) +#define PL_LOCK_IF( cond ) pl_lock_if( p_playlist, cond ) +static inline void pl_lock_if( playlist_t * p_playlist, bool cond ) +{ + if( cond ) PL_LOCK; else PL_ASSERT_LOCKED; +} + +#define PL_UNLOCK_IF( cond ) pl_unlock_if( p_playlist, cond ) +static inline void pl_unlock_if( playlist_t * p_playlist, bool cond ) +{ + if( cond ) PL_UNLOCK; +} #endif /* !__LIBVLC_PLAYLIST_INTERNAL_H */ diff --git a/src/playlist/search.c b/src/playlist/search.c index 5d9238a22f..fc46c5671a 100644 --- a/src/playlist/search.c +++ b/src/playlist/search.c @@ -44,14 +44,14 @@ playlist_item_t * playlist_ItemGetById( playlist_t * p_playlist , int i_id, bool b_locked ) { int i; - if( !b_locked ) PL_LOCK; + PL_LOCK_IF( !b_locked ); ARRAY_BSEARCH( p_playlist->all_items,->i_id, int, i_id, i ); if( i != -1 ) { - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); return ARRAY_VAL( p_playlist->all_items, i ); } - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); return NULL; } @@ -67,23 +67,26 @@ playlist_item_t * playlist_ItemGetByInput( playlist_t * p_playlist , bool b_locked ) { int i; - if( !b_locked ) PL_LOCK; + PL_LOCK_IF( !b_locked ); if( get_current_status_item( p_playlist ) && get_current_status_item( p_playlist )->p_input == p_item ) { - if( !b_locked ) PL_UNLOCK; - return get_current_status_item( p_playlist ); + /* FIXME: this is potentially dangerous, we could destroy + * p_ret any time soon */ + input_item_t *p_ret = get_current_status_item( p_playlist ); + PL_UNLOCK_IF( !b_locked ); + return p_ret; } /** \todo Check if this is always incremental and whether we can bsearch */ for( i = 0 ; i < p_playlist->all_items.i_size; i++ ) { if( ARRAY_VAL(p_playlist->all_items, i)->p_input->i_id == p_item->i_id ) { - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); return ARRAY_VAL(p_playlist->all_items, i); } } - if( !b_locked ) PL_UNLOCK; + PL_UNLOCK_IF( !b_locked ); return NULL; } -- 2.39.2