From 1d8030cc64df2cdfdc0faf3d06ea7d9ed0306948 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Wed, 30 Dec 2015 02:04:26 +0100 Subject: [PATCH] QtWidgets: prevent detach attempts from first()/etc. use [dialogs, kernel, util, widgets] The algorithm used was: - If possible, just declare the container const - Otherwise, for first()/last(), use constFirst()/constLast() and for front()/back(), to not destroy the use of the STL API subset, use qAsConst() Did some caching of function returns here and there, and converted one 0 to nullptr as a drive-by. Also saves almost 4KiB in text size on optimized GCC 4.9 Linux AMD64 builds. Change-Id: I04b7bfd68dc85c22de247cb65a310e1cbbca1e8c Reviewed-by: Thiago Macieira Reviewed-by: Lars Knoll Reviewed-by: Marc Mutz --- src/widgets/dialogs/qfiledialog.cpp | 8 +++---- src/widgets/dialogs/qfileinfogatherer.cpp | 4 ++-- src/widgets/dialogs/qfilesystemmodel.cpp | 2 +- src/widgets/dialogs/qsidebar.cpp | 4 ++-- src/widgets/dialogs/qwizard.cpp | 4 ++-- src/widgets/kernel/qapplication.cpp | 6 ++--- src/widgets/util/qcompleter.cpp | 4 ++-- src/widgets/util/qscroller.cpp | 27 ++++++++++++++-------- src/widgets/util/qundostack.cpp | 8 +++---- src/widgets/widgets/qdockarealayout.cpp | 6 ++--- src/widgets/widgets/qmainwindowlayout.cpp | 10 ++++---- src/widgets/widgets/qmenu.cpp | 2 +- src/widgets/widgets/qtabbar.cpp | 6 ++--- src/widgets/widgets/qtoolbararealayout.cpp | 2 +- 14 files changed, 51 insertions(+), 42 deletions(-) diff --git a/src/widgets/dialogs/qfiledialog.cpp b/src/widgets/dialogs/qfiledialog.cpp index 50667e2a91..c4ed72edc3 100644 --- a/src/widgets/dialogs/qfiledialog.cpp +++ b/src/widgets/dialogs/qfiledialog.cpp @@ -2689,7 +2689,7 @@ void QFileDialogPrivate::saveSettings() settings.beginGroup(QLatin1String("FileDialog")); if (usingWidgets()) { - settings.setValue(QLatin1String("sidebarWidth"), qFileDialogUi->splitter->sizes().first()); + settings.setValue(QLatin1String("sidebarWidth"), qFileDialogUi->splitter->sizes().constFirst()); settings.setValue(QLatin1String("shortcuts"), QUrl::toStringList(qFileDialogUi->sidebar->urls())); settings.setValue(QLatin1String("treeViewHeader"), qFileDialogUi->treeView->header()->saveState()); } @@ -3001,9 +3001,9 @@ void QFileDialogPrivate::createWidgets() q->selectNameFilter(options->initiallySelectedNameFilter()); q->setDefaultSuffix(options->defaultSuffix()); q->setHistory(options->history()); - if (options->initiallySelectedFiles().count() == 1) - q->selectFile(options->initiallySelectedFiles().first().fileName()); const auto initiallySelectedFiles = options->initiallySelectedFiles(); + if (initiallySelectedFiles.size() == 1) + q->selectFile(initiallySelectedFiles.first().fileName()); for (const QUrl &url : initiallySelectedFiles) q->selectUrl(url); lineEdit()->selectAll(); @@ -4095,7 +4095,7 @@ QStringList QFSCompleter::splitPath(const QString &path) const parts.removeFirst(); currentLocationList.removeLast(); } - if (!currentLocationList.isEmpty() && currentLocationList.last().isEmpty()) + if (!currentLocationList.isEmpty() && currentLocationList.constLast().isEmpty()) currentLocationList.removeLast(); return currentLocationList + parts; } diff --git a/src/widgets/dialogs/qfileinfogatherer.cpp b/src/widgets/dialogs/qfileinfogatherer.cpp index 48ae5f1fc4..b20db8fc7c 100644 --- a/src/widgets/dialogs/qfileinfogatherer.cpp +++ b/src/widgets/dialogs/qfileinfogatherer.cpp @@ -217,9 +217,9 @@ void QFileInfoGatherer::run() condition.wait(&mutex); if (abort.load()) return; - const QString thisPath = path.front(); + const QString thisPath = qAsConst(path).front(); path.pop_front(); - const QStringList thisList = files.front(); + const QStringList thisList = qAsConst(files).front(); files.pop_front(); locker.unlock(); diff --git a/src/widgets/dialogs/qfilesystemmodel.cpp b/src/widgets/dialogs/qfilesystemmodel.cpp index bfa40317eb..c72761f2ae 100644 --- a/src/widgets/dialogs/qfilesystemmodel.cpp +++ b/src/widgets/dialogs/qfilesystemmodel.cpp @@ -374,7 +374,7 @@ QFileSystemModelPrivate::QFileSystemNode *QFileSystemModelPrivate::node(const QS QModelIndex index = QModelIndex(); // start with "My Computer" #if defined(Q_OS_WIN) && !defined(Q_OS_WINCE) if (absolutePath.startsWith(QLatin1String("//"))) { // UNC path - QString host = QLatin1String("\\\\") + pathElements.first(); + QString host = QLatin1String("\\\\") + pathElements.constFirst(); if (absolutePath == QDir::fromNativeSeparators(host)) absolutePath.append(QLatin1Char('/')); if (longPath.endsWith(QLatin1Char('/')) && !absolutePath.endsWith(QLatin1Char('/'))) diff --git a/src/widgets/dialogs/qsidebar.cpp b/src/widgets/dialogs/qsidebar.cpp index 2e884e238d..1b1eb6472e 100644 --- a/src/widgets/dialogs/qsidebar.cpp +++ b/src/widgets/dialogs/qsidebar.cpp @@ -127,7 +127,7 @@ QMimeData *QUrlModel::mimeData(const QModelIndexList &indexes) const */ bool QUrlModel::canDrop(QDragEnterEvent *event) { - if (!event->mimeData()->formats().contains(mimeTypes().first())) + if (!event->mimeData()->formats().contains(mimeTypes().constFirst())) return false; const QList list = event->mimeData()->urls(); @@ -145,7 +145,7 @@ bool QUrlModel::canDrop(QDragEnterEvent *event) bool QUrlModel::dropMimeData(const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parent) { - if (!data->formats().contains(mimeTypes().first())) + if (!data->formats().contains(mimeTypes().constFirst())) return false; Q_UNUSED(action); Q_UNUSED(column); diff --git a/src/widgets/dialogs/qwizard.cpp b/src/widgets/dialogs/qwizard.cpp index aa10f65389..53f947354c 100644 --- a/src/widgets/dialogs/qwizard.cpp +++ b/src/widgets/dialogs/qwizard.cpp @@ -850,9 +850,9 @@ void QWizardPrivate::switchToPage(int newId, Direction direction) q->cleanupPage(oldId); initialized.remove(oldId); } - Q_ASSERT(history.last() == oldId); + Q_ASSERT(history.constLast() == oldId); history.removeLast(); - Q_ASSERT(history.last() == newId); + Q_ASSERT(history.constLast() == newId); } } diff --git a/src/widgets/kernel/qapplication.cpp b/src/widgets/kernel/qapplication.cpp index ebdbdbd3e6..eee91fea47 100644 --- a/src/widgets/kernel/qapplication.cpp +++ b/src/widgets/kernel/qapplication.cpp @@ -786,7 +786,7 @@ void QApplicationPrivate::initializeWidgetFontHash() QWidget *QApplication::activePopupWidget() { return QApplicationPrivate::popupWidgets && !QApplicationPrivate::popupWidgets->isEmpty() ? - QApplicationPrivate::popupWidgets->last() : 0; + QApplicationPrivate::popupWidgets->constLast() : nullptr; } @@ -2391,7 +2391,7 @@ void QApplicationPrivate::dispatchEnterLeave(QWidget* enter, QWidget* leave, con const QPoint globalPos = qIsInf(globalPosF.x()) ? QPoint(QWIDGETSIZE_MAX, QWIDGETSIZE_MAX) : globalPosF.toPoint(); - const QPoint windowPos = enterList.back()->window()->mapFromGlobal(globalPos); + const QPoint windowPos = qAsConst(enterList).back()->window()->mapFromGlobal(globalPos); for (auto it = enterList.crbegin(), end = enterList.crend(); it != end; ++it) { auto *w = *it; if (!QApplication::activeModalWidget() || QApplicationPrivate::tryModalHelper(w, 0)) { @@ -3795,7 +3795,7 @@ void QApplicationPrivate::closePopup(QWidget *popup) } else { // A popup was closed, so the previous popup gets the focus. - QWidget* aw = QApplicationPrivate::popupWidgets->last(); + QWidget* aw = QApplicationPrivate::popupWidgets->constLast(); if (QWidget *fw = aw->focusWidget()) fw->setFocus(Qt::PopupFocusReason); diff --git a/src/widgets/util/qcompleter.cpp b/src/widgets/util/qcompleter.cpp index 9c61a3a263..9f167f5924 100644 --- a/src/widgets/util/qcompleter.cpp +++ b/src/widgets/util/qcompleter.cpp @@ -755,9 +755,9 @@ void QUnsortedModelEngine::filterOnDemand(int n) const QAbstractItemModel *model = c->proxy->sourceModel(); int lastRow = model->rowCount(curParent) - 1; QIndexMapper im(curMatch.indices.last() + 1, lastRow); - int lastIndex = buildIndices(curParts.last(), curParent, n, im, &curMatch); + int lastIndex = buildIndices(curParts.constLast(), curParent, n, im, &curMatch); curMatch.partial = (lastRow != lastIndex); - saveInCache(curParts.last(), curParent, curMatch); + saveInCache(curParts.constLast(), curParent, curMatch); } QMatchData QUnsortedModelEngine::filter(const QString& part, const QModelIndex& parent, int n) diff --git a/src/widgets/util/qscroller.cpp b/src/widgets/util/qscroller.cpp index 2d6bcf72c6..02e3c2b82a 100644 --- a/src/widgets/util/qscroller.cpp +++ b/src/widgets/util/qscroller.cpp @@ -575,8 +575,11 @@ QPointF QScroller::pixelPerMeter() const if (QGraphicsObject *go = qobject_cast(d->target)) { QTransform viewtr; //TODO: the first view isn't really correct - maybe use an additional field in the prepare event? - if (go->scene() && !go->scene()->views().isEmpty()) - viewtr = go->scene()->views().first()->viewportTransform(); + if (const auto *scene = go->scene()) { + const auto views = scene->views(); + if (!views.isEmpty()) + viewtr = views.first()->viewportTransform(); + } QTransform tr = go->deviceTransform(viewtr); if (tr.isScaling()) { QPointF p0 = tr.map(QPointF(0, 0)); @@ -1121,12 +1124,15 @@ void QScrollerPrivate::pushSegment(ScrollType type, qreal deltaTime, qreal stopP return; ScrollSegment s; - if (orientation == Qt::Horizontal && !xSegments.isEmpty()) - s.startTime = xSegments.last().startTime + xSegments.last().deltaTime * xSegments.last().stopProgress; - else if (orientation == Qt::Vertical && !ySegments.isEmpty()) - s.startTime = ySegments.last().startTime + ySegments.last().deltaTime * ySegments.last().stopProgress; - else + if (orientation == Qt::Horizontal && !xSegments.isEmpty()) { + const auto &lastX = xSegments.constLast(); + s.startTime = lastX.startTime + lastX.deltaTime * lastX.stopProgress; + } else if (orientation == Qt::Vertical && !ySegments.isEmpty()) { + const auto &lastY = ySegments.constLast(); + s.startTime = lastY.startTime + lastY.deltaTime * lastY.stopProgress; + } else { s.startTime = monotonicTimer.elapsed(); + } s.startPos = startPos; s.deltaPos = deltaPos; @@ -1463,8 +1469,11 @@ bool QScrollerPrivate::prepareScrolling(const QPointF &position) #ifndef QT_NO_GRAPHICSVIEW if (QGraphicsObject *go = qobject_cast(target)) { //TODO: the first view isn't really correct - maybe use an additional field in the prepare event? - if (go->scene() && !go->scene()->views().isEmpty()) - setDpiFromWidget(go->scene()->views().first()); + if (const auto *scene = go->scene()) { + const auto views = scene->views(); + if (!views.isEmpty()) + setDpiFromWidget(views.first()); + } } #endif diff --git a/src/widgets/util/qundostack.cpp b/src/widgets/util/qundostack.cpp index 6643b4429c..6f733f99d5 100644 --- a/src/widgets/util/qundostack.cpp +++ b/src/widgets/util/qundostack.cpp @@ -588,9 +588,9 @@ void QUndoStack::push(QUndoCommand *cmd) QUndoCommand *cur = 0; if (macro) { - QUndoCommand *macro_cmd = d->macro_stack.last(); + QUndoCommand *macro_cmd = d->macro_stack.constLast(); if (!macro_cmd->d->child_list.isEmpty()) - cur = macro_cmd->d->child_list.last(); + cur = macro_cmd->d->child_list.constLast(); } else { if (d->index > 0) cur = d->command_list.at(d->index - 1); @@ -616,7 +616,7 @@ void QUndoStack::push(QUndoCommand *cmd) } } else { if (macro) { - d->macro_stack.last()->d->child_list.append(cmd); + d->macro_stack.constLast()->d->child_list.append(cmd); } else { d->command_list.append(cmd); d->checkUndoLimit(); @@ -963,7 +963,7 @@ void QUndoStack::beginMacro(const QString &text) d->clean_index = -1; // we've deleted the clean state d->command_list.append(cmd); } else { - d->macro_stack.last()->d->child_list.append(cmd); + d->macro_stack.constLast()->d->child_list.append(cmd); } d->macro_stack.append(cmd); diff --git a/src/widgets/widgets/qdockarealayout.cpp b/src/widgets/widgets/qdockarealayout.cpp index 29089311c6..c14fd5a942 100644 --- a/src/widgets/widgets/qdockarealayout.cpp +++ b/src/widgets/widgets/qdockarealayout.cpp @@ -3096,7 +3096,7 @@ void QDockAreaLayout::addDockWidget(QInternal::DockPosition pos, QDockWidget *do void QDockAreaLayout::tabifyDockWidget(QDockWidget *first, QDockWidget *second) { - QList path = indexOf(first); + const QList path = indexOf(first); if (path.isEmpty()) return; @@ -3133,7 +3133,7 @@ void QDockAreaLayout::resizeDocks(const QList &docks, while (path.size() > 1) { QDockAreaLayoutInfo *info = this->info(path); if (!info->tabbed && info->o == o) { - info->item_list[path.last()].size = size; + info->item_list[path.constLast()].size = size; int totalSize = 0; foreach (const QDockAreaLayoutItem &item, info->item_list) { if (!item.skip()) { @@ -3147,7 +3147,7 @@ void QDockAreaLayout::resizeDocks(const QList &docks, path.removeLast(); } - const int dockNum = path.first(); + const int dockNum = path.constFirst(); Q_ASSERT(dockNum < QInternal::DockCount); QRect &r = this->docks[dockNum].rect; QSize s = r.size(); diff --git a/src/widgets/widgets/qmainwindowlayout.cpp b/src/widgets/widgets/qmainwindowlayout.cpp index ae87ea526d..e46063917d 100644 --- a/src/widgets/widgets/qmainwindowlayout.cpp +++ b/src/widgets/widgets/qmainwindowlayout.cpp @@ -1163,7 +1163,7 @@ void QMainWindowLayout::insertToolBar(QToolBar *before, QToolBar *toolbar) // copy the toolbar also in the saved state savedState.toolBarAreaLayout.insertItem(before, item); } - if (!currentGapPos.isEmpty() && currentGapPos.first() == 0) { + if (!currentGapPos.isEmpty() && currentGapPos.constFirst() == 0) { currentGapPos = layoutState.toolBarAreaLayout.currentGapIndex(); if (!currentGapPos.isEmpty()) { currentGapPos.prepend(0); @@ -1501,7 +1501,7 @@ void QMainWindowLayout::splitDockWidget(QDockWidget *after, Qt::DockWidgetArea QMainWindowLayout::dockWidgetArea(QWidget *widget) const { - QList pathToWidget = layoutState.dockAreaLayout.indexOf(widget); + const QList pathToWidget = layoutState.dockAreaLayout.indexOf(widget); if (pathToWidget.isEmpty()) return Qt::NoDockWidgetArea; return toDockWidgetArea(pathToWidget.first()); @@ -1783,7 +1783,7 @@ QLayoutItem *QMainWindowLayout::takeAt(int index) } #ifndef QT_NO_TOOLBAR - if (!currentGapPos.isEmpty() && currentGapPos.first() == 0) { + if (!currentGapPos.isEmpty() && currentGapPos.constFirst() == 0) { currentGapPos = layoutState.toolBarAreaLayout.currentGapIndex(); if (!currentGapPos.isEmpty()) { currentGapPos.prepend(0); @@ -2057,7 +2057,7 @@ void QMainWindowLayout::animationFinished(QWidget *widget) if (parentInfo->tabbed) { // merge the two tab widgets - int idx = path.last(); + int idx = path.constLast(); Q_ASSERT(parentInfo->item_list[idx].widgetItem->widget() == dwgw); delete parentInfo->item_list[idx].widgetItem; parentInfo->item_list.removeAt(idx); @@ -2283,7 +2283,7 @@ QLayoutItem *QMainWindowLayout::unplug(QWidget *widget, bool group) #ifndef QT_NO_DOCKWIDGET if (QDockWidget *dw = qobject_cast(widget)) { - Q_ASSERT(path.first() == 1); + Q_ASSERT(path.constFirst() == 1); bool actualGroup = false; #ifndef QT_NO_TABBAR if (group && (dockOptions & QMainWindow::GroupedDragging) && path.size() > 3) { diff --git a/src/widgets/widgets/qmenu.cpp b/src/widgets/widgets/qmenu.cpp index 514cb1a8c9..c07d48a513 100644 --- a/src/widgets/widgets/qmenu.cpp +++ b/src/widgets/widgets/qmenu.cpp @@ -117,7 +117,7 @@ public: { Q_D(QTornOffMenu); // make the torn-off menu a sibling of p (instead of a child) - QWidget *parentWidget = d->causedStack.isEmpty() ? p : d->causedStack.last(); + QWidget *parentWidget = d->causedStack.isEmpty() ? p : d->causedStack.constLast(); if (parentWidget->parentWidget()) parentWidget = parentWidget->parentWidget(); setParent(parentWidget, Qt::Window | Qt::Tool); diff --git a/src/widgets/widgets/qtabbar.cpp b/src/widgets/widgets/qtabbar.cpp index cf485974f6..ba3fc2fcd7 100644 --- a/src/widgets/widgets/qtabbar.cpp +++ b/src/widgets/widgets/qtabbar.cpp @@ -590,7 +590,7 @@ QRect QTabBarPrivate::normalizedScrollRect(int index) } bool tearTopVisible = index != 0 && topEdge != -scrollOffset; - bool tearBottomVisible = index != tabList.size() - 1 && bottomEdge != tabList.last().rect.bottom() + 1 - scrollOffset; + bool tearBottomVisible = index != tabList.size() - 1 && bottomEdge != tabList.constLast().rect.bottom() + 1 - scrollOffset; if (tearTopVisible && !tearLeftRect.isNull()) topEdge = tearLeftRect.bottom() + 1; if (tearBottomVisible && !tearRightRect.isNull()) @@ -621,7 +621,7 @@ QRect QTabBarPrivate::normalizedScrollRect(int index) } bool tearLeftVisible = index != 0 && leftEdge != -scrollOffset; - bool tearRightVisible = index != tabList.size() - 1 && rightEdge != tabList.last().rect.right() + 1 - scrollOffset; + bool tearRightVisible = index != tabList.size() - 1 && rightEdge != tabList.constLast().rect.right() + 1 - scrollOffset; if (tearLeftVisible && !tearLeftRect.isNull()) leftEdge = tearLeftRect.right() + 1; if (tearRightVisible && !tearRightRect.isNull()) @@ -642,7 +642,7 @@ void QTabBarPrivate::makeVisible(int index) const bool horiz = !verticalTabs(shape); const int tabStart = horiz ? tabRect.left() : tabRect.top(); const int tabEnd = horiz ? tabRect.right() : tabRect.bottom(); - const int lastTabEnd = horiz ? tabList.last().rect.right() : tabList.last().rect.bottom(); + const int lastTabEnd = horiz ? tabList.constLast().rect.right() : tabList.constLast().rect.bottom(); const QRect scrollRect = normalizedScrollRect(index); const int scrolledTabBarStart = qMax(1, scrollRect.left() + scrollOffset); const int scrolledTabBarEnd = qMin(lastTabEnd - 1, scrollRect.right() + scrollOffset); diff --git a/src/widgets/widgets/qtoolbararealayout.cpp b/src/widgets/widgets/qtoolbararealayout.cpp index 2e79b502ea..05564bb6b1 100644 --- a/src/widgets/widgets/qtoolbararealayout.cpp +++ b/src/widgets/widgets/qtoolbararealayout.cpp @@ -347,7 +347,7 @@ void QToolBarAreaLayoutInfo::removeToolBar(QToolBar *toolBar) void QToolBarAreaLayoutInfo::insertToolBarBreak(QToolBar *before) { if (before == 0) { - if (!lines.isEmpty() && lines.last().toolBarItems.isEmpty()) + if (!lines.isEmpty() && lines.constLast().toolBarItems.isEmpty()) return; lines.append(QToolBarAreaLayoutLine(o)); return;