-1

I try to learn more about programming by studying open source using UML. The code I have found is in the stage between Qt 3 and Qt 4. The project is not that active so I ask this question here. Maybe I should add that the program using this code do run.

Note, I am a junior. I ask because I want to learn.

My question is simple:
Do this code leak memory?

If not, why ?

  void warn(const QString & s) {

      // not showed dialog to compute needed size

      QDialog d_aux;
      Q3VBoxLayout * vbox_aux = new Q3VBoxLayout(&d_aux);

      vbox_aux->setMargin(5);
      Q3TextEdit * e = new Q3TextEdit(&d_aux);
      e->setText(s);

      // showed dialog

      QDialog * d = new QDialog;

      d->setCaption("My caption");

      Q3VBoxLayout * vbox = new Q3VBoxLayout(d);

      vbox->setMargin(5);

      Q3TextView * t = new Q3TextView(d);
      QFontMetrics fm(QApplication::font());
      int maxw = (MyWindow::get_workspace()->width() * 4) / 5;
      int maxh = (MyWindow::get_workspace()->height() * 4) / 5;
      int he = (e->lines() + 5) * fm.height();

      t->setText(s);
      t->setMinimumSize(maxw, (he > maxh) ? maxh : he);

      vbox->addWidget(t);

      d->show();
  }

Thanks // JG

László Papp
  • 51,870
  • 39
  • 111
  • 135
Jim G
  • 43
  • 1
  • 3
    It certainly looks like it does. There are 5 `new` expressions and not a single `delete`. The memory allocated might be deallocated somewhere else, but from the code shown it's not obvious where (if at all). – jrok May 04 '14 at 13:47
  • 1
    @jrok: Then again it's Qt and everything implicitly takes ownership all over the place. I'd pay for Qt++11... – Kerrek SB May 04 '14 at 13:49
  • @Kerrek. Indeed. After a few second in Qt docs, it appears that `QDialog d_aux` is the parent of `vbox_aux` and it disposes of it later. But `d` does look like it leaks (taking `t` and `vbox` with it). – jrok May 04 '14 at 13:50
  • Thanks for your quick answers I am new to Qt and I supected Qt did something magical / fishy behind the scene. But still, I was almost certain this code was leaking. Now I know. I will mark this code as leaky in my UML diagram and move on. A nugget for future studies in other words. Thanks !! – Jim G May 04 '14 at 14:56
  • @KerrekSB: there has been Qt++11 for ages. ;) – László Papp May 04 '14 at 18:32
  • @LaszloPapp: Where what? A Qt with move semantics and unique pointers? – Kerrek SB May 04 '14 at 18:53
  • @KerrekSB: sure, can send my bank account number in private... :) – László Papp May 04 '14 at 18:55

2 Answers2

3

You have one: The QDialog * d = new QDialog; The other pointers got a parent instance which takes ownership of the pointers. (Please confirm it in the documentation of Q3VBoxLayout, Q3TextEdit and Q3TextView)

1

I think you are leaking memory, but it would be simple to verify with a program like valgrind on Linux and so on. Let us see what dynamic memory allocations you do in your function:

  Q3VBoxLayout * vbox_aux = new Q3VBoxLayout(&d_aux);

That is alright because it gets a parent. Let us see the next:

  Q3TextEdit * e = new Q3TextEdit(&d_aux);

That is also alright for the same reason above. Let us see the next:

  QDialog * d = new QDialog;

Here, you problems begin to arise because the dialog does not have a parent. You have several ways of fixing it.

1) Assign a parent, although this might not be ideal in this case as you do not seem to have any parent'ish look in your code for this widget. That is, nothing that could really become the parent of it unlike your Qt application if you are using that. This fix may require some major rework depending on the whole context that you have shown.

2) Use a smart pointer, e.g. QPointer around it, so it will be managed for you automatically. That should have been available at the age of the code you are using. This may also need some code rework depending on more context that you have not provided.

Let us see the next dynamic memory allocation:

  Q3VBoxLayout * vbox = new Q3VBoxLayout(d);

That is alright for the reasons mentioned before. Let us see the next and last:

  Q3TextView * t = new Q3TextView(d);

That is also alright for the reasons mentioned before.

László Papp
  • 51,870
  • 39
  • 111
  • 135
  • I give this answer the _green stamp_ thanks to the level of detail and suggested answers. Context is mentioned a few times. I found this function in a file named _myio.cpp_ which mostly deals with parsing files and stringify enums. A bit oddish I think, will have to dig deeper in order to find out why. Analysing code with UML start to be really fun... – Jim G May 06 '14 at 17:00
  • @JimG: ok, np. Not sure why your answer got so downvoted. IMHO, it is a valid question. – László Papp May 06 '14 at 17:13