From 56766dc08b225b86597b6e6823965df95b5ac7ca Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Sat, 21 Jul 2018 15:07:11 +0800 Subject: [PATCH 1/4] Avoid binding constant reference to returned object In such cases, it makes no sense doing so. --- src/base/bittorrent/peerinfo.cpp | 4 ++-- src/base/net/reverseresolution.cpp | 4 ++-- src/base/scanfoldersmodel.cpp | 10 +++++----- src/gui/addnewtorrentdialog.cpp | 4 ++-- src/gui/properties/propertieswidget.cpp | 2 +- src/gui/transferlistfilterswidget.cpp | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/base/bittorrent/peerinfo.cpp b/src/base/bittorrent/peerinfo.cpp index 3db80ceb2..6d5deb46a 100644 --- a/src/base/bittorrent/peerinfo.cpp +++ b/src/base/bittorrent/peerinfo.cpp @@ -246,8 +246,8 @@ QString PeerInfo::connectionType() const void PeerInfo::calcRelevance(const TorrentHandle *torrent) { - const QBitArray &allPieces = torrent->pieces(); - const QBitArray &peerPieces = pieces(); + const QBitArray allPieces = torrent->pieces(); + const QBitArray peerPieces = pieces(); int localMissing = 0; int remoteHaves = 0; diff --git a/src/base/net/reverseresolution.cpp b/src/base/net/reverseresolution.cpp index 15c593776..058f444f9 100644 --- a/src/base/net/reverseresolution.cpp +++ b/src/base/net/reverseresolution.cpp @@ -75,7 +75,7 @@ void ReverseResolution::resolve(const QString &ip) void ReverseResolution::hostResolved(const QHostInfo &host) { - const QString &ip = m_lookups.take(host.lookupId()); + const QString ip = m_lookups.take(host.lookupId()); Q_ASSERT(!ip.isNull()); if (host.error() != QHostInfo::NoError) { @@ -83,7 +83,7 @@ void ReverseResolution::hostResolved(const QHostInfo &host) return; } - const QString &hostname = host.hostName(); + const QString hostname = host.hostName(); qDebug() << Q_FUNC_INFO << ip << QString("->") << hostname; m_cache.insert(ip, new QString(hostname)); diff --git a/src/base/scanfoldersmodel.cpp b/src/base/scanfoldersmodel.cpp index 74c9dc952..a5b00dbbf 100644 --- a/src/base/scanfoldersmodel.cpp +++ b/src/base/scanfoldersmodel.cpp @@ -212,11 +212,11 @@ ScanFoldersModel::PathStatus ScanFoldersModel::addPath(const QString &watchPath, if (!watchDir.exists()) return DoesNotExist; if (!watchDir.isReadable()) return CannotRead; - const QString &canonicalWatchPath = watchDir.canonicalPath(); + const QString canonicalWatchPath = watchDir.canonicalPath(); if (findPathData(canonicalWatchPath) != -1) return AlreadyInList; QDir downloadDir(downloadPath); - const QString &canonicalDownloadPath = downloadDir.canonicalPath(); + const QString canonicalDownloadPath = downloadDir.canonicalPath(); if (!m_fsWatcher) { m_fsWatcher = new FileSystemWatcher(this); @@ -236,12 +236,12 @@ ScanFoldersModel::PathStatus ScanFoldersModel::addPath(const QString &watchPath, ScanFoldersModel::PathStatus ScanFoldersModel::updatePath(const QString &watchPath, const PathType &downloadType, const QString &downloadPath) { QDir watchDir(watchPath); - const QString &canonicalWatchPath = watchDir.canonicalPath(); + const QString canonicalWatchPath = watchDir.canonicalPath(); int row = findPathData(canonicalWatchPath); if (row == -1) return DoesNotExist; QDir downloadDir(downloadPath); - const QString &canonicalDownloadPath = downloadDir.canonicalPath(); + const QString canonicalDownloadPath = downloadDir.canonicalPath(); m_pathList.at(row)->downloadType = downloadType; m_pathList.at(row)->downloadPath = Utils::Fs::toNativePath(canonicalDownloadPath); @@ -256,7 +256,7 @@ void ScanFoldersModel::addToFSWatcher(const QStringList &watchPaths) foreach (const QString &path, watchPaths) { QDir watchDir(path); - const QString &canonicalWatchPath = watchDir.canonicalPath(); + const QString canonicalWatchPath = watchDir.canonicalPath(); m_fsWatcher->addPath(canonicalWatchPath); } } diff --git a/src/gui/addnewtorrentdialog.cpp b/src/gui/addnewtorrentdialog.cpp index 30dc31d97..cb6cdf420 100644 --- a/src/gui/addnewtorrentdialog.cpp +++ b/src/gui/addnewtorrentdialog.cpp @@ -551,7 +551,7 @@ void AddNewTorrentDialog::renameSelectedFile() if (!newPath.endsWith('/')) newPath += '/'; // Check for overwriting for (int i = 0; i < m_torrentInfo.filesCount(); ++i) { - const QString ¤tName = m_torrentInfo.filePath(i); + const QString currentName = m_torrentInfo.filePath(i); #if defined(Q_OS_UNIX) || defined(Q_WS_QWS) if (currentName.startsWith(newPath, Qt::CaseSensitive)) { #else @@ -565,7 +565,7 @@ void AddNewTorrentDialog::renameSelectedFile() } // Replace path in all files for (int i = 0; i < m_torrentInfo.filesCount(); ++i) { - const QString ¤tName = m_torrentInfo.filePath(i); + const QString currentName = m_torrentInfo.filePath(i); if (currentName.startsWith(oldPath)) { QString newName = currentName; newName.replace(0, oldPath.length(), newPath); diff --git a/src/gui/properties/propertieswidget.cpp b/src/gui/properties/propertieswidget.cpp index 2a19a600f..fadcc3675 100644 --- a/src/gui/properties/propertieswidget.cpp +++ b/src/gui/properties/propertieswidget.cpp @@ -751,7 +751,7 @@ void PropertiesWidget::renameSelectedFile() if (!newPath.endsWith('/')) newPath += '/'; // Check for overwriting for (int i = 0; i < m_torrent->filesCount(); ++i) { - const QString ¤tName = m_torrent->filePath(i); + const QString currentName = m_torrent->filePath(i); #if defined(Q_OS_UNIX) || defined(Q_WS_QWS) if (currentName.startsWith(newPath, Qt::CaseSensitive)) { #else diff --git a/src/gui/transferlistfilterswidget.cpp b/src/gui/transferlistfilterswidget.cpp index 5857ff9b7..6613c68f0 100644 --- a/src/gui/transferlistfilterswidget.cpp +++ b/src/gui/transferlistfilterswidget.cpp @@ -515,7 +515,7 @@ void TrackerFiltersList::torrentAboutToBeDeleted(BitTorrent::TorrentHandle *cons QString TrackerFiltersList::trackerFromRow(int row) const { Q_ASSERT(row > 1); - const QString &tracker = item(row)->text(); + const QString tracker = item(row)->text(); QStringList parts = tracker.split(' '); Q_ASSERT(parts.size() >= 2); parts.removeLast(); // Remove trailing number From 517fc3995047787530ab079fbdcb58abcd1a8ce6 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Sat, 21 Jul 2018 16:35:40 +0800 Subject: [PATCH 2/4] Remove unnecessary dynamic allocation Also remove unneeded `if ()` conditional. --- src/base/net/downloadhandler.cpp | 41 +++++++++++--------------------- src/base/net/downloadhandler.h | 1 - 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/src/base/net/downloadhandler.cpp b/src/base/net/downloadhandler.cpp index 4f04102e6..04b8500c2 100644 --- a/src/base/net/downloadhandler.cpp +++ b/src/base/net/downloadhandler.cpp @@ -48,6 +48,20 @@ namespace { QString tr(const char *message); QString errorCodeToString(QNetworkReply::NetworkError status); + + bool saveToFile(const QByteArray &replyData, QString &filePath) + { + QTemporaryFile tmpfile {Utils::Fs::tempPath() + "XXXXXX"}; + tmpfile.setAutoRemove(false); + + if (!tmpfile.open()) + return false; + + filePath = tmpfile.fileName(); + + tmpfile.write(replyData); + return true; + } } Net::DownloadHandler::DownloadHandler(QNetworkReply *reply, DownloadManager *manager, const DownloadRequest &downloadRequest) @@ -145,33 +159,6 @@ void Net::DownloadHandler::checkDownloadSize(qint64 bytesReceived, qint64 bytesT } } -bool Net::DownloadHandler::saveToFile(const QByteArray &replyData, QString &filePath) -{ - QTemporaryFile *tmpfile = new QTemporaryFile(Utils::Fs::tempPath() + "XXXXXX"); - if (!tmpfile->open()) { - delete tmpfile; - return false; - } - - tmpfile->setAutoRemove(false); - filePath = tmpfile->fileName(); - qDebug("Temporary filename is: %s", qUtf8Printable(filePath)); - if (m_reply->isOpen() || m_reply->open(QIODevice::ReadOnly)) { - tmpfile->write(replyData); - tmpfile->close(); - // XXX: tmpfile needs to be deleted on Windows before using the file - // or it will complain that the file is used by another process. - delete tmpfile; - return true; - } - else { - delete tmpfile; - Utils::Fs::forceRemove(filePath); - } - - return false; -} - void Net::DownloadHandler::handleRedirection(QUrl newUrl) { // Resolve relative urls diff --git a/src/base/net/downloadhandler.h b/src/base/net/downloadhandler.h index ef6dc9553..f750f1657 100644 --- a/src/base/net/downloadhandler.h +++ b/src/base/net/downloadhandler.h @@ -66,7 +66,6 @@ namespace Net private: void assignNetworkReply(QNetworkReply *reply); - bool saveToFile(const QByteArray &replyData, QString &filePath); void handleRedirection(QUrl newUrl); QNetworkReply *m_reply; From a70219eea04a1bd48e93e2461f78c558ab5e8730 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Sat, 21 Jul 2018 17:16:28 +0800 Subject: [PATCH 3/4] Move member variable initialization Move the initialization from constructor body to member initializer list. Remove superfluous initializer. --- src/gui/banlistoptionsdialog.cpp | 2 +- src/gui/executionlogwidget.cpp | 3 +-- src/gui/transferlistfilterswidget.cpp | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/gui/banlistoptionsdialog.cpp b/src/gui/banlistoptionsdialog.cpp index 9b54a16f4..30c67ac03 100644 --- a/src/gui/banlistoptionsdialog.cpp +++ b/src/gui/banlistoptionsdialog.cpp @@ -41,10 +41,10 @@ BanListOptionsDialog::BanListOptionsDialog(QWidget *parent) : QDialog(parent) , m_ui(new Ui::BanListOptionsDialog) + , m_model(new QStringListModel(BitTorrent::Session::instance()->bannedIPs(), this)) , m_modified(false) { m_ui->setupUi(this); - m_model = new QStringListModel(BitTorrent::Session::instance()->bannedIPs(), this); m_sortFilter = new QSortFilterProxyModel(this); m_sortFilter->setDynamicSortFilter(true); diff --git a/src/gui/executionlogwidget.cpp b/src/gui/executionlogwidget.cpp index 305cf1074..2a74213c5 100644 --- a/src/gui/executionlogwidget.cpp +++ b/src/gui/executionlogwidget.cpp @@ -39,12 +39,11 @@ ExecutionLogWidget::ExecutionLogWidget(QWidget *parent, const Log::MsgTypes &types) : QWidget(parent) , m_ui(new Ui::ExecutionLogWidget) + , m_msgList(new LogListWidget(MAX_LOG_MESSAGES, Log::MsgTypes(types))) , m_peerList(new LogListWidget(MAX_LOG_MESSAGES)) { m_ui->setupUi(this); - m_msgList = new LogListWidget(MAX_LOG_MESSAGES, Log::MsgTypes(types)); - #ifndef Q_OS_MAC m_ui->tabConsole->setTabIcon(0, GuiIconProvider::instance()->getIcon("view-calendar-journal")); m_ui->tabConsole->setTabIcon(1, GuiIconProvider::instance()->getIcon("view-filter")); diff --git a/src/gui/transferlistfilterswidget.cpp b/src/gui/transferlistfilterswidget.cpp index 6613c68f0..e2d746903 100644 --- a/src/gui/transferlistfilterswidget.cpp +++ b/src/gui/transferlistfilterswidget.cpp @@ -561,7 +561,6 @@ QStringList TrackerFiltersList::getHashes(int row) TransferListFiltersWidget::TransferListFiltersWidget(QWidget *parent, TransferListWidget *transferList) : QFrame(parent) , m_transferList(transferList) - , m_trackerFilters(nullptr) { Preferences *const pref = Preferences::instance(); From 650f585bf3d4cd2586e47cca092c5c1200f8048f Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Mon, 23 Jul 2018 12:49:32 +0800 Subject: [PATCH 4/4] Remove static keyword overuse --- src/base/utils/fs.cpp | 2 +- src/base/utils/misc.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/base/utils/fs.cpp b/src/base/utils/fs.cpp index 1a22eab0b..2371a00ef 100644 --- a/src/base/utils/fs.cpp +++ b/src/base/utils/fs.cpp @@ -110,7 +110,7 @@ bool Utils::Fs::smartRemoveEmptyFolderTree(const QString &path) if (path.isEmpty() || !QDir(path).exists()) return true; - static const QStringList deleteFilesList = { + const QStringList deleteFilesList = { // Windows "Thumbs.db", "desktop.ini", diff --git a/src/base/utils/misc.cpp b/src/base/utils/misc.cpp index 72dbb840d..74c7aefc5 100644 --- a/src/base/utils/misc.cpp +++ b/src/base/utils/misc.cpp @@ -139,7 +139,7 @@ void Utils::Misc::shutdownComputer(const ShutdownDialogAction &action) else EventToSend = kAEShutDown; AEAddressDesc targetDesc; - static const ProcessSerialNumber kPSNOfSystemProcess = {0, kSystemProcess}; + const ProcessSerialNumber kPSNOfSystemProcess = {0, kSystemProcess}; AppleEvent eventReply = {typeNull, NULL}; AppleEvent appleEventToSend = {typeNull, NULL};