Magellan Linux

Contents of /trunk/amarok/patches/amarok-2.2.2-kde220532.patch

Parent Directory Parent Directory | Revision Log Revision Log


Revision 987 - (show annotations) (download)
Fri Feb 19 16:00:40 2010 UTC (14 years, 2 months ago) by niro
File size: 16478 byte(s)
amarok 2.2.2 fixes

1 Fixes potential crashes during scanning discovered in 2.2.2 after tagging.
2 Jeff Mitchell <mitchell@kde.org>
3
4
5 diff --git a/src/collection/sqlcollection/ScanResultProcessor.cpp b/src/collection/sqlcollection/ScanResultProcessor.cpp
6 index b680623..68b31df 100644
7 --- a/src/collection/sqlcollection/ScanResultProcessor.cpp
8 +++ b/src/collection/sqlcollection/ScanResultProcessor.cpp
9 @@ -42,19 +42,59 @@ ScanResultProcessor::ScanResultProcessor( SqlCollection *collection )
10 ScanResultProcessor::~ScanResultProcessor()
11 {
12 //everything has a URL, so enough to just delete from here
13 + QSet<QStringList*> currSet; //prevent double deletes
14 foreach( QStringList *list, m_urlsHashByUid )
15 - delete list;
16 + {
17 + if( list )
18 + {
19 + if( !currSet.contains( list ) )
20 + {
21 + delete list;
22 + currSet.insert( list );
23 + }
24 + }
25 + else
26 + debug() << "GAAH! Tried to double-delete a value in m_urlsHashByUid";
27 + }
28 foreach( QLinkedList<QStringList*> *list, m_albumsHashByName )
29 {
30 - foreach( QStringList *slist, *list )
31 - delete slist;
32 - delete list;
33 + if( list )
34 + {
35 + foreach( QStringList *slist, *list )
36 + {
37 + if( slist )
38 + {
39 + if( !currSet.contains( slist ) )
40 + {
41 + delete slist;
42 + currSet.insert( slist );
43 + }
44 + else
45 + debug() << "GAAH! Tried to double-delete a value in m_albumsHashByName";
46 + }
47 + }
48 + delete list;
49 + }
50 }
51 foreach( QLinkedList<QStringList*> *list, m_tracksHashByAlbum )
52 {
53 - foreach( QStringList *slist, *list )
54 - delete slist;
55 - delete list;
56 + if( list )
57 + {
58 + foreach( QStringList *slist, *list )
59 + {
60 + if( slist )
61 + {
62 + if( !currSet.contains( slist ) )
63 + {
64 + delete slist;
65 + currSet.insert( slist );
66 + }
67 + else
68 + debug() << "GAAH! Tried to double-delete a value in m_tracksHashByAlbum";
69 + }
70 + }
71 + delete list;
72 + }
73 }
74 }
75
76 @@ -67,11 +107,11 @@ ScanResultProcessor::setScanType( ScanType type )
77 void
78 ScanResultProcessor::addDirectory( const QString &dir, uint mtime )
79 {
80 - DEBUG_BLOCK
81 - debug() << "SRP::addDirectory on " << dir << " with mtime " << mtime;
82 + //DEBUG_BLOCK
83 + //debug() << "SRP::addDirectory on " << dir << " with mtime " << mtime;
84 if( dir.isEmpty() )
85 {
86 - debug() << "got directory with no path from the scanner, not adding";
87 + //debug() << "got directory with no path from the scanner, not adding";
88 return;
89 }
90 setupDatabase();
91 @@ -254,7 +294,7 @@ ScanResultProcessor::rollback()
92 void
93 ScanResultProcessor::processDirectory( const QList<QVariantMap > &data )
94 {
95 -// DEBUG_BLOCK
96 + //DEBUG_BLOCK
97 setupDatabase();
98 //using the following heuristics:
99 //if more than one album is in the dir, use the artist of each track as albumartist
100 @@ -274,24 +314,54 @@ ScanResultProcessor::processDirectory( const QList<QVariantMap > &data )
101 if( row.value( Field::ALBUM ).toString() != album )
102 multipleAlbums = true;
103 }
104 +
105 if( multipleAlbums || album.isEmpty() || artists.size() == 1 )
106 {
107 foreach( const QVariantMap &row, data )
108 {
109 - int artist = genericId( &m_artists, row.value( Field::ARTIST ).toString(), &m_nextArtistNum );
110 - addTrack( row, artist );
111 + QString uid = row.value( Field::UNIQUEID ).toString();
112 + if( m_uidsSeenThisScan.contains( uid ) )
113 + {
114 + QString originalLocation = ( ( m_urlsHashByUid.contains( uid ) &&
115 + m_urlsHashByUid[uid] != 0 ) ?
116 + MountPointManager::instance()->getAbsolutePath( m_urlsHashByUid[uid]->at( 1 ).toInt(), m_urlsHashByUid[uid]->at( 2 ) ) : "(unknown)" );
117 + debug() << "Skipping file with uniqueid " << uid << " as it was already seen this scan," <<
118 + "file is at " << row.value( Field::URL ).toString() << ", original file is at " << originalLocation;
119 + }
120 + else
121 + {
122 + int artist = genericId( &m_artists, row.value( Field::ARTIST ).toString(), &m_nextArtistNum );
123 + //debug() << "artist found = " << artist;
124 + addTrack( row, artist );
125 + m_uidsSeenThisScan.insert( uid );
126 + }
127 }
128 }
129 else
130 {
131 QString albumArtist = findAlbumArtist( artists, data.count() );
132 + //debug() << "albumArtist found = " << albumArtist;
133 //an empty string means that no albumartist was found
134 int artist = albumArtist.isEmpty() ? 0 : genericId( &m_artists, albumArtist, &m_nextArtistNum );
135 + //debug() << "artist found = " << artist;
136
137 //debug() << "albumartist " << albumArtist << "for artists" << artists;
138 foreach( const QVariantMap &row, data )
139 {
140 - addTrack( row, artist );
141 + QString uid = row.value( Field::UNIQUEID ).toString();
142 + if( m_uidsSeenThisScan.contains( uid ) )
143 + {
144 + QString originalLocation = ( ( m_urlsHashByUid.contains( uid ) &&
145 + m_urlsHashByUid[uid] != 0 ) ?
146 + MountPointManager::instance()->getAbsolutePath( m_urlsHashByUid[uid]->at( 1 ).toInt(), m_urlsHashByUid[uid]->at( 2 ) ) : "(unknown)" );
147 + debug() << "Skipping file with uniqueid " << uid << " as it was already seen this scan," <<
148 + "file is at " << row.value( Field::URL ).toString() << ", original file is at " << originalLocation;
149 + }
150 + else
151 + {
152 + addTrack( row, artist );
153 + m_uidsSeenThisScan.insert( uid );
154 + }
155 }
156 }
157 }
158 @@ -299,6 +369,7 @@ ScanResultProcessor::processDirectory( const QList<QVariantMap > &data )
159 QString
160 ScanResultProcessor::findAlbumArtist( const QSet<QString> &artists, int trackCount ) const
161 {
162 + //DEBUG_BLOCK
163 QMap<QString, int> artistCount;
164 bool featuring;
165 QStringList trackArtists;
166 @@ -371,6 +442,7 @@ void
167 ScanResultProcessor::addTrack( const QVariantMap &trackData, int albumArtistId )
168 {
169 //DEBUG_BLOCK
170 + //debug() << "albumArtistId = " << albumArtistId;
171 //amarok 1 stored all tracks of a compilation in different directories.
172 //when using its "Organize Collection" feature
173 //try to detect these cases
174 @@ -419,7 +491,15 @@ ScanResultProcessor::addTrack( const QVariantMap &trackData, int albumArtistId )
175
176 //urlId will take care of the urls table part of AFT
177 int url = urlId( path, uid );
178 -
179 +/*
180 + foreach( QString key, m_urlsHashByUid.keys() )
181 + debug() << "Key: " << key << ", list: " << *m_urlsHashByUid[key];
182 + foreach( int key, m_urlsHashById.keys() )
183 + debug() << "Key: " << key << ", list: " << *m_urlsHashById[key];
184 + typedef QPair<int, QString> blahType; //QFOREACH is stupid when it comes to QPairs
185 + foreach( blahType key, m_urlsHashByLocation.keys() )
186 + debug() << "Key: " << key << ", list: " << *m_urlsHashByLocation[key];
187 +*/
188 QStringList *trackList = new QStringList();
189 int id = m_nextTrackNum;
190 //debug() << "Appending new track number with tracknum: " << id;
191 @@ -470,7 +550,7 @@ ScanResultProcessor::addTrack( const QVariantMap &trackData, int albumArtistId )
192 //insert into hashes
193 if( m_tracksHashByUrl.contains( url ) && m_tracksHashByUrl[url] != 0 )
194 {
195 - //debug() << "m_tracksHashByUrl contains the url!";
196 + //debug() << "m_tracksHashByUrl already contains url " << url;
197 //need to replace, not overwrite/add a new one
198 QStringList *oldValues = m_tracksHashByUrl[url];
199 QString oldId = oldValues->at( 0 );
200 @@ -490,8 +570,24 @@ ScanResultProcessor::addTrack( const QVariantMap &trackData, int albumArtistId )
201 m_tracksHashById.insert( id, trackList );
202 }
203
204 + //debug() << "album = " << album;
205 +
206 if( m_tracksHashByAlbum.contains( album ) && m_tracksHashByAlbum[album] != 0 )
207 - m_tracksHashByAlbum[album]->append( trackList );
208 + {
209 + //contains isn't the fastest on linked lists, but in reality this is on the order of maybe
210 + //ten quick pointer comparisons per track on average...probably lower
211 + //debug() << "trackList is " << trackList;
212 + if( !m_tracksHashByAlbum[album]->contains( trackList ) )
213 + {
214 + //debug() << "appending trackList to m_tracksHashByAlbum";
215 + m_tracksHashByAlbum[album]->append( trackList );
216 + }
217 + else
218 + {
219 + //debug() << "not appending trackList to m_tracksHashByAlbum";
220 + }
221 +
222 + }
223 else
224 {
225 QLinkedList<QStringList*> *list = new QLinkedList<QStringList*>();
226 @@ -595,6 +691,8 @@ ScanResultProcessor::albumId( const QString &album, int albumArtistId )
227 QLinkedList<QStringList*> *list = m_albumsHashByName[album];
228 foreach( QStringList *slist, *list )
229 {
230 + //debug() << "albumArtistId = " << albumArtistId;
231 + //debug() << "Checking list: " << *slist;
232 if( slist->at( 2 ).isEmpty() && albumArtistId == 0 )
233 {
234 //debug() << "artist is empty and albumArtistId = 0, returning " << slist->at( 0 );
235 @@ -631,7 +729,10 @@ ScanResultProcessor::albumInsert( const QString &album, int albumArtistId )
236 albumList->append( QString() );
237 m_albumsHashById[returnedNum] = albumList;
238 if( m_albumsHashByName.contains( album ) && m_albumsHashByName[album] != 0 )
239 - m_albumsHashByName[album]->append( albumList );
240 + {
241 + if( !m_albumsHashByName[album]->contains( albumList ) )
242 + m_albumsHashByName[album]->append( albumList );
243 + }
244 else
245 {
246 QLinkedList<QStringList*> *list = new QLinkedList<QStringList*>();
247 @@ -645,7 +746,7 @@ ScanResultProcessor::albumInsert( const QString &album, int albumArtistId )
248 int
249 ScanResultProcessor::urlId( const QString &url, const QString &uid )
250 {
251 - /*
252 +/*
253 DEBUG_BLOCK
254 foreach( QString key, m_urlsHashByUid.keys() )
255 debug() << "Key: " << key << ", list: " << *m_urlsHashByUid[key];
256 @@ -654,8 +755,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
257 typedef QPair<int, QString> blahType; //QFOREACH is stupid when it comes to QPairs
258 foreach( blahType key, m_urlsHashByLocation.keys() )
259 debug() << "Key: " << key << ", list: " << *m_urlsHashByLocation[key];
260 - */
261 -
262 +*/
263 QFileInfo fileInfo( url );
264 const QString dir = fileInfo.absoluteDir().absolutePath();
265 int dirId = directoryId( dir );
266 @@ -665,6 +765,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
267 QPair<int, QString> locationPair( deviceId, rpath );
268 //debug() << "in urlId with url = " << url << " and uid = " << uid;
269 //debug() << "checking locationPair " << locationPair;
270 +/*
271 if( m_urlsHashByLocation.contains( locationPair ) )
272 {
273 QStringList values;
274 @@ -674,6 +775,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
275 values << "zero";
276 //debug() << "m_urlsHashByLocation contains it! It is " << values;
277 }
278 +*/
279 QStringList currUrlIdValues;
280 if( m_urlsHashByUid.contains( uid ) && m_urlsHashByUid[uid] != 0 )
281 currUrlIdValues = *m_urlsHashByUid[uid];
282 @@ -717,6 +819,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
283 //debug() << "m_urlsHashByUid contains this UID, updating deviceId and path";
284 QStringList *list = m_urlsHashByUid[uid];
285 //debug() << "list from UID hash is " << list << " with values " << *list;
286 + QPair<int, QString> oldLocationPair( list->at( 1 ).toInt(), list->at( 2 ) );
287 list->replace( 1, QString::number( deviceId ) );
288 list->replace( 2, rpath );
289 list->replace( 3, QString::number( dirId ) );
290 @@ -737,6 +840,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
291 delete oldList;
292 }
293 m_urlsHashByLocation[locationPair] = list;
294 + m_urlsHashByLocation.remove( oldLocationPair );
295 }
296 m_permanentTablesUrlUpdates.insert( uid, url );
297 m_changedUrls.insert( uid, QPair<QString, QString>( MountPointManager::instance()->getAbsolutePath( currUrlIdValues[1].toInt(), currUrlIdValues[2] ), url ) );
298 @@ -751,6 +855,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
299 {
300 QStringList *list = m_urlsHashByLocation[locationPair];
301 //debug() << "Replacing hash " << list->at( 4 ) << " with " << uid;
302 + QString oldId = list->at( 4 );
303 list->replace( 4, uid );
304 if( m_urlsHashByUid.contains( uid )
305 && m_urlsHashByUid[uid] != 0
306 @@ -762,6 +867,7 @@ ScanResultProcessor::urlId( const QString &url, const QString &uid )
307 delete oldList;
308 }
309 m_urlsHashByUid[uid] = list;
310 + m_urlsHashByUid.remove( oldId );
311 }
312 m_permanentTablesUidUpdates.insert( url, uid );
313 m_changedUids.insert( currUrlIdValues[4], uid );
314 @@ -855,7 +961,8 @@ ScanResultProcessor::directoryId( const QString &dir )
315 int
316 ScanResultProcessor::checkExistingAlbums( const QString &album )
317 {
318 -// DEBUG_BLOCK
319 + //DEBUG_BLOCK
320 + //debug() << "looking for album " << album;
321 // "Unknown" albums shouldn't be handled as compilations
322 if( album.isEmpty() )
323 return 0;
324 @@ -865,7 +972,10 @@ ScanResultProcessor::checkExistingAlbums( const QString &album )
325 //it's probably a compilation.
326 //this handles A1 compilations that were automatically organized by Amarok
327 if( !m_albumsHashByName.contains( album ) || m_albumsHashByName[album] == 0 )
328 + {
329 + //debug() << "hashByName doesn't contain album, or it's zero";
330 return 0;
331 + }
332
333 QStringList trackIds;
334 QLinkedList<QStringList*> *llist = m_albumsHashByName[album];
335 @@ -915,8 +1025,10 @@ ScanResultProcessor::checkExistingAlbums( const QString &album )
336 }
337 }
338
339 + //debug() << "trackIds = " << trackIds;
340 if( trackIds.isEmpty() )
341 {
342 + //debug() << "trackIds empty, returning zero";
343 return 0;
344 }
345 else
346 @@ -933,6 +1045,7 @@ ScanResultProcessor::checkExistingAlbums( const QString &album )
347 list->replace( 3, compilationString );
348 }
349 }
350 + //debug() << "returning " << compilationId;
351 return compilationId;
352 }
353 }
354 @@ -1167,6 +1280,17 @@ ScanResultProcessor::copyHashesToTempTables()
355 foreach( blahType key, m_urlsHashByLocation.keys() )
356 debug() << "Key: " << key << ", list: " << *m_urlsHashByLocation[key];
357 debug() << "Next album num: " << m_nextAlbumNum;
358 +
359 + foreach( int key, m_tracksHashById.keys() )
360 + debug() << "Key: " << key << ", list: " << *m_tracksHashById[key];
361 + foreach( int key, m_tracksHashByUrl.keys() )
362 + debug() << "Key: " << key << ", list: " << *m_tracksHashByUrl[key];
363 + foreach( int key, m_tracksHashByAlbum.keys() )
364 + {
365 + debug() << "Key: " << key;
366 + foreach( QStringList* item, *m_tracksHashByAlbum[key] )
367 + debug() << "list: " << item << " is " << *item;
368 + }
369 */
370
371 DEBUG_BLOCK
372 diff --git a/src/collection/sqlcollection/ScanResultProcessor.h b/src/collection/sqlcollection/ScanResultProcessor.h
373 index 71afc98..c7f200f 100644
374 --- a/src/collection/sqlcollection/ScanResultProcessor.h
375 +++ b/src/collection/sqlcollection/ScanResultProcessor.h
376 @@ -94,6 +94,7 @@ class ScanResultProcessor : public QObject
377 QMap<QString, int> m_directories;
378 QMap<QString, QList< QPair< QString, QString > > > m_imageMap;
379
380 + QSet<QString> m_uidsSeenThisScan;
381 QHash<QString, uint> m_filesInDirs;
382
383 TrackUrls m_changedUids; //not really track urls