Merge pull request #270 from lorenzhs/misc-fixes

Miscellaneous cppcheck and other fixes

src/libtomahawk/Source.cpp

@@ -235,9 +235,6 @@ Source::friendlyNamesLessThan( const QString& first, const QString& second )
     favored.append( QRegExp( "\\b([A-Z][a-z']* ?){2,10}" ) ); //properly capitalized person's name
     favored.append( QRegExp( "[a-zA-Z ']+" ) ); //kind of person's name
 
-    bool matchFirst = false;
-    bool matchSecond = false;
-
     //We check if the strings match the regexps. The regexps represent friendly name patterns we do
     //*not* want (penalties) or want (favored), prioritized. If none of the strings match a regexp,
     //we go to the next regexp. If one of the strings matches, and we're matching penalties, we say
@@ -261,23 +258,23 @@ Source::friendlyNamesLessThan( const QString& first, const QString& second )
             isPenalty = false;
         }
 
-        matchFirst = rx.exactMatch( first );
+        const bool matchFirst = rx.exactMatch( first );
-        matchSecond = rx.exactMatch( second );
+        const bool matchSecond = rx.exactMatch( second );
 
-        if ( matchFirst == false && matchSecond == false )
+        if ( !matchFirst && !matchSecond )
             continue;
 
-        if ( matchFirst == true && matchSecond == true )
+        if ( matchFirst && matchSecond )
             break;
 
-        if ( matchFirst == true && matchSecond == false )
+        if ( matchFirst && !matchSecond )
-            return isPenalty ? false : true;
+            return !isPenalty;
 
-        if ( matchFirst == false && matchSecond == true )
+        if ( !matchFirst && matchSecond)
-            return isPenalty ? true : false;
+            return isPenalty;
     }
 
-    return ( first.compare( second ) == -1 ) ? true : false;
+    return first.compare( second ) == -1;
 }
 
 

src/libtomahawk/UrlHandler.cpp

@@ -95,7 +95,7 @@ localFileIODeviceFactory( const Tomahawk::result_ptr&, const QString& url,
         io->open( QIODevice::ReadOnly );
 
     // std::functions cannot accept temporaries as parameters
-    QSharedPointer< QIODevice > sp = QSharedPointer<QIODevice>( io );
+    QSharedPointer< QIODevice > sp( io );
     callback( url, sp );
 }
 

src/libtomahawk/playlist/AlbumModel.cpp

@@ -144,10 +144,9 @@ AlbumModel::addAlbums( const QList<Tomahawk::album_ptr>& albums )
 
     emit beginInsertRows( QModelIndex(), crows.first, crows.second );
 
