From 8a3a0edb833aa7907f85af384a27efac17569457 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Mon, 15 Jan 2024 21:22:20 +0100 Subject: [PATCH] cleanup: silence std::move()-related Coverity warnings Unfortunately Coverity doesn't understand that most Qt data structures are copy-on-write. It's a mis-feature of Qt, but it is the way it is. Thus, passing by value is not an issue. Out of ca. 25 warnings only two were legit. Let's silence the others by either std::move()ing or passing by reference, as would be idiomatic C++, which Qt is not. Signed-off-by: Berthold Stoeger --- commands/command_divelist.cpp | 2 +- commands/command_pictures.cpp | 2 +- core/configuredivecomputer.cpp | 6 +++--- core/configuredivecomputer.h | 6 +++--- core/divefilter.cpp | 6 +++--- core/format.cpp | 2 +- core/imagedownloader.cpp | 4 ++-- desktop-widgets/configuredivecomputerdialog.cpp | 2 +- desktop-widgets/configuredivecomputerdialog.h | 2 +- desktop-widgets/divelistview.cpp | 8 ++++---- desktop-widgets/divelistview.h | 4 ++-- desktop-widgets/divelogimportdialog.cpp | 2 +- desktop-widgets/findmovedimagesdialog.cpp | 5 +++-- desktop-widgets/mainwindow.cpp | 6 +++--- desktop-widgets/mainwindow.h | 4 ++-- desktop-widgets/simplewidgets.cpp | 2 +- desktop-widgets/templatelayout.cpp | 2 +- desktop-widgets/templatelayout.h | 2 +- qt-models/divetripmodel.cpp | 2 +- stats/chartlistmodel.cpp | 4 ++-- stats/statsvariables.cpp | 2 +- stats/statsview.cpp | 2 +- 22 files changed, 39 insertions(+), 38 deletions(-) diff --git a/commands/command_divelist.cpp b/commands/command_divelist.cpp index c55652ab1..261e492c1 100644 --- a/commands/command_divelist.cpp +++ b/commands/command_divelist.cpp @@ -234,7 +234,7 @@ DivesAndSitesToRemove DiveListBase::addDives(DivesAndTripsToAdd &toAdd) if (!change.newShown.empty() || !change.newHidden.empty()) emit diveListNotifier.numShownChanged(); - return { res, sites }; + return { std::move(res), std::move(sites) }; } // This helper function renumbers dives according to an array of id/number pairs. diff --git a/commands/command_pictures.cpp b/commands/command_pictures.cpp index 9a17ca9bf..604f7dfea 100644 --- a/commands/command_pictures.cpp +++ b/commands/command_pictures.cpp @@ -83,7 +83,7 @@ static std::vector removePictures(std::vectorvendor; @@ -192,7 +192,7 @@ bool ConfigureDiveComputer::saveXMLBackup(QString fileName, DeviceDetails *detai return true; } -bool ConfigureDiveComputer::restoreXMLBackup(QString fileName, DeviceDetails *details) +bool ConfigureDiveComputer::restoreXMLBackup(const QString &fileName, DeviceDetails *details) { QFile file(fileName); if (!file.open(QIODevice::ReadOnly)) { @@ -491,7 +491,7 @@ bool ConfigureDiveComputer::restoreXMLBackup(QString fileName, DeviceDetails *de return true; } -void ConfigureDiveComputer::startFirmwareUpdate(QString fileName, device_data_t *data, bool forceUpdate) +void ConfigureDiveComputer::startFirmwareUpdate(const QString &fileName, device_data_t *data, bool forceUpdate) { setState(FWUPDATE); if (firmwareThread) diff --git a/core/configuredivecomputer.h b/core/configuredivecomputer.h index 04a18be4b..3abe72b9e 100644 --- a/core/configuredivecomputer.h +++ b/core/configuredivecomputer.h @@ -34,9 +34,9 @@ public: states currentState; void saveDeviceDetails(DeviceDetails *details, device_data_t *data); void fetchDeviceDetails(); - bool saveXMLBackup(QString fileName, DeviceDetails *details, device_data_t *data); - bool restoreXMLBackup(QString fileName, DeviceDetails *details); - void startFirmwareUpdate(QString fileName, device_data_t *data, bool forceUpdate); + bool saveXMLBackup(const QString &fileName, DeviceDetails *details, device_data_t *data); + bool restoreXMLBackup(const QString &fileName, DeviceDetails *details); + void startFirmwareUpdate(const QString &fileName, device_data_t *data, bool forceUpdate); void resetSettings(device_data_t *data); QString dc_open(device_data_t *data); diff --git a/core/divefilter.cpp b/core/divefilter.cpp index 62ad820c9..3bc1ba9fd 100644 --- a/core/divefilter.cpp +++ b/core/divefilter.cpp @@ -145,10 +145,10 @@ bool DiveFilter::showDive(const struct dive *d) const void DiveFilter::startFilterDiveSites(QVector ds) { if (++diveSiteRefCount > 1) { - setFilterDiveSite(ds); + setFilterDiveSite(std::move(ds)); } else { std::sort(ds.begin(), ds.end()); - dive_sites = ds; + dive_sites = std::move(ds); // When switching into dive site mode, reload the dive sites. // TODO: why here? why not catch the filterReset signal in the map widget #ifdef MAP_SUPPORT @@ -176,7 +176,7 @@ void DiveFilter::setFilterDiveSite(QVector ds) std::sort(ds.begin(), ds.end()); if (ds == dive_sites) return; - dive_sites = ds; + dive_sites = std::move(ds); emit diveListNotifier.filterReset(); #ifdef MAP_SUPPORT diff --git a/core/format.cpp b/core/format.cpp index 3da97293a..e9a7c9525 100644 --- a/core/format.cpp +++ b/core/format.cpp @@ -33,7 +33,7 @@ enum length_modifier_t { }; // Helper function to insert '+' or ' ' after last space -static QString insert_sign(QString s, char sign) +static QString insert_sign(const QString &s, char sign) { // For space we can take a shortcut: insert in front if (sign == ' ') diff --git a/core/imagedownloader.cpp b/core/imagedownloader.cpp index 6c2837b20..c90ac9f5f 100644 --- a/core/imagedownloader.cpp +++ b/core/imagedownloader.cpp @@ -186,7 +186,7 @@ Thumbnailer::Thumbnail Thumbnailer::getPictureThumbnailFromStream(QDataStream &s { QImage res; stream >> res; - return { res, MEDIATYPE_PICTURE, zero_duration }; + return { std::move(res), MEDIATYPE_PICTURE, zero_duration }; } void Thumbnailer::markVideoThumbnail(QImage &img) @@ -362,7 +362,7 @@ void Thumbnailer::frameExtracted(QString filename, QImage thumbnail, duration_t addVideoThumbnailToCache(filename, duration, thumbnail, offset); QMutexLocker l(&lock); workingOn.remove(filename); - emit thumbnailChanged(filename, thumbnail, duration); + emit thumbnailChanged(filename, std::move(thumbnail), duration); } } diff --git a/desktop-widgets/configuredivecomputerdialog.cpp b/desktop-widgets/configuredivecomputerdialog.cpp index da916be7f..de43658dc 100644 --- a/desktop-widgets/configuredivecomputerdialog.cpp +++ b/desktop-widgets/configuredivecomputerdialog.cpp @@ -267,7 +267,7 @@ ConfigureDiveComputerDialog::ConfigureDiveComputerDialog(QWidget *parent) : QDia settings.endGroup(); } -OstcFirmwareCheck::OstcFirmwareCheck(QString product) : parent(0) +OstcFirmwareCheck::OstcFirmwareCheck(const QString &product) : parent(0) { QUrl url; memset(&devData, 1, sizeof(devData)); diff --git a/desktop-widgets/configuredivecomputerdialog.h b/desktop-widgets/configuredivecomputerdialog.h index ec833a264..63117088a 100644 --- a/desktop-widgets/configuredivecomputerdialog.h +++ b/desktop-widgets/configuredivecomputerdialog.h @@ -123,7 +123,7 @@ private: class OstcFirmwareCheck : public QObject { Q_OBJECT public: - explicit OstcFirmwareCheck(QString product); + explicit OstcFirmwareCheck(const QString &product); void checkLatest(QWidget *parent, device_data_t *data); public slots: diff --git a/desktop-widgets/divelistview.cpp b/desktop-widgets/divelistview.cpp index 7a7e09fed..864cd7afc 100644 --- a/desktop-widgets/divelistview.cpp +++ b/desktop-widgets/divelistview.cpp @@ -823,7 +823,7 @@ void DiveListView::loadImages() matchImagesToDives(fileNames); } -void DiveListView::matchImagesToDives(QStringList fileNames) +void DiveListView::matchImagesToDives(const QStringList &fileNames) { ShiftImageTimesDialog shiftDialog(this, fileNames); shiftDialog.setOffset(lastImageTimeOffset()); @@ -843,9 +843,9 @@ void DiveListView::matchImagesToDives(QStringList fileNames) auto it = std::find_if(pics.begin(), pics.end(), [d](const Command::PictureListForAddition &l) { return l.d == d; }); if (it == pics.end()) - pics.push_back(Command::PictureListForAddition { d, { pObj } }); + pics.push_back(Command::PictureListForAddition { d, { std::move(pObj) } }); else - it->pics.push_back(pObj); + it->pics.push_back(std::move(pObj)); } if (pics.empty()) @@ -862,7 +862,7 @@ void DiveListView::loadWebImages() loadImagesFromURLs(urlDialog.url()); } -void DiveListView::loadImagesFromURLs(QString urls) +void DiveListView::loadImagesFromURLs(const QString &urls) { QStringList validUrls; QStringList urlList = urls.split('\n'); diff --git a/desktop-widgets/divelistview.h b/desktop-widgets/divelistview.h index 2d458b801..3d6051035 100644 --- a/desktop-widgets/divelistview.h +++ b/desktop-widgets/divelistview.h @@ -76,8 +76,8 @@ private: void updateLastImageTimeOffset(int offset); int lastImageTimeOffset(); void addToTrip(int delta); - void matchImagesToDives(QStringList fileNames); - void loadImagesFromURLs(QString urls); + void matchImagesToDives(const QStringList &fileNames); + void loadImagesFromURLs(const QString &urls); bool eventFilter(QObject *, QEvent *) override; void mouseDoubleClickEvent(QMouseEvent *event) override; void contextMenuEvent(QContextMenuEvent *event) override; diff --git a/desktop-widgets/divelogimportdialog.cpp b/desktop-widgets/divelogimportdialog.cpp index b6633e7bd..faab2d6ad 100644 --- a/desktop-widgets/divelogimportdialog.cpp +++ b/desktop-widgets/divelogimportdialog.cpp @@ -361,7 +361,7 @@ DiveLogImportDialog::DiveLogImportDialog(QStringList fn, QWidget *parent) : QDia ui(new Ui::DiveLogImportDialog) { ui->setupUi(this); - fileNames = fn; + fileNames = std::move(fn); column = 0; delta = "0"; hw = ""; diff --git a/desktop-widgets/findmovedimagesdialog.cpp b/desktop-widgets/findmovedimagesdialog.cpp index 088f195f6..a68291b14 100644 --- a/desktop-widgets/findmovedimagesdialog.cpp +++ b/desktop-widgets/findmovedimagesdialog.cpp @@ -94,7 +94,8 @@ struct Dir { double progressFrom, progressTo; }; -QVector FindMovedImagesDialog::learnImages(const QString &rootdir, int maxRecursions, QVector imagePathsIn) +QVector FindMovedImagesDialog::learnImages(const QString &rootdir, int maxRecursions, + QVector imagePathsIn) { QMap matches; @@ -193,7 +194,7 @@ void FindMovedImagesDialog::on_scanButton_clicked() QFuture> future = QtConcurrent::run( // Note that we capture everything but "this" by copy to avoid dangling references. [this, dirName, imagePaths]() - { return learnImages(dirName, 20, imagePaths);} + { return learnImages(dirName, 20, std::move(imagePaths)); } ); watcher.setFuture(future); } diff --git a/desktop-widgets/mainwindow.cpp b/desktop-widgets/mainwindow.cpp index 9c03f2e5b..7bbe669ff 100644 --- a/desktop-widgets/mainwindow.cpp +++ b/desktop-widgets/mainwindow.cpp @@ -1273,7 +1273,7 @@ void MainWindow::setTitle() setWindowTitle("Subsurface: " + displayedFilename(existing_filename) + unsaved + shown); } -void MainWindow::importFiles(const QStringList fileNames) +void MainWindow::importFiles(const QStringList &fileNames) { if (fileNames.isEmpty()) return; @@ -1289,7 +1289,7 @@ void MainWindow::importFiles(const QStringList fileNames) Command::importDives(&log, IMPORT_MERGE_ALL_TRIPS, source); } -void MainWindow::loadFiles(const QStringList fileNames) +void MainWindow::loadFiles(const QStringList &fileNames) { if (fileNames.isEmpty()) { refreshDisplay(); @@ -1359,7 +1359,7 @@ void MainWindow::on_actionImportDiveLog_triggered() } if (csvFiles.size()) { - DiveLogImportDialog diveLogImport(csvFiles, this); + DiveLogImportDialog diveLogImport(std::move(csvFiles), this); diveLogImport.exec(); } } diff --git a/desktop-widgets/mainwindow.h b/desktop-widgets/mainwindow.h index 8055f2142..7df68750e 100644 --- a/desktop-widgets/mainwindow.h +++ b/desktop-widgets/mainwindow.h @@ -67,8 +67,8 @@ public: Count }; - void loadFiles(const QStringList files); - void importFiles(const QStringList importFiles); + void loadFiles(const QStringList &files); + void importFiles(const QStringList &importFiles); void setToolButtonsEnabled(bool enabled); void setApplicationState(ApplicationState state); void enterPreviousState(); diff --git a/desktop-widgets/simplewidgets.cpp b/desktop-widgets/simplewidgets.cpp index 5b1c13044..07edfb5b7 100644 --- a/desktop-widgets/simplewidgets.cpp +++ b/desktop-widgets/simplewidgets.cpp @@ -515,7 +515,7 @@ QString TextHyperlinkEventFilter::tryToFormulateUrl(QTextCursor *cursor) maybeUrlStr = left + right; } - return stringMeetsOurUrlRequirements(maybeUrlStr) ? maybeUrlStr : QString(); + return stringMeetsOurUrlRequirements(maybeUrlStr) ? std::move(maybeUrlStr) : QString(); } QString TextHyperlinkEventFilter::fromCursorTilWhitespace(QTextCursor *cursor, bool searchBackwards) diff --git a/desktop-widgets/templatelayout.cpp b/desktop-widgets/templatelayout.cpp index 81ee337be..9edc15a40 100644 --- a/desktop-widgets/templatelayout.cpp +++ b/desktop-widgets/templatelayout.cpp @@ -59,7 +59,7 @@ void set_bundled_templates_as_read_only() QFile::setPermissions(pathUser + QDir::separator() + f, QFileDevice::ReadOwner | QFileDevice::ReadUser); } -void copy_bundled_templates(QString src, QString dst, QStringList *templateBackupList) +void copy_bundled_templates(QString src, const QString &dst, QStringList *templateBackupList) { QDir dir(src); if (!dir.exists()) diff --git a/desktop-widgets/templatelayout.h b/desktop-widgets/templatelayout.h index 724b43b59..85bfeb840 100644 --- a/desktop-widgets/templatelayout.h +++ b/desktop-widgets/templatelayout.h @@ -12,7 +12,7 @@ class QTextStream; void find_all_templates(); void set_bundled_templates_as_read_only(); -void copy_bundled_templates(QString src, QString dst, QStringList *templateBackupList); +void copy_bundled_templates(QString src, const QString &dst, QStringList *templateBackupList); enum token_t {LITERAL, FORSTART, FORSTOP, BLOCKSTART, BLOCKSTOP, IFSTART, IFSTOP, PARSERERROR}; diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp index 8ecd16f9e..e7ded819d 100644 --- a/qt-models/divetripmodel.cpp +++ b/qt-models/divetripmodel.cpp @@ -179,7 +179,7 @@ static QString displaySac(const struct dive *d, bool units) if (!d->sac) return QString(); QString s = get_volume_string(d->sac, units); - return units ? s + gettextFromC::tr("/min") : s; + return units ? s + gettextFromC::tr("/min") : std::move(s); } static QString displayWeight(const struct dive *d, bool units) diff --git a/stats/chartlistmodel.cpp b/stats/chartlistmodel.cpp index 1cf35e6e0..88fa64edc 100644 --- a/stats/chartlistmodel.cpp +++ b/stats/chartlistmodel.cpp @@ -35,8 +35,8 @@ void ChartListModel::initIcon(ChartSubType type, const char *name, int iconSize) QPixmap iconWarning = icon.copy(); QPainter painter(&iconWarning); painter.drawPixmap(0, 0, warningPixmap); - subTypeIcons[(size_t)type].normal = icon; - subTypeIcons[(size_t)type].warning = iconWarning; + subTypeIcons[(size_t)type].normal = std::move(icon); + subTypeIcons[(size_t)type].warning = std::move(iconWarning); } const QPixmap &ChartListModel::getIcon(ChartSubType type, bool warning) const diff --git a/stats/statsvariables.cpp b/stats/statsvariables.cpp index ad27c4404..9f67bd42c 100644 --- a/stats/statsvariables.cpp +++ b/stats/statsvariables.cpp @@ -271,7 +271,7 @@ QString StatsBinner::formatWithUnit(const StatsBin &bin) const { QString unit = unitSymbol(); QString name = format(bin); - return unit.isEmpty() ? name : QStringLiteral("%1 %2").arg(name, unit); + return unit.isEmpty() ? std::move(name) : QStringLiteral("%1 %2").arg(name, unit); } QString StatsBinner::formatLowerBound(const StatsBin &bin) const diff --git a/stats/statsview.cpp b/stats/statsview.cpp index 3b21058aa..cc8064040 100644 --- a/stats/statsview.cpp +++ b/stats/statsview.cpp @@ -710,7 +710,7 @@ static std::vector makePercentageLabels(int count, int total, bool isHo if (isHorizontal) return { QString("%1 (%2)").arg(countString, percentageString) }; else - return { countString, percentageString }; + return { std::move(countString), std::move(percentageString) }; } // From a list of dive bins, make (dives, label) pairs, where the label