[vlc-devel] [PATCH] gui/qt: fix crash when sorting playlist

[Rémi Denis-Courmont]

On perjantaina 8. heinäkuuta 2016 3.50.23 EEST Filip Roséen wrote:
> Fix for ticket #17023
> ---------------------
> 
>     - https://trac.videolan.org/vlc/ticket/17023
> 
> The problem behind the ticket in question is that the QModelIndex sent
> through "emit currentIndexChanged( ... )" previously referred to an
> AbstractPLItem that is destroyed during the playlist sorting.
> 
> This patch fixes the issue by grabbing the associated input_item_t*
> prior to sorting/destroying the item list, and then conditionally using
> the retrieved handle to construct a new QModelIndex (referring to the
> correct entity), referring to the correct entity within the
> recontrustructed list.

I´m not familiar with Qt playlist code, though I could easily miss 
something... but I don´t see where this patch "grabs" the input item. I mean, 
I don´t see where you take an actual (counted) reference to it.

It rather looks like you pick the input item ID. This has been deprecated for 
years because it´s obviously broken.

> 
> fix #17023
> ---
>  modules/gui/qt/components/playlist/playlist_model.cpp | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/modules/gui/qt/components/playlist/playlist_model.cpp
> b/modules/gui/qt/components/playlist/playlist_model.cpp index
> 1ee67a7..2df4722 100644
> --- a/modules/gui/qt/components/playlist/playlist_model.cpp
> +++ b/modules/gui/qt/components/playlist/playlist_model.cpp
> @@ -794,6 +794,10 @@ void PLModel::sort( QModelIndex caller, QModelIndex
> rootIndex, const int column,
>                                             : rootItem;
> 
>      if( !item ) return;
> 
> +    input_item_t* p_caller_item = caller.isValid()
> +        ? static_cast<AbstractPLItem*>( caller.internalPointer()
> )->inputItem() +        : NULL;
> +
>      int i_root_id = item->id( PLAYLIST_ID );
> 
>      QModelIndex qIndex = index( item, 0 );
> @@ -825,10 +829,16 @@ void PLModel::sort( QModelIndex caller, QModelIndex
> rootIndex, const int column, endInsertRows( );
>      }
>      PL_UNLOCK;
> +
>      /* if we have popup item, try to make sure that you keep that item
> visible */ -    if( caller.isValid() ) emit currentIndexChanged( caller );
> +    if( p_caller_item )
> +    {
> +        QModelIndex idx = indexByInputItemID( p_caller_item->i_id, 0 );
> 
> -    else if( currentIndex().isValid() ) emit currentIndexChanged(
> currentIndex() );
> +        emit currentIndexChanged( idx );
> +    }
> +    else if( currentIndex().isValid() )
> +        emit currentIndexChanged( currentIndex() );
>  }
> 
>  void PLModel::filter( const QString& search_text, const QModelIndex & idx,
> bool b_recursive )


-- 
Rémi Denis-Courmont
http://www.remlab.net/