From 59414b7c586d7e75b1738f70d58a34f26bf17338 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 27 Sep 2016 09:03:28 +0200 Subject: Plug memleak in tst_QStackedWidget To keep the change minimal, keep 'sw' as a pointer variable, but back it by a stack-allocated QStackedWidget instead of a heap-allocated one that's never deleted. Change-Id: I9e2a8c07979b861eb7e7040c144d8e75c90d0bc9 Reviewed-by: Giuseppe D'Angelo --- tests/auto/widgets/widgets/qstackedwidget/tst_qstackedwidget.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/auto/widgets/widgets/qstackedwidget/tst_qstackedwidget.cpp b/tests/auto/widgets/widgets/qstackedwidget/tst_qstackedwidget.cpp index d060c4ceae..d9219f8941 100644 --- a/tests/auto/widgets/widgets/qstackedwidget/tst_qstackedwidget.cpp +++ b/tests/auto/widgets/widgets/qstackedwidget/tst_qstackedwidget.cpp @@ -168,7 +168,8 @@ private: void tst_QStackedWidget::dynamicPages() { - QStackedWidget *sw = new QStackedWidget; + QStackedWidget stackedWidget; + QStackedWidget *sw = &stackedWidget; TestPage *w1 = new TestPage(true); w1->setN(3); -- cgit v1.2.3 From dc737fa0d77a7549396efcf9ffa6e917dd40595f Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 27 Sep 2016 09:59:46 +0200 Subject: tst_QShortcut: Fix UB (invalid cast) in shortcutDestroyed() The slot is invoked from QObject::destroyed(), which is emitted from ~QObject. By that time the object is no longer a QShortcut, so the static_cast it invalid. Found by UBSan: tst_qshortcut.cpp:1210:53: runtime error: downcast of address 0x6020000289d0 which does not point to an object of type 'QShortcut' 0x6020000289d0: note: object is of type 'QObject' 10 00 80 17 c0 ce 63 df 93 2b 00 00 b0 02 00 00 d0 60 00 00 02 00 00 00 ff ff ff 04 04 00 00 00 ^~~~~~~~~~~~~~~~~~~~~~~ vptr for 'QObject' #0 0x42b3bb in tst_QShortcut::shortcutDestroyed(QObject*) tst_qshortcut.cpp:1210 #1 0x446cc9 in tst_QShortcut::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) .moc/tst_qshortcut.moc:186 #2 0x2b93dba52c86 in QMetaObject::activate(QObject*, int, int, void**) qobject.cpp:3787 #3 0x2b93dba55400 in QObject::destroyed(QObject*) .moc/moc_qobject.cpp:213 #4 0x2b93dba8d80d in QObject::~QObject() qobject.cpp:967 #5 0x2b93c6b6e032 in QShortcut::~QShortcut() qshortcut.cpp:476 #6 0x2b93c6b6e370 in QShortcut::~QShortcut() qshortcut.cpp:481 #7 0x42a5de in void qDeleteAll::const_iterator>(QList::const_iterator, QList::const_iterator) qalgorithms.h:317 #8 0x42a5de in void qDeleteAll >(QList const&) qalgorithms.h:325 #9 0x42a5de in tst_QShortcut::clearAllShortcuts() tst_qshortcut.cpp:1136 Fix by replacing QVector::replaceAll() with the erase-remove idiom, which does not require the cast, because it can perform mixed-type lookups. Change-Id: I4251c1895fa4398023f489dbfd7108d90c1a6c94 Reviewed-by: Giuseppe D'Angelo --- tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp b/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp index 15aef8d503..8fcc14bf00 100644 --- a/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp +++ b/tests/auto/widgets/kernel/qshortcut/tst_qshortcut.cpp @@ -1211,7 +1211,8 @@ QShortcut *tst_QShortcut::setupShortcut(QWidget *parent, const char *name, int t void tst_QShortcut::shortcutDestroyed(QObject* obj) { - shortcuts.removeAll(static_cast(obj)); + shortcuts.erase(std::remove(shortcuts.begin(), shortcuts.end(), obj), + shortcuts.end()); } void tst_QShortcut::sendKeyEvents(int k1, QChar c1, int k2, QChar c2, int k3, QChar c3, int k4, QChar c4) -- cgit v1.2.3 From f9acbaccde278eaf44a5324c2a63a99a4cccfb1c Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Fri, 26 Aug 2016 12:29:48 +0200 Subject: QPixmap::load: ensure QBitmap stays a QBitmap even on failure ... and avoid detach()ing potentially large data for just preserving the QPlatformPixmap::pixelType(). A QBitmap differs from a QPixmap (its base class, urgh) by always having a data != nullptr and a Bitmap pixel type, yet load() was unconditionally setting 'data' to nullptr on failure, turning a QBitmap into a non-QBitmap. Fix by move-assigning a null QBitmap instead of resetting 'data'. Add some tests. Change-Id: Ida58b3b24d96472a5f9d0f18f81cc763edcf3c16 Reviewed-by: Anton Kudryavtsev Reviewed-by: Edward Welbourne Reviewed-by: Ulf Hermann --- tests/auto/gui/image/qpixmap/tst_qpixmap.cpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'tests') diff --git a/tests/auto/gui/image/qpixmap/tst_qpixmap.cpp b/tests/auto/gui/image/qpixmap/tst_qpixmap.cpp index 4ffe357d09..286f00c111 100644 --- a/tests/auto/gui/image/qpixmap/tst_qpixmap.cpp +++ b/tests/auto/gui/image/qpixmap/tst_qpixmap.cpp @@ -1508,6 +1508,33 @@ void tst_QPixmap::loadAsBitmapOrPixmap() QVERIFY(!bitmap.isNull()); QCOMPARE(bitmap.depth(), 1); QVERIFY(bitmap.isQBitmap()); + + // check that a QBitmap stays a QBitmap even when loading fails: + ok = bitmap.load(QString()); + QVERIFY(!ok); + QVERIFY(bitmap.isNull()); + QVERIFY(bitmap.isQBitmap()); + + ok = bitmap.load("does not exist"); + QVERIFY(!ok); + QVERIFY(bitmap.isNull()); + QVERIFY(bitmap.isQBitmap()); + + ok = bitmap.load("does not exist.png"); + QVERIFY(!ok); + QVERIFY(bitmap.isNull()); + QVERIFY(bitmap.isQBitmap()); + + QTemporaryFile garbage; + QVERIFY(garbage.open()); + const QString garbagePath = garbage.fileName(); + garbage.write(reinterpret_cast(&garbage), sizeof garbage); + garbage.close(); + + ok = bitmap.load(garbagePath); + QVERIFY(!ok); + QVERIFY(bitmap.isNull()); + QVERIFY(bitmap.isQBitmap()); } void tst_QPixmap::toImageDeepCopy() -- cgit v1.2.3 From 5cc0d92c2493aea8397a51eea80167abdd6a1fa6 Mon Sep 17 00:00:00 2001 From: Frank Reininghaus Date: Wed, 18 May 2016 19:25:36 +0200 Subject: QAbstractItemView: use only a 1x1 QRect for selecting on mouse press MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before commit f1e90768095be37419ba4bf3c69ec5c860bdbcb6, mousePressEvent called the virtual method setSelection(const QRect&, SelectionFlags) with the 1x1 rectangle which contains only the clicked QPoint, unless the SelectionFlag "Current" was set because Shift was pressed during the mouse press. Since that commit, the behavior has been changed such that the rectangle is the one that is spanned by the center of the clicked item and the clicked pixel. In theory, the result should be the same (i.e., only the clicked item should be selected), but * the code path in QListView::setSelection for 1x1 QRects is more efficient, and * using a larger QRect can cause problems with custom views, see the comments in QTBUG-18009 This commit ensures that the 1x1 QRect is used again, unless the SelectionFlag "Current" is used. Change-Id: I70dd70c083c20a3af6cd6095aa89a489756b505f Task-number: QTBUG-18009 Reviewed-by: Olivier Goffart (Woboq GmbH) Reviewed-by: Thorbjørn Lund Martsum --- .../qabstractitemview/tst_qabstractitemview.cpp | 52 ++++++++++++++++++++++ 1 file changed, 52 insertions(+) (limited to 'tests') diff --git a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp index 27c9e07037..90ea6f3ef7 100644 --- a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp +++ b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp @@ -152,6 +152,7 @@ private slots: void QTBUG48968_reentrant_updateEditorGeometries(); void QTBUG50535_update_on_new_selection_model(); void testSelectionModelInSyncWithView(); + void testClickToSelect(); }; class MyAbstractItemDelegate : public QAbstractItemDelegate @@ -2074,5 +2075,56 @@ void tst_QAbstractItemView::testSelectionModelInSyncWithView() QCOMPARE(view.selectionModel()->selection().indexes(), QModelIndexList() << model.index(0, 0)); } +class SetSelectionTestView : public QListView +{ + Q_OBJECT +public: + SetSelectionTestView() : QListView() {} + +signals: + void setSelectionCalled(const QRect &rect); + +protected: + void setSelection(const QRect &rect, QItemSelectionModel::SelectionFlags flags) Q_DECL_OVERRIDE + { + emit setSelectionCalled(rect); + QListView::setSelection(rect, flags); + } +}; + +void tst_QAbstractItemView::testClickToSelect() +{ + // This test verifies that the QRect that is passed from QAbstractItemView::mousePressEvent + // to the virtual method QAbstractItemView::setSelection(const QRect &, SelectionFlags) + // is the 1x1 rect which conains exactly the clicked pixel if no modifiers are pressed. + + QStringList list; + list << "A" << "B" << "C"; + QStringListModel model(list); + + SetSelectionTestView view; + view.setModel(&model); + view.show(); + QTest::qWaitForWindowExposed(&view); + + QSignalSpy spy(&view, &SetSelectionTestView::setSelectionCalled); + + const QModelIndex indexA(model.index(0, 0)); + const QRect visualRectA = view.visualRect(indexA); + const QPoint centerA = visualRectA.center(); + + // Click the center of the visualRect of item "A" + QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::NoModifier, centerA); + QCOMPARE(spy.count(), 1); + QCOMPARE(spy.back().front().value(), QRect(centerA, QSize(1, 1))); + + // Click a point slightly away from the center + const QPoint nearCenterA = centerA + QPoint(1, 1); + QVERIFY(visualRectA.contains(nearCenterA)); + QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::NoModifier, nearCenterA); + QCOMPARE(spy.count(), 2); + QCOMPARE(spy.back().front().value(), QRect(nearCenterA, QSize(1, 1))); +} + QTEST_MAIN(tst_QAbstractItemView) #include "tst_qabstractitemview.moc" -- cgit v1.2.3