AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Avoid adding widget to its own layout

Browse Source 
Widgets and layouts added or inserted to a layout are checked for:
- Not being NULL
- Not being the parent widget of a layout or the layout itself,
  respectively

Without this commit, adding a widget to its own layout would result in a
CPU-hogging infinite loop. Now, a warning is written to stderr and the
add or insert function call is ignored.

The checks are implemented as public functions of QLayoutPrivate and
thus accessible in QLayout's descendants to be used in various
"addWidget", "insertWidget", etc functions.

Unlike 'classical' layouts like QGridLayout, QFormLayout does indeed
accept widgets that are NULL. To not break this behavior, any call for
the check functions first tests if the widget or layout, respectively,
to test is NULL or not and calls the check only in the latter case.

Automated tests for QBoxLayout, QGridLayout, and QFormLayout were added.
For an unpatched Qt 5.3, each of those automated tests will freeze as
explained in QTBUG-40609. For a fixed version, warning messages about
invalid parameters to addWidget/addLayout/... calls will be read by
QTest::ignoreMessage, resulting in a passed test.

Change-Id: I1522d5727e643da3f7c025755975aca9f482676d
Task-number: QTBUG-40609
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>
This commit is contained in:
Thomas Fischer 2014-08-24 14:01:26 +02:00 committed by Marc Mutz
parent 8206a263ab
commit 983dde1f2f
11 changed files with 179 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
src/widgets/kernel/qboxlayout.cpp
View File

