From e25f2392eb4a208449c3aa53196c81583dba08dc Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 11 Oct 2016 00:43:29 +0200 Subject: QGraphicsWidget: add missing break statement to switch in event() If the QEvent::GraphicsSceneMousePress case falls through, it does so because d->hasDecoration() == false or the virtual call to windowFrameEvent() returned false. It falls through to the case for QEvent::GraphicsSceneMouseMove, etc, which ensures d->windowData and then checks hasDecoration() again, with some other conditions on top, and calls the same virtual function, windowFrameEvent(), with the same arguments again. Now, it could, theoretically, be possible that that second call would, due to the presence of a windowData that wasn't there before, return true when before it did return false. But the only modification to *this between the calls to windowFrameEvent() is the potential allocation of d->windowData, which, if actually effected, will have d->windowData->grabbedSection == Qt::NoSection, hence windowFrameEvent() won't even be called a second time It is therefore safe to assume that a break was intended here, so add it. Discovered independently be GCC 7 and Coverity. Coverity-Id: 11149 Change-Id: Id708a1689ed0f0c914622e388c456ea4576fda02 Reviewed-by: Edward Welbourne --- src/widgets/graphicsview/qgraphicswidget.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'src/widgets/graphicsview') diff --git a/src/widgets/graphicsview/qgraphicswidget.cpp b/src/widgets/graphicsview/qgraphicswidget.cpp index 125174627d..5a4f96a2aa 100644 --- a/src/widgets/graphicsview/qgraphicswidget.cpp +++ b/src/widgets/graphicsview/qgraphicswidget.cpp @@ -1449,6 +1449,7 @@ bool QGraphicsWidget::event(QEvent *event) case QEvent::GraphicsSceneMousePress: if (d->hasDecoration() && windowFrameEvent(event)) return true; + break; case QEvent::GraphicsSceneMouseMove: case QEvent::GraphicsSceneMouseRelease: case QEvent::GraphicsSceneMouseDoubleClick: -- cgit v1.2.3 From ef36fd02178482cd312ea551303856ef563421af Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Sat, 8 Oct 2016 16:41:46 +0200 Subject: QGraphicsSceneBspTreeIndex: fix misleading code in event() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old code employed a switch statement to filter timer events, but fell unconditionally through to the default case of calling QObject::event(). The final return statement following the switch is thus dead code. Fix by turning the switch into an if and returning QObject::event() unconditionally afterwards, which much better describes the intent of the code, and also fixes the GCC 7 warning about implicit fall- through in the switch (which wasn't implicit to a human, but GCC's comment-reading-capabilities are somewhat limited at this point). Change-Id: I6756a65b3679a446d09fd721dfd0adc24fdf7772 Reviewed-by: Sérgio Martins Reviewed-by: Edward Welbourne --- src/widgets/graphicsview/qgraphicsscenebsptreeindex.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'src/widgets/graphicsview') diff --git a/src/widgets/graphicsview/qgraphicsscenebsptreeindex.cpp b/src/widgets/graphicsview/qgraphicsscenebsptreeindex.cpp index ce43b1332d..9916591ffa 100644 --- a/src/widgets/graphicsview/qgraphicsscenebsptreeindex.cpp +++ b/src/widgets/graphicsview/qgraphicsscenebsptreeindex.cpp @@ -691,8 +691,7 @@ void QGraphicsSceneBspTreeIndex::itemChange(const QGraphicsItem *item, QGraphics bool QGraphicsSceneBspTreeIndex::event(QEvent *event) { Q_D(QGraphicsSceneBspTreeIndex); - switch (event->type()) { - case QEvent::Timer: + if (event->type() == QEvent::Timer) { if (d->indexTimerId && static_cast(event)->timerId() == d->indexTimerId) { if (d->restartIndexTimer) { d->restartIndexTimer = false; @@ -701,11 +700,8 @@ bool QGraphicsSceneBspTreeIndex::event(QEvent *event) d->_q_updateIndex(); } } - // Fallthrough intended - support timers in subclasses. - default: - return QObject::event(event); } - return true; + return QObject::event(event); } QT_END_NAMESPACE -- cgit v1.2.3