[vlc-devel] [PATCH 5/8] playlist: playlist_NodeDelete: privatize forceful delete

Filip Roséen filip at atch.se
Sun May 21 20:48:15 CEST 2017


Only the core should have permission to do forceful deletion of
entities.

This patch removes a mostly static, from outside of the core, argument
to playlist_NodeDelete, while also making it safer to use as it was
previously possible for anything with access to the function to delete
read-only entities.
---
 include/vlc_playlist.h                              |  2 +-
 modules/control/dbus/dbus_tracklist.c               |  2 +-
 modules/gui/macosx/VLCPLModel.m                     |  2 +-
 modules/gui/ncurses.c                               |  2 +-
 .../gui/qt/components/playlist/playlist_model.cpp   |  2 +-
 modules/gui/skins2/vars/playtree.cpp                |  2 +-
 modules/lua/libs/playlist.c                         |  2 +-
 src/playlist/engine.c                               | 10 ++++++++--
 src/playlist/item.c                                 |  4 ++--
 src/playlist/playlist_internal.h                    | 21 +++++++++++++++++++++
 src/playlist/services_discovery.c                   |  4 ++--
 src/playlist/tree.c                                 | 14 ++++++++++----
 12 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/include/vlc_playlist.h b/include/vlc_playlist.h