@ -51,20 +51,6 @@
QT_BEGIN_NAMESPACE
/*
Returns \c true if the \a widget can be added to the \a layout;
otherwise returns \c false.
*/
static bool checkWidget(QLayout *layout, QWidget *widget)
{
if (!widget) {
qWarning("QLayout: Cannot add null widget to %s/%s", layout->metaObject()->className(),
layout->objectName().toLocal8Bit().data());
return false;
}
return true;
}
struct QBoxLayoutItem
{
QBoxLayoutItem(QLayoutItem *it, int stretch_ = 0)
@ -958,6 +944,8 @@ void QBoxLayout::insertSpacerItem(int index, QSpacerItem *spacerItem)
void QBoxLayout::insertLayout(int index, QLayout *layout, int stretch)
{
Q_D(QBoxLayout);
if (!d->checkLayout(layout))
return;
if (!adoptLayout(layout))
return;
if (index < 0) // append
@ -991,7 +979,7 @@ void QBoxLayout::insertWidget(int index, QWidget *widget, int stretch,
Qt::Alignment alignment)
{
Q_D(QBoxLayout);
if (!checkWidget(this, widget))
if (!d->checkWidget(widget))
return;
addChildWidget(widget);
if (index < 0) // append

22
src/widgets/kernel/qformlayout.cpp
View File

@ -1280,6 +1280,8 @@ void QFormLayout::addRow(QLayout *layout)
void QFormLayout::insertRow(int row, QWidget *label, QWidget *field)
{
Q_D(QFormLayout);
if ((label && !d->checkWidget(label)) || (field && !d->checkWidget(field)))
return;
row = d->insertRow(row);
if (label)
@ -1295,6 +1297,8 @@ void QFormLayout::insertRow(int row, QWidget *label, QWidget *field)
void QFormLayout::insertRow(int row, QWidget *label, QLayout *field)
{
Q_D(QFormLayout);
if ((label && !d->checkWidget(label)) || (field && !d->checkLayout(field)))
return;
row = d->insertRow(row);
if (label)
@ -1313,6 +1317,10 @@ void QFormLayout::insertRow(int row, QWidget *label, QLayout *field)
*/
void QFormLayout::insertRow(int row, const QString &labelText, QWidget *field)
{
Q_D(QFormLayout);
if (field && !d->checkWidget(field))
return;
QLabel *label = 0;
if (!labelText.isEmpty()) {
label = new QLabel(labelText);
@ -1331,6 +1339,10 @@ void QFormLayout::insertRow(int row, const QString &labelText, QWidget *field)
*/
void QFormLayout::insertRow(int row, const QString &labelText, QLayout *field)
{
Q_D(QFormLayout);
if (field && !d->checkLayout(field))
return;
insertRow(row, labelText.isEmpty() ? 0 : new QLabel(labelText), field);
}
@ -1344,11 +1356,8 @@ void QFormLayout::insertRow(int row, const QString &labelText, QLayout *field)
void QFormLayout::insertRow(int row, QWidget *widget)
{
Q_D(QFormLayout);
if (!widget) {
qWarning("QFormLayout: Cannot add null field to %s", qPrintable(objectName()));
if (!d->checkWidget(widget))
return;
}
row = d->insertRow(row);
d->setWidget(row, SpanningRole, widget);
@ -1365,11 +1374,8 @@ void QFormLayout::insertRow(int row, QWidget *widget)
void QFormLayout::insertRow(int row, QLayout *layout)
{
Q_D(QFormLayout);
if (!layout) {
qWarning("QFormLayout: Cannot add null field to %s", qPrintable(objectName()));
if (!d->checkLayout(layout))
return;
}
row = d->insertRow(row);
d->setLayout(row, SpanningRole, layout);

23
src/widgets/kernel/qgridlayout.cpp
View File

@ -1430,20 +1430,6 @@ void QGridLayout::addItem(QLayoutItem *item, int row, int column, int rowSpan, i
invalidate();
}
/*
Returns \c true if the widget \a w can be added to the layout \a l;
otherwise returns \c false.
*/
static bool checkWidget(QLayout *l, QWidget *w)
{
if (!w) {
qWarning("QLayout: Cannot add null widget to %s/%s", l->metaObject()->className(),
l->objectName().toLocal8Bit().data());
return false;
}
return true;
}
/*!
Adds the given \a widget to the cell grid at \a row, \a column. The
top-left position is (0, 0) by default.
@ -1454,7 +1440,8 @@ static bool checkWidget(QLayout *l, QWidget *w)
*/
void QGridLayout::addWidget(QWidget *widget, int row, int column, Qt::Alignment alignment)
{
if (!checkWidget(this, widget))
Q_D(QGridLayout);
if (!d->checkWidget(widget))
return;
if (row < 0 || column < 0) {
qWarning("QGridLayout: Cannot add %s/%s to %s/%s at row %d column %d",
@ -1483,7 +1470,7 @@ void QGridLayout::addWidget(QWidget *widget, int fromRow, int fromColumn,
int rowSpan, int columnSpan, Qt::Alignment alignment)
{
Q_D(QGridLayout);
if (!checkWidget(this, widget))
if (!d->checkWidget(widget))
return;
int toRow = (rowSpan < 0) ? -1 : fromRow + rowSpan - 1;
int toColumn = (columnSpan < 0) ? -1 : fromColumn + columnSpan - 1;
@ -1518,6 +1505,8 @@ void QGridLayout::addWidget(QWidget *widget, int fromRow, int fromColumn,
void QGridLayout::addLayout(QLayout *layout, int row, int column, Qt::Alignment alignment)
{
Q_D(QGridLayout);
if (!d->checkLayout(layout))
return;
if (!adoptLayout(layout))
return;
QGridBox *b = new QGridBox(layout);
@ -1538,6 +1527,8 @@ void QGridLayout::addLayout(QLayout *layout, int row, int column,
int rowSpan, int columnSpan, Qt::Alignment alignment)
{
Q_D(QGridLayout);
if (!d->checkLayout(layout))
return;
if (!adoptLayout(layout))
return;
QGridBox *b = new QGridBox(layout);

41
src/widgets/kernel/qlayout.cpp
View File

@ -857,6 +857,47 @@ void QLayoutPrivate::reparentChildWidgets(QWidget *mw)
}
}
/*!
Returns \c true if the \a widget can be added to the \a layout;
otherwise returns \c false.
*/
bool QLayoutPrivate::checkWidget(QWidget *widget) const
{
Q_Q(const QLayout);
if (!widget) {
qWarning("QLayout: Cannot add a null widget to %s/%s", q->metaObject()->className(),
qPrintable(q->objectName()));
return false;
}
if (widget == q->parentWidget()) {
qWarning("QLayout: Cannot add parent widget %s/%s to its child layout %s/%s",
widget->metaObject()->className(), qPrintable(widget->objectName()),
q->metaObject()->className(), qPrintable(q->objectName()));
return false;
}
return true;
}
/*!
Returns \c true if the \a otherLayout can be added to the \a layout;
otherwise returns \c false.
*/
bool QLayoutPrivate::checkLayout(QLayout *otherLayout) const
{
Q_Q(const QLayout);
if (!otherLayout) {
qWarning("QLayout: Cannot add a null layout to %s/%s", q->metaObject()->className(),
qPrintable(q->objectName()));
return false;
}
if (otherLayout == q) {
qWarning("QLayout: Cannot add layout %s/%s to itself", q->metaObject()->className(),
qPrintable(q->objectName()));
return false;
}
return true;
}
/*!
This function is called from \c addWidget() functions in
subclasses to add \a w as a managed widget of a layout.

2
src/widgets/kernel/qlayout_p.h
View File

@ -76,6 +76,8 @@ public:
void getMargin(int *result, int userMargin, QStyle::PixelMetric pm) const;
void doResize(const QSize &);
void reparentChildWidgets(QWidget *mw);
bool checkWidget(QWidget *widget) const;
bool checkLayout(QLayout *otherLayout) const;
static QWidgetItem *createWidgetItem(const QLayout *layout, QWidget *widget);
static QSpacerItem *createSpacerItem(const QLayout *layout, int w, int h, QSizePolicy::Policy hPolicy = QSizePolicy::Minimum, QSizePolicy::Policy vPolicy = QSizePolicy::Minimum);

32
tests/auto/widgets/kernel/qboxlayout/tst_qboxlayout.cpp
View File

@ -79,6 +79,8 @@ private slots:
void taskQTBUG_7103_minMaxWidthNotRespected();
void taskQTBUG_27420_takeAtShouldUnparentLayout();
void taskQTBUG_40609_addingWidgetToItsOwnLayout();
void taskQTBUG_40609_addingLayoutToItself();
void replaceWidget();
};
@ -329,6 +331,36 @@ void tst_QBoxLayout::taskQTBUG_27420_takeAtShouldUnparentLayout()
QVERIFY(!inner.isNull());
}
void tst_QBoxLayout::taskQTBUG_40609_addingWidgetToItsOwnLayout(){
QWidget widget;
widget.setObjectName("347b469225a24a0ef05150a");
QVBoxLayout layout(&widget);
layout.setObjectName("ef9e2b42298e0e6420105bb");
QTest::ignoreMessage(QtWarningMsg, "QLayout: Cannot add a null widget to QVBoxLayout/ef9e2b42298e0e6420105bb");
layout.addWidget(Q_NULLPTR);
QCOMPARE(layout.count(), 0);
QTest::ignoreMessage(QtWarningMsg, "QLayout: Cannot add parent widget QWidget/347b469225a24a0ef05150a to its child layout QVBoxLayout/ef9e2b42298e0e6420105bb");
layout.addWidget(&widget);
QCOMPARE(layout.count(), 0);
}
void tst_QBoxLayout::taskQTBUG_40609_addingLayoutToItself(){
QWidget widget;
widget.setObjectName("fe44e5cb6c08006597126a");
QVBoxLayout layout(&widget);
layout.setObjectName("cc751dd0f50f62b05a62da");
QTest::ignoreMessage(QtWarningMsg, "QLayout: Cannot add a null layout to QVBoxLayout/cc751dd0f50f62b05a62da");
layout.addLayout(Q_NULLPTR);
QCOMPARE(layout.count(), 0);
QTest::ignoreMessage(QtWarningMsg, "QLayout: Cannot add layout QVBoxLayout/cc751dd0f50f62b05a62da to itself");
layout.addLayout(&layout);
QCOMPARE(layout.count(), 0);
}
struct Descr
{
Descr(int min, int sh, int max = -1, bool exp= false, int _stretch = 0, bool _empty = false)

24
tests/auto/widgets/kernel/qformlayout/tst_qformlayout.cpp
View File

@ -135,6 +135,8 @@ private slots:
*/
void taskQTBUG_27420_takeAtShouldUnparentLayout();
void taskQTBUG_40609_addingWidgetToItsOwnLayout();
void taskQTBUG_40609_addingLayoutToItself();
};
@ -949,6 +951,28 @@ void tst_QFormLayout::taskQTBUG_27420_takeAtShouldUnparentLayout()
QVERIFY(!inner.isNull());
}
void tst_QFormLayout::taskQTBUG_40609_addingWidgetToItsOwnLayout(){
QWidget widget;
widget.setObjectName("6435cbada60548b4522cbb6");
QFormLayout layout(&widget);
layout.setObjectName("c03c0e22c0b6d019a93a248");
QTest::ignoreMessage(QtWarningMsg, "QLayout: Cannot add parent widget QWidget/6435cbada60548b4522cbb6 to its child layout QFormLayout/c03c0e22c0b6d019a93a248");
layout.addRow(QLatin1String("48c81f39b7320082f8"), &widget);
QCOMPARE(layout.count(), 0);
}
void tst_QFormLayout::taskQTBUG_40609_addingLayoutToItself(){
QWidget widget;
widget.setObjectName("2bc425637d084c07ce65956");
QFormLayout layout(&widget);
layout.setObjectName("60e31de0c8800eaba713a4f2");
QTest::ignoreMessage(QtWarningMsg, "QLayout: Cannot add layout QFormLayout/60e31de0c8800eaba713a4f2 to itself");
layout.addRow(QLatin1String("9a2cd4f40c06b489f889"), &layout);
QCOMPARE(layout.count(), 0);
}
void tst_QFormLayout::replaceWidget()
{
QWidget w;

32
tests/auto/widgets/kernel/qgridlayout/tst_qgridlayout.cpp
View File

@ -101,6 +101,8 @@ private slots:
void distributeMultiCell();
void taskQTBUG_27420_takeAtShouldUnparentLayout();
void taskQTBUG_40609_addingWidgetToItsOwnLayout();
void taskQTBUG_40609_addingLayoutToItself();
void replaceWidget();
private:
@ -1660,6 +1662,36 @@ void tst_QGridLayout::taskQTBUG_27420_takeAtShouldUnparentLayout()
QVERIFY(!inner.isNull());
}
void tst_QGridLayout::taskQTBUG_40609_addingWidgetToItsOwnLayout(){
QWidget widget;
widget.setObjectName("9bb37ca762aeb7269b8");
QGridLayout layout(&widget);
layout.setObjectName("d631e91a35f2b66a6dff35");
QTest::ignoreMessage(QtWarningMsg, "QLayout: Cannot add a null widget to QGridLayout/d631e91a35f2b66a6dff35");
layout.addWidget(Q_NULLPTR, 0, 0);
QCOMPARE(layout.count(), 0);
QTest::ignoreMessage(QtWarningMsg, "QLayout: Cannot add parent widget QWidget/9bb37ca762aeb7269b8 to its child layout QGridLayout/d631e91a35f2b66a6dff35");
layout.addWidget(&widget, 0, 0);
QCOMPARE(layout.count(), 0);
}
void tst_QGridLayout::taskQTBUG_40609_addingLayoutToItself(){
QWidget widget;
widget.setObjectName("0373d417fffe2c59c6fe543");
QGridLayout layout(&widget);
layout.setObjectName("5d79e1b0aed83f100e3c2");
QTest::ignoreMessage(QtWarningMsg, "QLayout: Cannot add a null layout to QGridLayout/5d79e1b0aed83f100e3c2");
layout.addLayout(Q_NULLPTR, 0, 0);
QCOMPARE(layout.count(), 0);
QTest::ignoreMessage(QtWarningMsg, "QLayout: Cannot add layout QGridLayout/5d79e1b0aed83f100e3c2 to itself");
layout.addLayout(&layout, 0, 0);
QCOMPARE(layout.count(), 0);
}
void tst_QGridLayout::replaceWidget()
{
QWidget wdg;

8
tests/manual/qlayout/gridwidget.cpp
View File

@ -53,6 +53,7 @@ GridWidget::GridWidget(QWidget *parent) :
QWidget(parent)
{
QGridLayout *hb = new QGridLayout(this);
hb->setObjectName("GridWidget");
QComboBox *combo = new QComboBox();
combo->addItem("123");
QComboBox *combo2 = new QComboBox();
@ -71,4 +72,11 @@ GridWidget::GridWidget(QWidget *parent) :
hb->addWidget(new QPushButton("123"), 1, 4);
hb->addWidget(new QSpinBox(), 0, 5);
hb->addWidget(new QSpinBox(), 1, 5);
qDebug("There should be four warnings, but no crash or freeze:");
hb->addWidget(this, 6, 6); ///< This command should print a warning, but should not add "this"
hb->addWidget(Q_NULLPTR, 6, 7); ///< This command should print a warning, but should not add "NULL"
hb->addLayout(hb, 7, 6); ///< This command should print a warning, but should not add "hb"
hb->addLayout(Q_NULLPTR, 7, 7); ///< This command should print a warning, but should not add "NULL"
qDebug("Neither crashed nor frozen");
}

8
tests/manual/qlayout/hbwidget.cpp
View File

@ -53,6 +53,7 @@ HbWidget::HbWidget(QWidget *parent) :
QWidget(parent)
{
QHBoxLayout *hb = new QHBoxLayout(this);
hb->setObjectName("HbWidget");
QComboBox *combo = new QComboBox(this);
combo->addItem("123");
QComboBox *combo2 = new QComboBox();
@ -67,4 +68,11 @@ HbWidget::HbWidget(QWidget *parent) :
hb->addWidget(new QDateTimeEdit());
hb->addWidget(new QPushButton("123"));
hb->addWidget(new QSpinBox());
qDebug("There should be four warnings, but no crash or freeze:");
hb->addWidget(this); ///< This command should print a warning, but should not add "this"
hb->addWidget(Q_NULLPTR); ///< This command should print a warning, but should not add "NULL"
hb->addLayout(hb); ///< This command should print a warning, but should not add "hb"
hb->addLayout(Q_NULLPTR); ///< This command should print a warning, but should not add "NULL"
qDebug("Neither crashed nor frozen");
}

8
tests/manual/qlayout/vbwidget.cpp
View File

@ -53,6 +53,7 @@ VbWidget::VbWidget(QWidget *parent) :
QWidget(parent)
{
QVBoxLayout *hb = new QVBoxLayout(this);
hb->setObjectName("VbWidget");
QComboBox *combo = new QComboBox(this);
combo->addItem("123");
QComboBox *combo2 = new QComboBox();
@ -67,4 +68,11 @@ VbWidget::VbWidget(QWidget *parent) :
hb->addWidget(new QDateTimeEdit());
hb->addWidget(new QPushButton("123"));
hb->addWidget(new QSpinBox());
qDebug("There should be four warnings, but no crash or freeze:");
hb->addWidget(this); ///< This command should print a warning, but should not add "this"
hb->addWidget(Q_NULLPTR); ///< This command should print a warning, but should not add "NULL"
hb->addLayout(hb); ///< This command should print a warning, but should not add "hb"
hb->addLayout(Q_NULLPTR); ///< This command should print a warning, but should not add "NULL"
qDebug("Neither crashed nor frozen");
}