CODEWITHSUNDEEP
c++qtpointers
Marco81
Marco81

Reputation: 151

QT: addLayout causes crash

I was trying to rewrite a simple "find dialog" example without using pointer as the class members. I cannot figure out why this code causes crash of this example

//finddialog.h    
#ifndef FINDDIALOG_H
#define FINDDIALOG_H

#include<QtWidgets>
//#include <QDialog>


class FindDialog : public QDialog
{
    Q_OBJECT

public:
    FindDialog(QWidget *parent = 0);
    ~FindDialog();

signals:
    void findNext(const QString &str, Qt::CaseSensitivity cs);
    void findPrevious(const QString &str, Qt::CaseSensitivity cs);

private slots:
    void findClicked();
    void enablefindButton(const QString &text);
private:
    QLabel label{tr("Find &what:")};
    QLineEdit lineEdit;
    QCheckBox caseCheckBox{tr("Match &case")};
    QCheckBox backwardCheckBox{tr("Search &bacward")};
    QPushButton findButton{tr("&Find")};
    QPushButton closeButton{tr("&Close")};

};

#endif // FINDDIALOG_H

This is the implementation

//finddialog.cpp    
#include <QtWidgets>
#include "finddialog.h"

FindDialog::FindDialog(QWidget *parent)
    : QDialog(parent)
{

    label.setBuddy(&lineEdit);

    findButton.setDefault(true);
    findButton.setEnabled(false);

    connect(&lineEdit, SIGNAL(textChanged(const QString &)),
            this, SLOT(enablefindButton(const QString &)));
    connect(&findButton, SIGNAL(clicked()),
            this, SLOT(findClicked()));
    connect(&closeButton, SIGNAL(clicked()),
            this, SLOT(close()));

    QHBoxLayout topLeftLayout;
    topLeftLayout.addWidget(&label);
    topLeftLayout.addWidget(&lineEdit);

    QVBoxLayout leftLayout;
    leftLayout.addLayout(&topLeftLayout);
    leftLayout.addWidget(&caseCheckBox);
    leftLayout.addWidget(&backwardCheckBox);

    QVBoxLayout rightLayout;
    rightLayout.addWidget(&findButton);
    rightLayout.addWidget(&closeButton);
    rightLayout.addStretch();

    QHBoxLayout mainLayout{this};
    mainLayout.addLayout(&leftLayout);
    mainLayout.addLayout(&rightLayout);
    setLayout(&mainLayout);

    setWindowTitle(tr("Find"));
    setFixedHeight(sizeHint().height());
}

FindDialog::~FindDialog()
{

}

void FindDialog::findClicked()
{
    QString text = lineEdit.text();
    Qt::CaseSensitivity cs = caseCheckBox.isChecked() ? Qt::CaseSensitive : Qt::CaseInsensitive;

    if (backwardCheckBox.isChecked())
        emit findPrevious(text,cs);
    else
        emit findNext(text,cs);
}

void FindDialog::enablefindButton(const QString &text)
{
    findButton.setEnabled(!text.isEmpty());
}

And the main is simply like this 

//main.cpp
#include <QApplication>
#include "finddialog.h"
int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    FindDialog w;
    w.show();

    return a.exec();
}

I've found the problem is with the methods 'addLyout', i.e. commenting all the call to this method, the application does not crash. From the documentation the method require a pointer to a QLayout as input, so I've passed the address of each layout, but I am missing something. May you help me figuring out what is going on?

Upvotes: 0

Views: 600

Answers (1)

Nikos C.
Nikos C.

Reputation: 51832

Your layout instances are stack variables, so they get destroyed after the constructor returns. Other objects now have pointers to instances that no longer exist. This usually means a crash.

Make your layout instances private members of FindDialog to prevent that from happening.

Note that when creating QObject instances on the stack, you need to be careful about declaration order. When a QObject gets destroyed, it will delete all its child objects, unless they have already been destroyed. That means you need to make sure your widgets get destroyed before their layouts do. (This is true for all QObjects with a parent/child relationship, not just layouts.) If you don't pay attention to this, parents will try to delete children which are on the stack (which is not allowed, obviously.)

The destruction order of class members is the reverse of the construction order. So members declared last get destroyed first. In the case of QLayout subclasses, they need to be destroyed after their child widgets. So you need to declare them first:

private:
    QHBoxLayout topLeftLayout;
    QVBoxLayout leftLayout;
    QVBoxLayout rightLayout;
    QHBoxLayout mainLayout{this};

    QLabel label{tr("Find &what:")};
    QLineEdit lineEdit;
    QCheckBox caseCheckBox{tr("Match &case")};
    QCheckBox backwardCheckBox{tr("Search &bacward")};
    QPushButton findButton{tr("&Find")};
    QPushButton closeButton{tr("&Close")};

This is one of the reasons why most Qt code you see out there is allocating QObjects on the heap (with new) instead. You won't have to worry about this issue when putting them on the heap. Obviously you can then run into memory leaks when having parentless objects that never get deleted. So choose your poison.

Upvotes: 1

Related Questions