index d9c982354e..6774410433 100644
--- a/include/vlc_playlist.h
+++ b/include/vlc_playlist.h
@@ -384,7 +384,7 @@ VLC_API int playlist_LiveSearchUpdate(playlist_t *, playlist_item_t *, const cha
 /* Node management */
 VLC_API playlist_item_t * playlist_NodeCreate( playlist_t *, const char *, playlist_item_t * p_parent, int i_pos, int i_flags );
 VLC_API playlist_item_t * playlist_ChildSearchName(playlist_item_t*, const char* ) VLC_USED;
-VLC_API void playlist_NodeDelete( playlist_t *, playlist_item_t *, bool );
+VLC_API void playlist_NodeDelete( playlist_t *, playlist_item_t * );
 
 /**************************
  * Audio output management
diff --git a/modules/control/dbus/dbus_tracklist.c b/modules/control/dbus/dbus_tracklist.c
index 0f3676ff8d..4a8254dc9c 100644
--- a/modules/control/dbus/dbus_tracklist.c
+++ b/modules/control/dbus/dbus_tracklist.c
@@ -257,7 +257,7 @@ DBUS_METHOD( RemoveTrack )
 
         if( item->i_id == i_id )
         {
-            playlist_NodeDelete( p_playlist, item, false );
+            playlist_NodeDelete( p_playlist, item );
             break;
         }
     }
diff --git a/modules/gui/macosx/VLCPLModel.m b/modules/gui/macosx/VLCPLModel.m
index dc3b7c872c..379dbafd09 100644
--- a/modules/gui/macosx/VLCPLModel.m
+++ b/modules/gui/macosx/VLCPLModel.m
@@ -210,7 +210,7 @@ static int VolumeUpdated(vlc_object_t *p_this, const char *psz_var,
         PL_LOCK;
         playlist_item_t *p_root = playlist_ItemGetById(p_playlist, [item plItemId]);
         if( p_root != NULL )
-            playlist_NodeDelete(p_playlist, p_root, false);
+            playlist_NodeDelete(p_playlist, p_root);
         PL_UNLOCK;
     }];
 }
diff --git a/modules/gui/ncurses.c b/modules/gui/ncurses.c
index 4075313462..12c80ca827 100644
--- a/modules/gui/ncurses.c
+++ b/modules/gui/ncurses.c
@@ -1365,7 +1365,7 @@ static bool HandlePlaylistKey(intf_thread_t *intf, int key)
 
         PL_LOCK;
         item = playlist_ItemGetByInput(p_playlist, input);
-        playlist_NodeDelete(p_playlist, item, false);
+        playlist_NodeDelete(p_playlist, item);
 
         if (sys->box_idx >= sys->box_lines_total - 1)
             sys->box_idx = sys->box_lines_total - 2;
diff --git a/modules/gui/qt/components/playlist/playlist_model.cpp b/modules/gui/qt/components/playlist/playlist_model.cpp
index 94ee4f2f39..dd88306248 100644
--- a/modules/gui/qt/components/playlist/playlist_model.cpp
+++ b/modules/gui/qt/components/playlist/playlist_model.cpp
@@ -769,7 +769,7 @@ void PLModel::doDelete( QModelIndexList selected )
         playlist_item_t *p_root = playlist_ItemGetById( p_playlist,
                                                         item->id() );
         if( p_root != NULL )
-            playlist_NodeDelete( p_playlist, p_root, false );
+            playlist_NodeDelete( p_playlist, p_root );
         PL_UNLOCK;
 
         if( p_root != NULL )
diff --git a/modules/gui/skins2/vars/playtree.cpp b/modules/gui/skins2/vars/playtree.cpp
index 9ef679e06e..c0d2fda468 100644
--- a/modules/gui/skins2/vars/playtree.cpp
+++ b/modules/gui/skins2/vars/playtree.cpp
@@ -59,7 +59,7 @@ void Playtree::delSelected()
                 playlist_ItemGetById( m_pPlaylist, it->getId() );
             if( pItem )
             {
-                playlist_NodeDelete( m_pPlaylist, pItem, false );
+                playlist_NodeDelete( m_pPlaylist, pItem );
             }
             playlist_Unlock( m_pPlaylist );
 
diff --git a/modules/lua/libs/playlist.c b/modules/lua/libs/playlist.c
index d03c63e541..5cf879ea7a 100644
--- a/modules/lua/libs/playlist.c
+++ b/modules/lua/libs/playlist.c
@@ -144,7 +144,7 @@ static int vlclua_playlist_delete( lua_State * L )
     PL_LOCK;
     playlist_item_t *p_item = playlist_ItemGetById( p_playlist, i_id );
     if( p_item != NULL )
-       playlist_NodeDelete( p_playlist, p_item, false );
+       playlist_NodeDelete( p_playlist, p_item );
     PL_UNLOCK;
 
     return vlclua_push_ret( L, (p_item != NULL) ? 0 : -1 );
diff --git a/src/playlist/engine.c b/src/playlist/engine.c
index c94e6370c2..0e321dd290 100644
--- a/src/playlist/engine.c
+++ b/src/playlist/engine.c
@@ -329,8 +329,14 @@ void playlist_Destroy( playlist_t *p_playlist )
 
     /* Remove all remaining items */
     if( p_playlist->p_media_library != NULL )
-        playlist_NodeDelete( p_playlist, p_playlist->p_media_library, true );
-    playlist_NodeDelete( p_playlist, p_playlist->p_playing, true );
+    {
+        playlist_NodeDeleteExplicit( p_playlist, p_playlist->p_media_library,
+            PLAYLIST_DELETE_FORCE );
+    }
+
+    playlist_NodeDeleteExplicit( p_playlist, p_playlist->p_playing,
+        PLAYLIST_DELETE_FORCE );
+
     assert( p_playlist->root.i_children <= 0 );
     PL_UNLOCK;
 
diff --git a/src/playlist/item.c b/src/playlist/item.c
index 5be9bba118..bf0a39882b 100644
--- a/src/playlist/item.c
+++ b/src/playlist/item.c
@@ -112,7 +112,7 @@ static void input_item_add_subitem_tree ( const vlc_event_t * p_event,
         }
         assert( i < p_parent->i_children );
 
-        playlist_NodeDelete( p_playlist, p_item, false );
+        playlist_NodeDeleteExplicit( p_playlist, p_item, 0 );
 
         /* If there is a pending request referring to the item we just deleted
          * it needs to be updated so that we do not try to play an entity that
@@ -426,7 +426,7 @@ void playlist_Clear( playlist_t * p_playlist, bool b_locked )
     PL_LOCK_IF( !b_locked );
 
     for( int i = p_root->i_children - 1; i >= 0 ;i-- )
-        playlist_NodeDelete( p_playlist, p_root->pp_children[i], false );
+        playlist_NodeDelete( p_playlist, p_root->pp_children[i] );
 
     PL_UNLOCK_IF( !b_locked );
 }
diff --git a/src/playlist/playlist_internal.h b/src/playlist/playlist_internal.h
index b3d589aef3..83982d7da0 100644
--- a/src/playlist/playlist_internal.h
+++ b/src/playlist/playlist_internal.h
@@ -126,6 +126,27 @@ int playlist_InsertInputItemTree ( playlist_t *,
 /* Tree walking */
 int playlist_NodeInsert(playlist_item_t*, playlist_item_t *, int);
 
+/**
+ * Flags for playlist_NodeDeleteExplicit
+ * \defgroup playlist_NodeDeleteExplicit_flags
+ * @{
+ **/
+#define PLAYLIST_DELETE_FORCE 0x01 /**< delete node even if read-only */
+/** @} */
+
+/**
+ * Delete a node with explicit semantics
+ *
+ * This function acts like \ref playlist_NodeDelete with the advantage of the
+ * caller being able control some of the semantics of the function.
+ *
+ * \ref p_playlist the playlist where the node is to be deleted
+ * \ref p_node the node to delete
+ * \ref flags a bitfield consisting of \ref playlist_NodeDeleteExplicit_flags
+ **/
+void playlist_NodeDeleteExplicit(playlist_t*, playlist_item_t*,
+    int flags);
+
 void playlist_ItemRelease( playlist_t *, playlist_item_t * );
 
 void ResetCurrentlyPlaying( playlist_t *p_playlist, playlist_item_t *p_cur );
diff --git a/src/playlist/services_discovery.c b/src/playlist/services_discovery.c
index 1392afb29f..eb524a093a 100644
--- a/src/playlist/services_discovery.c
+++ b/src/playlist/services_discovery.c
@@ -103,7 +103,7 @@ static void playlist_sd_item_removed(services_discovery_t *sd,
        becomes empty, delete that node as well */
     if (node != sds->node && node->i_children == 1)
         item = node;
-    playlist_NodeDelete(p_playlist, item, true);
+    playlist_NodeDeleteExplicit(p_playlist, item, PLAYLIST_DELETE_FORCE );
     PL_UNLOCK;
 }
 
@@ -153,7 +153,7 @@ static void playlist_ServicesDiscoveryInternalRemove(playlist_t *playlist,
     /* Remove the sd playlist node if it exists */
     playlist_Lock(playlist);
     if (sds->node != NULL)
-        playlist_NodeDelete(playlist, sds->node, true);
+        playlist_NodeDeleteExplicit(playlist, sds->node, PLAYLIST_DELETE_FORCE );
     playlist_Unlock(playlist);
 
     free(sds);
diff --git a/src/playlist/tree.c b/src/playlist/tree.c
index 0dbcf70905..caaa9e133f 100644
--- a/src/playlist/tree.c
+++ b/src/playlist/tree.c
@@ -89,17 +89,23 @@ playlist_item_t * playlist_NodeCreate( playlist_t *p_playlist,
  * \param p_playlist the playlist
  * \param p_root the node
  */
-void playlist_NodeDelete( playlist_t *p_playlist, playlist_item_t *p_root,
-                          bool b_force )
+void playlist_NodeDelete( playlist_t *p_playlist, playlist_item_t *p_root )
+{
+    playlist_NodeDeleteExplicit( p_playlist, p_root, 0 );
+}
+
+void playlist_NodeDeleteExplicit( playlist_t *p_playlist,
+    playlist_item_t *p_root, int flags )
 {
     PL_ASSERT_LOCKED;
 
     /* Delete the children */
     for( int i = p_root->i_children - 1 ; i >= 0; i-- )
-        playlist_NodeDelete( p_playlist, p_root->pp_children[i], b_force );
+        playlist_NodeDeleteExplicit( p_playlist, p_root->pp_children[i], flags );
 
     /* Delete the node */
-    if( p_root->i_flags & PLAYLIST_RO_FLAG && !b_force )
+    if( p_root->i_flags & PLAYLIST_RO_FLAG &&
+        !( flags & PLAYLIST_DELETE_FORCE ) )
         return;
 
     pl_priv(p_playlist)->b_reset_currently_playing = true;
-- 
2.13.0


More information about the vlc-devel mailing list