[amarok] /: AudioCdCollection: don't create a reference out of temporary var (crashfix)

[Bart Cerneels]

The temporary QString returned by url() should remain valid until the
end of the local scope. At least that is what I think, but it's a
tricky one. I hope I'm proven wrong by the bug being fixed.

On Mon, May 7, 2012 at 11:42 PM, Matěj Laitl <matej at laitl.cz> wrote:
> Git commit a3d378de5aca583bbbc13d1966df258f27e7f174 by Matěj Laitl.
> Committed on 07/05/2012 at 23:31.
> Pushed by laitl into branch 'master'.
>
> AudioCdCollection: don't create a reference out of temporary var (crashfix)
>
> There's following code in AudioCdCollection::updateProxyTracks():
> const QString &urlString = url.url().remove( "audiocd:/" );
>
> Which I think is wrong, because url.url() returns a temporary QString
> and QString::remove() returns a reference to itself. However, the
> temporary is AFAICS destroyed as soon as this line ends. This IMO
> results in urlString being an invalid reference.
>
> I'm not really sure about this, I haven't read C++ spec, but given many
> users report crash on the following line, this could be the culprit.
> I was never able to reproduce the bug, so I'm shooting blindly.
>
> Reporters, please test reproducibility with current git and reopen if
> this is not fixed.
>
> BUG: 256585
> FIXED-IN: 2.6
> DIGEST: fix grave crash
>
> M  +1    -0    ChangeLog
> M  +1    -1    src/core-impl/collections/audiocd/AudioCdCollection.cpp
>
> http://commits.kde.org/amarok/a3d378de5aca583bbbc13d1966df258f27e7f174
>
> diff --git a/ChangeLog b/ChangeLog
> index 573c15c..5894bf3 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -84,6 +84,7 @@ VERSION 2.6-Beta 1
>       "1.2 GB free" is shown instead of "85% used"; thicker capacity bar.
>
>   BUGFIXES:
> +    * Fix crash on startup related to Audio CD collection. (BR 256585)
>     * When turning dynamic playlist on, immediately populate playlist and clear
>       any possible playlist sorting. (BR 220558)
>     * Fix transcoding with ffmpeg >= 0.10; patch by Julian Simioni.
> diff --git a/src/core-impl/collections/audiocd/AudioCdCollection.cpp b/src/core-impl/collections/audiocd/AudioCdCollection.cpp
> index 637c2d3..95c4e06 100644
> --- a/src/core-impl/collections/audiocd/AudioCdCollection.cpp
> +++ b/src/core-impl/collections/audiocd/AudioCdCollection.cpp
> @@ -575,7 +575,7 @@ AudioCdCollection::updateProxyTracks()
>     foreach( const KUrl &url, m_proxyMap.keys() )
>     {
>
> -        const QString &urlString = url.url().remove( "audiocd:/" );
> +        QString urlString = url.url().remove( "audiocd:/" );
>         const QStringList &parts = urlString.split( '/' );
>
>         if( parts.count() != 2 )