-    PlayableItem* albumitem;
     foreach( const album_ptr& album, trimmedAlbums )
     {
-        albumitem = new PlayableItem( album, rootItem() );
+        PlayableItem* albumitem = new PlayableItem( album, rootItem() );
         albumitem->index = createIndex( rootItem()->children.count() - 1, 0, albumitem );
 
         connect( albumitem, SIGNAL( dataChanged() ), SLOT( onDataChanged() ) );
@@ -190,10 +189,9 @@ AlbumModel::addArtists( const QList<Tomahawk::artist_ptr>& artists )
 
     emit beginInsertRows( QModelIndex(), crows.first, crows.second );
 
-    PlayableItem* albumitem;
     foreach ( const artist_ptr& artist, trimmedArtists )
     {
-        albumitem = new PlayableItem( artist, rootItem() );
+        PlayableItem* albumitem = new PlayableItem( artist, rootItem() );
         albumitem->index = createIndex( rootItem()->children.count() - 1, 0, albumitem );
 
         connect( albumitem, SIGNAL( dataChanged() ), SLOT( onDataChanged() ) );
@@ -219,10 +217,9 @@ AlbumModel::addQueries( const QList<Tomahawk::query_ptr>& queries )
 
     emit beginInsertRows( QModelIndex(), crows.first, crows.second );
 
-    PlayableItem* albumitem;
     foreach ( const query_ptr& query, queries )
     {
-        albumitem = new PlayableItem( query, rootItem() );
+        PlayableItem* albumitem = new PlayableItem( query, rootItem() );
         albumitem->index = createIndex( rootItem()->children.count() - 1, 0, albumitem );
 
         connect( albumitem, SIGNAL( dataChanged() ), SLOT( onDataChanged() ) );

src/libtomahawk/playlist/PlayableModel.cpp

@@ -679,11 +679,10 @@ PlayableModel::insertInternal( const QList< T >& items, int row, const QList< To
     emit beginInsertRows( parent, crows.first, crows.second );
 
     int i = 0;
-    PlayableItem* plitem;
     foreach ( const T& item, items )
     {
         PlayableItem* pItem = itemFromIndex( parent );
-        plitem = new PlayableItem( item, pItem, row + i );
+        PlayableItem* plitem = new PlayableItem( item, pItem, row + i );
         plitem->index = createIndex( row + i, 0, plitem );
 
         if ( plitem->query() )

src/libtomahawk/playlist/PlaylistModel.cpp

@@ -290,11 +290,10 @@ PlaylistModel::insertEntries( const QList< Tomahawk::plentry_ptr >& entries, int
 
     QList< Tomahawk::query_ptr > queries;
     int i = 0;
-    PlayableItem* plitem;
     foreach( const plentry_ptr& entry, entries )
     {
         PlayableItem* pItem = itemFromIndex( parent );
-        plitem = new PlayableItem( entry, pItem, row + i );
+        PlayableItem* plitem = new PlayableItem( entry, pItem, row + i );
         plitem->index = createIndex( row + i, 0, plitem );
 
         if ( logs.count() > i )

src/libtomahawk/playlist/TreeModel.cpp

@@ -193,10 +193,9 @@ TreeModel::addAlbums( const QModelIndex& parent, const QList<Tomahawk::album_ptr
 
     emit beginInsertRows( parent, crows.first, crows.second );
 
-    PlayableItem* albumitem = 0;
     foreach( const album_ptr& album, albums )
     {
-        albumitem = new PlayableItem( album, parentItem );
+        PlayableItem* albumitem = new PlayableItem( album, parentItem );
         albumitem->index = createIndex( parentItem->children.count() - 1, 0, albumitem );
         connect( albumitem, SIGNAL( dataChanged() ), SLOT( onDataChanged() ) );
 
@@ -283,10 +282,9 @@ TreeModel::onArtistsAdded( const QList<Tomahawk::artist_ptr>& artists )
 
     emit beginInsertRows( QModelIndex(), crows.first, crows.second );
 
-    PlayableItem* artistitem;
     foreach( const artist_ptr& artist, artists )
     {
-        artistitem = new PlayableItem( artist, rootItem() );
+        PlayableItem* artistitem = new PlayableItem( artist, rootItem() );
         artistitem->index = createIndex( rootItem()->children.count() - 1, 0, artistitem );
         connect( artistitem, SIGNAL( dataChanged() ), SLOT( onDataChanged() ) );
     }
@@ -312,10 +310,9 @@ TreeModel::onTracksAdded( const QList<Tomahawk::query_ptr>& tracks, const QModel
 
     emit beginInsertRows( parent, crows.first, crows.second );
 
-    PlayableItem* item = 0;
     foreach( const query_ptr& query, tracks )
     {
-        item = new PlayableItem( query, parentItem );
+        PlayableItem* item = new PlayableItem( query, parentItem );
         item->index = createIndex( parentItem->children.count() - 1, 0, item );
 
         connect( item, SIGNAL( dataChanged() ), SLOT( onDataChanged() ) );

src/libtomahawk/resolvers/JSResolver.cpp

@@ -265,11 +265,9 @@ JSResolver::init()
     d->timeout = m.value( "timeout", 25 ).toUInt() * 1000;
     bool compressed = m.value( "compressed", "false" ).toString() == "true";
 
-    QByteArray icoData = m.value( "icon" ).toByteArray();
+    QByteArray icoData = QByteArray::fromBase64( m.value( "icon" ).toByteArray() );
     if ( compressed )
-        icoData = qUncompress( QByteArray::fromBase64( icoData ) );
+        icoData = qUncompress( icoData );
-    else
-        icoData = QByteArray::fromBase64( icoData );
     QPixmap ico;
     ico.loadFromData( icoData );
 
@@ -554,7 +552,7 @@ JSResolver::parseResultVariantList( const QVariantList& reslist )
         if ( m.value( "preview" ).toBool() == true )
             continue;
 
-        unsigned int duration = m.value( "duration", 0 ).toUInt();
+        int duration = m.value( "duration", 0 ).toInt();
         if ( duration <= 0 && m.contains( "durationString" ) )
         {
             QTime time = QTime::fromString( m.value( "durationString" ).toString(), "hh:mm:ss" );
@@ -614,10 +612,11 @@ JSResolver::parseArtistVariantList( const QVariantList& reslist )
 
     foreach( const QVariant& rv, reslist )
     {
-        if ( rv.toString().trimmed().isEmpty() )
+        const QString val = rv.toString();
+        if ( val.trimmed().isEmpty() )
             continue;
 
-        Tomahawk::artist_ptr ap = Tomahawk::Artist::get( rv.toString(), false );
+        Tomahawk::artist_ptr ap = Tomahawk::Artist::get( val, false );
 
         results << ap;
     }
@@ -633,10 +632,11 @@ JSResolver::parseAlbumVariantList( const Tomahawk::artist_ptr& artist, const QVa
 
     foreach( const QVariant& rv, reslist )
     {
-        if ( rv.toString().trimmed().isEmpty() )
+        const QString val = rv.toString();
+        if ( val.trimmed().isEmpty() )
             continue;
 
-        Tomahawk::album_ptr ap = Tomahawk::Album::get( artist, rv.toString(), false );
+        Tomahawk::album_ptr ap = Tomahawk::Album::get( artist, val, false );
 
         results << ap;
     }
@@ -673,18 +673,16 @@ JSResolver::loadUi()
     bool compressed = m.value( "compressed", "false" ).toBool();
     qDebug() << "Resolver has a preferences widget! compressed?" << compressed;
 
-    QByteArray uiData = m[ "widget" ].toByteArray();
+    QByteArray uiData = QByteArray::fromBase64( m[ "widget" ].toByteArray() );
-
     if ( compressed )
-        uiData = qUncompress( QByteArray::fromBase64( uiData ) );
+        uiData = qUncompress(  uiData );
-    else
-        uiData = QByteArray::fromBase64( uiData );
 
     QVariantMap images;
     foreach(const QVariant& item, m[ "images" ].toList())
     {
-        QString key = item.toMap().keys().first();
+        const QVariantMap m = item.toMap();
-        QVariant value = item.toMap().value(key);
+        QString key = m.keys().first();
+        QVariant value = m.value(key);
         images[key] = value;
     }
 
@@ -740,11 +738,13 @@ JSResolver::widgetData( QWidget* widget, const QString& property )
 void
 JSResolver::setWidgetData( const QVariant& value, QWidget* widget, const QString& property )
 {
-    for ( int i = 0; i < widget->metaObject()->propertyCount(); i++ )
+    const QMetaObject *metaObject = widget->metaObject();
+    for ( int i = 0; i < metaObject->propertyCount(); i++ )
     {
-        if ( widget->metaObject()->property( i ).name() == property )
+        const QMetaProperty &prop = metaObject->property( i );
+        if ( prop.name() == property )
         {
-            widget->metaObject()->property( i ).write( widget, value );
+            prop.write( widget, value );
             return;
         }
     }
@@ -780,7 +780,8 @@ JSResolver::fillDataInWidgets( const QVariantMap& data )
 
     foreach(const QVariant& dataWidget, d->dataWidgets)
     {
-        QString widgetName = dataWidget.toMap()["widget"].toString();
+        const QVariantMap m = dataWidget.toMap();
+        QString widgetName = m["widget"].toString();
         QWidget* widget= d->configWidget.data()->findChild<QWidget*>( widgetName );
         if ( !widget )
         {
@@ -789,8 +790,8 @@ JSResolver::fillDataInWidgets( const QVariantMap& data )
             return;
         }
 
-        QString propertyName = dataWidget.toMap()["property"].toString();
+        QString propertyName = m["property"].toString();
-        QString name = dataWidget.toMap()["name"].toString();
+        QString name = m["name"].toString();
 
         setWidgetData( data[ name ], widget, propertyName );
     }
@@ -814,14 +815,14 @@ JSResolver::loadCollections()
 
     if ( d->capabilities.testFlag( Browsable ) )
     {
-        QVariantMap collectionInfo = d->engine->mainFrame()->evaluateJavaScript( "Tomahawk.resolver.instance.collection();" ).toMap();
+        const QVariantMap collectionInfo = d->engine->mainFrame()->evaluateJavaScript( "Tomahawk.resolver.instance.collection();" ).toMap();
         if ( collectionInfo.isEmpty() ||
              !collectionInfo.contains( "prettyname" ) ||
              !collectionInfo.contains( "description" ) )
             return;
 
-        QString prettyname = collectionInfo.value( "prettyname" ).toString();
+        const QString prettyname = collectionInfo.value( "prettyname" ).toString();
-        QString desc = collectionInfo.value( "description" ).toString();
+        const QString desc = collectionInfo.value( "description" ).toString();
 
         foreach ( Tomahawk::collection_ptr collection, m_collections )
         {
@@ -844,12 +845,11 @@ JSResolver::loadCollections()
 
         if ( collectionInfo.contains( "iconfile" ) )
         {
-            bool ok = false;
             QString iconPath = QFileInfo( filePath() ).path() + "/"
                                + collectionInfo.value( "iconfile" ).toString();
 
             QPixmap iconPixmap;
-            ok = iconPixmap.load( iconPath );
+            bool ok = iconPixmap.load( iconPath );
             if ( ok && !iconPixmap.isNull() )
                 sc->setIcon( QIcon( iconPixmap ) );
         }

src/libtomahawk/resolvers/JSResolverHelper.cpp

@@ -319,7 +319,7 @@ JSResolverHelper::addUrlResult( const QString& url, const QVariantMap& result )
         QString guid = result.value( "guid" ).toString();
         Q_ASSERT( !guid.isEmpty() );
         // Append nodeid to guid to make it globally unique.
-        guid += Tomahawk::Database::instance()->impl()->dbid();
+        guid += instanceUUID();
 
         // Do we already have this playlist loaded?
         {
@@ -354,7 +354,7 @@ JSResolverHelper::addUrlResult( const QString& url, const QVariantMap& result )
     {
         QString xspfUrl = result.value( "url" ).toString();
         Q_ASSERT( !xspfUrl.isEmpty() );
-        QString guid = QString( "xspf-%1-%2" ).arg( xspfUrl.toUtf8().toBase64().constData() ).arg( Tomahawk::Database::instance()->impl()->dbid() );
+        QString guid = QString( "xspf-%1-%2" ).arg( xspfUrl.toUtf8().toBase64().constData() ).arg( instanceUUID() );
 
         // Do we already have this playlist loaded?
         {
@@ -388,7 +388,7 @@ JSResolverHelper::addUrlResult( const QString& url, const QVariantMap& result )
 void
 JSResolverHelper::reportCapabilities( const QVariant& v )
 {
-    bool ok = 0;
+    bool ok;
     int intCap = v.toInt( &ok );
     Tomahawk::ExternalResolver::Capabilities capabilities;
     if ( !ok )
@@ -429,7 +429,7 @@ JSResolverHelper::setResolverConfig( const QVariantMap& config )
 
 
 QString
-JSResolverHelper::acountId()
+JSResolverHelper::accountId()
 {
     return m_resolver->d_func()->accountId;
 }
@@ -440,7 +440,7 @@ JSResolverHelper::addCustomUrlHandler( const QString& protocol,
                                              const QString& callbackFuncName,
                                              const QString& isAsynchronous )
 {
-    m_urlCallbackIsAsync = ( isAsynchronous.toLower() == "true" ) ? true : false;
+    m_urlCallbackIsAsync = ( isAsynchronous.toLower() == "true" );
 
     std::function< void( const Tomahawk::result_ptr&, const QString&,
                            std::function< void( const QString&, QSharedPointer< QIODevice >& ) > )> fac =
@@ -765,13 +765,13 @@ JSResolverHelper::indexDataFromVariant( const QVariantMap &map, struct Tomahawk:
 void
 JSResolverHelper::createFuzzyIndex( const QVariantList& list )
 {
-    if ( m_resolver->d_func()->fuzzyIndex.isNull() )
+    if ( hasFuzzyIndex() )
     {
-        m_resolver->d_func()->fuzzyIndex.reset( new FuzzyIndex( m_resolver, m_resolver->d_func()->accountId + ".lucene" , true ) );
+        m_resolver->d_func()->fuzzyIndex->wipeIndex();
     }
     else
     {
-        m_resolver->d_func()->fuzzyIndex->wipeIndex();
+        m_resolver->d_func()->fuzzyIndex.reset( new FuzzyIndex( m_resolver, accountId() + ".lucene" , true ) );
     }
 
     addToFuzzyIndex( list );
@@ -781,7 +781,7 @@ JSResolverHelper::createFuzzyIndex( const QVariantList& list )
 void
 JSResolverHelper::addToFuzzyIndex( const QVariantList& list )
 {
-    if ( m_resolver->d_func()->fuzzyIndex.isNull() )
+    if ( !hasFuzzyIndex() )
     {
         tLog() << Q_FUNC_INFO << "Cannot add entries to non-existing index.";
         return;
@@ -810,7 +810,7 @@ JSResolverHelper::addToFuzzyIndex( const QVariantList& list )
 
 
 bool
-cmpTuple ( QVariant x, QVariant y )
+cmpTuple ( QVariant& x, QVariant& y )
 {
     return x.toList().at( 1 ).toFloat() < y.toList().at( 1 ).toFloat();
 }
@@ -903,7 +903,7 @@ void
 JSResolverHelper::gotStreamUrl( std::function< void( const QString&, QSharedPointer< QIODevice >& ) > callback, NetworkReply* reply )
 {
     // std::functions cannot accept temporaries as parameters
-    QSharedPointer< QIODevice > sp = QSharedPointer< QIODevice >( reply->reply(), &QObject::deleteLater );
+    QSharedPointer< QIODevice > sp ( reply->reply(), &QObject::deleteLater );
     QString url = reply->reply()->url().toString();
     reply->disconnectFromReply();
     reply->deleteLater();

src/libtomahawk/resolvers/JSResolverHelper.h

@@ -54,7 +54,7 @@ public:
      *
      * INTERNAL USE ONLY!
      */
-    Q_INVOKABLE QString acountId();
+    Q_INVOKABLE QString accountId();
 
     Q_INVOKABLE void addCustomUrlHandler( const QString& protocol, const QString& callbackFuncName, const QString& isAsynchronous = "false" );
     Q_INVOKABLE void reportStreamUrl( const QString& qid, const QString& streamUrl );

src/libtomahawk/resolvers/ScriptEngine.cpp

@@ -74,7 +74,6 @@ ScriptEngine::sslErrorHandler( QNetworkReply* qnr, const QList<QSslError>& errli
     tDebug() << Q_FUNC_INFO;
 
     QByteArray digest = errlist.first().certificate().digest();
-    int result = -1;
 
     if ( !TomahawkSettings::instance()->isSslCertKnown( digest ) )
     {
@@ -92,7 +91,7 @@ ScriptEngine::sslErrorHandler( QNetworkReply* qnr, const QList<QSslError>& errli
         question.setStandardButtons( QMessageBox::No );
         question.addButton( tr( "Trust certificate" ), QMessageBox::AcceptRole );
 
-        result = question.exec();
+        int result = question.exec();
 
         //FIXME: discuss whether we want to store rejects, too (needs settings management to remove the decision?)
         if ( result == QMessageBox::AcceptRole )

src/libtomahawk/resolvers/ScriptResolver.cpp

@@ -402,7 +402,7 @@ ScriptResolver::doSetup( const QVariantMap& m )
     m_timeout = m.value( "timeout", 5 ).toUInt() * 1000;
     bool compressed = m.value( "compressed", "false" ).toString() == "true";
 
-    bool ok = 0;
+    bool ok;
     int intCap = m.value( "capabilities" ).toInt( &ok );
     if ( !ok )
         m_capabilities = NullCapability;