playlist: Use PL_LOCK_IF and PL_UNLOCK_IF to perform some more checks on lock...

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

@@ -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;
 }

src/playlist/engine.c

@@ -272,7 +272,6 @@ input_thread_t * playlist_CurrentInput( playlist_t * p_playlist )
     return p_input;
 }
 
-
 /**
  * @}
  */

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;
     }
 }

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 */

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;
 }