Differential D11529 Diff 40346 src/file/indexcleaner.cpp

Changeset View

Standalone View

View Options

src/file/indexcleaner.cpp

Show All 13 Lines
14 * You should have received a copy of the GNU Lesser General Public 14​ * You should have received a copy of the GNU Lesser General Public
15 * License along with this library; if not, write to the Free Software 15​ * License along with this library; if not, write to the Free Software
16 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA 16​ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
17 * 17​ *
18 */ 18​ */
19 19​
20#include "indexcleaner.h" 20​#include "indexcleaner.h"
21#include "fileindexerconfig.h" 21​#include "fileindexerconfig.h"
22​#include "storagedevices.h"
22 23​
23#include "database.h" 24​#include "database.h"
24#include "transaction.h" 25​#include "transaction.h"
25#include "idutils.h" 26​#include "idutils.h"
26 27​
27#include <QDebug> 28​#include <QDebug>
28#include <QFile> 29​#include <QFile>
29#include <QMimeDatabase> 30​#include <QMimeDatabase>
30 31​
31using namespace Baloo; 32​using namespace Baloo;
32 33​
33IndexCleaner::IndexCleaner(Database* db, FileIndexerConfig* config) 34​IndexCleaner::IndexCleaner(Database* db, FileIndexerConfig* config)
34 : m_db(db) 35​ : m_db(db)
35 , m_config(config) 36​ , m_config(config)
36{ 37​{
37 Q_ASSERT(db); 38​ Q_ASSERT(db);
38 Q_ASSERT(config); 39​ Q_ASSERT(config);
39} 40​}
40 41​
41void IndexCleaner::run() 42​void IndexCleaner::run()
42{ 43​{
43 QMimeDatabase mimeDb; 44​ QMimeDatabase mimeDb;
44 45​
michaelhUnsubmitted
Done

Unrelated whitespace change

michaelh: Unrelated whitespace change
45 Transaction tr(m_db, Transaction::ReadWrite); 46​ Transaction tr(m_db, Transaction::ReadWrite);
46 47​
47 auto shouldDelete = [&](quint64 id) { 48​ auto shouldDelete = [&](quint64 id) {
48 if (!id) { 49​ if (!id) {
49 return false; 50​ return false;
50 } 51​ }
51 52​
52 QString url = tr.documentUrl(id); 53​ QString url = tr.documentUrl(id);
54​ QFile file(url);
53 55​
54 if (!QFile::exists(url)) { 56​ if (!file.exists() && file.symLinkTarget().isEmpty()) {
55 qDebug() << "not exists: " << url; 57​ qDebug() << "not exists: " << url;
56 return true; 58​ return true;
57 } 59​ }
58 60​
59 if (!m_config->shouldBeIndexed(url)) { 61​ if (!m_config->shouldBeIndexed(url)) {
60 qDebug() << "should not be indexed: " << url; 62​ qDebug() << "should not be indexed: " << url;
61 return true; 63​ return true;
62 } 64​ }
63 65​
64 // FIXME: This mimetype is not completely accurate! 66​ // FIXME: This mimetype is not completely accurate!
65 QString mimetype = mimeDb.mimeTypeForFile(url, QMimeDatabase::MatchExtension).name(); 67​ QString mimetype = mimeDb.mimeTypeForFile(url, QMimeDatabase::MatchExtension).name();
66 if (!m_config->shouldMimeTypeBeIndexed(mimetype)) { 68​ if (!m_config->shouldMimeTypeBeIndexed(mimetype)) {
67 qDebug() << "mimetype should not be indexed: " << url << mimetype; 69​ qDebug() << "mimetype should not be indexed: " << url << mimetype;
68 return true; 70​ return true;
69 } 71​ }
70 72​
71 return false; 73​ return false;
72 }; 74​ };
73 75​
74 const auto includeFolders = m_config->includeFolders(); 76​ for (const auto& device: m_config->storageDevices()->allMedia()) {
brunsUnsubmitted
Not Done

Undesired detach

bruns: Undesired detach
smithjdAuthorUnsubmitted
Not Done

This example will also detach with or without a const intermediate.

...returning a copy from a method means that the copy is shared and has negative side-effects when used with range-based loops. - https://www.dvratil.cz/2015/06/qt-containers-and-c11-range-based-loops

smithjd: This example will also detach with or without a const intermediate. > ...returning a copy…
brunsUnsubmitted
Not Done

No, if done correctly, it will not detach.
For iterating, you need a readonly copy of the container. Using const auto gurantees you will not modify the container. Calling begin() on a const Qt container will not detach.

Try to understand the linked article.

bruns: No, if done correctly, it will not detach. For iterating, you need a readonly copy of the…
75 for (const QString& folder : includeFolders) { 77​ if (!device.isMounted()) {
78​ qDebug() << "skipping device:" << device.udi();
79​ } else {
80​ QString devicePath(device.mountPath() + QLatin1Char('/'));
81​
michaelhUnsubmitted
Done

You can use QStringLiteral("%1/").arg(device.mountPath()) here and maybe define it outside the loop

michaelh: You can use `QStringLiteral("%1/").arg(device.mountPath())` here and maybe define it outside…
82​ for (const QString& folder : m_config->includeFolders()) {
brunsUnsubmitted
Not Done

Undesired detach

bruns: Undesired detach
smithjdAuthorUnsubmitted
Not Done

Same as above.

smithjd: Same as above.
83​ if (folder.startsWith(devicePath)) {
76 quint64 id = filePathToId(QFile::encodeName(folder)); 84​ quint64 id = filePathToId(QFile::encodeName(folder));
77 tr.removeRecursively(id, shouldDelete); 85​ tr.removeRecursively(id, shouldDelete);
78 } 86​ }
87​ }
88​ }
89​ }
79 tr.commit(); 90​ tr.commit();
80 91​
81 Q_EMIT done(); 92​ Q_EMIT done();
82} 93​}