Bob
Bob

Reputation: 1455

Does this code provide memory leaks?

Finally i have installed Ubuntu and set up Qt+Valgrind to prevent memory leaks which i could not do in Windows. So i can not understand does this code provide a memory leak? In fact that Valgrind says i have only 500+ issues but nothing about the leak. I

#include <QWidget>
#include <QFrame>
#include <QVBoxLayout>
#include <QApplication>

int main(int argc, char *argv[])

{
    QApplication a(argc, argv);

    QWidget * wdgt = new QWidget;  //this line should be the cause of leakage 
                                   //if it exist (as far as i know)
    QVBoxLayout *layout = new QVBoxLayout;
    QFrame * frame = new QFrame;

    frame->setFrameStyle(QFrame::Panel | QFrame::Plain);
    frame->setLineWidth(5);
    layout->addWidget(frame);

    wdgt->setLayout(layout);
    wdgt->setFixedSize(800,600);
    wdgt->show();

    return a.exec();
}

Upvotes: 5

Views: 1189

Answers (3)

jpo38
jpo38

Reputation: 21504

See this post: Creating and deallocating a Qt widget object

It explaines that if one Qt object has a parent, it will be automatically deleted when the parent is destroyed.

In your code:

  • wdgt is the parent of layout because you did wdgt->setLayout(layout).
  • wdgt is the parent of frame because you did layout->addWidget(frame) and layout's parent is wdgt. As commented by thuga, layout pass the ownership to their own parent.

In your code, only wdgt is orphan (no Qt parent to delete it automatically).

To fix this, you can either give him a parent:

QWidget * wdgt = new QWidget(&app);

So that wdgt is a child of app and will then be deleted automatically when app is destroyed.

or delete it yourself:

int main(int argc, char *argv[])
{
    ...
    int res = a.exec();
    delete wdgt; // this will delete wdgt, but also frame and layout
    return res;
}

or, fianlly, create it as an object, so that it is deleted when going out of scope:

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);

    QWidget wdgt;

    QVBoxLayout *layout = new QVBoxLayout;
    QFrame * frame = new QFrame;

    frame->setFrameStyle(QFrame::Panel | QFrame::Plain);
    frame->setLineWidth(5);
    layout->addWidget(frame);

    wdgt.setLayout(layout);
    wdgt.setFixedSize(800,600);
    wdgt.show();

    return a.exec();
}

By the way, note that if you do QVBoxLayout *layout = new QVBoxLayout(wdgt), there is no need to do wdgt->setLayout(layout). So those two piece of codes are equivalent:

QVBoxLayout *layout = new QVBoxLayout(wdgt); // parenting upon construction

is the same as:

QVBoxLayout *layout = new QVBoxLayout; // no parent
wdgt->setLayout( layout ); // reparenting

Upvotes: 9

Your code leaks memory, but first of all you shouldn't write code where you even have to care about resources leaking. Let the compiler handle it for you:

// main.cpp
#include <QtWidgets>

int main(int argc, char *argv[]) {
    QApplication a(argc, argv);
    QWidget widget;
    QVBoxLayout layout(&widget);
    QFrame frame;

    frame.setFrameStyle(QFrame::Panel | QFrame::Plain);
    frame.setLineWidth(5);
    layout.addWidget(&frame);

    widget.setFixedSize(800,600);
    widget.show();
    return a.exec();
}

Upvotes: 0

Simon Warta
Simon Warta

Reputation: 11408

Yes your Code leaks memory because you create objects using new without using Qt's memory management.

Go for

QApplication a(argc, argv);
QWidget * wdgt = new QWidget(&app);
QVBoxLayout *layout = new QVBoxLayout(wdgt); // optional, setLayout does that
QFrame * frame = new QFrame(layout); // optional, addWidget does that

to use Qt's memory management.


Alternatively you can use C++11 shared pointers:

QApplication a(argc, argv);
std::shared_ptr<QWidget> wdgt = std::make_shared<QWidget>();

QVBoxLayout *layout = new QVBoxLayout;
QFrame * frame = new QFrame;

As soon as the last user of a shared pointer goes out of scope, your objects are deleted automatically.

Upvotes: 0

Related Questions