Tips on memory management with C++ and Qt

[Alex Merry]

On Monday 16 February 2009 10:59:32 Maximilian Kossick wrote:
> On Mon, Feb 16, 2009 at 8:40 AM, Mark Kretschmann <kretschmann at kde.org> 
wrote:
> > 1)
> > Use of "Smart Pointers":
> > A smart pointer in C++ means (in its most simple incarnation) a
> > pointer that automatically sets itself to 0, when the object it points
> > to is being destroyed (deleted).

But beware that they don't stop you derefencing a null-pointer.

> >
> >
> > 3)
> > Never, ever, use private d-pointer classes in QObject derived subclasses:
> >
> > What can happen is that you do a "delete d;" in your destructor, and
> > then Qt goes ahead and auto-deletes other QObject pointers contained
> > in the private class again, through means of its automatic deleting of
> > QObjects with a parent Object. -> <BOOOOM>
> >
> > Read more about this topic in Michael Pyne's interesting blog article:
> >
> > http://www.purinchu.net/wp/2009/02/04/another-programming-tidbit/
>
> This is not correct. Using d-pointers in QObjects is perfectly fine as
> long as the destructors are written correctly. The referenced article
> only says that one has to be careful about deleting a private
> d-pointer when other classes keep pointers to members of the private
> class around.

For Amarok, it's a very good rule.  As far as I'm aware, there are no parts of 
Amarok that have binary compatibility guarantees, and so there are no parts of 
Amarok that have any use for d-pointers.

Since a common cause of memory leaks is people forgetting to delete d-
pointers, I would suggest not ever using d-pointers in Amarok.  The reason for 
their existance in kdelibs, for example, is that it helps maintain binary 
compatibility.  If you don't need to maintain binary compatility, don't use d-
pointers.

>
> > 4)
> > Use Valgrind:
> >
> > This is one of the most advanced memory debugging tools available,
> > it's free, and we even have found volunteers that run regular Valgrind
> > checks (both for memory access bugs and memory leaks) on Amarok trunk.
> > Reading the Valgrind logs correctly is a bit of an art in itself, but
> > I'm willing to explain this in another posting, if there is a demand.

We need a script to clean up Valgrind logs.  For example, anything that 
references libasound or libxine in the backtrace can be discarded, since 
there's nothing we can do about such things (and they appear to make up the 
majority of the multiple-megabyte logs).

I will try to do this at some point, but if someone beats me to it, I won't 
complain.


5) Always initialize member variables.  Or, indeed, variables generally.

Actually, this is not so hard-and-fast.  Inline objects (as opposed to 
pointers-to-objects) don't need explicitly initialising if you're happy with 
their default value (and they have default constructors).  For example

QString m_foo;

doesn't necessarily need any initialization logic, since the default 
constructor will be called by the compiler, giving you an empty string.  But

QWidget* m_bar;

_should_ be set to 0 or set to point to a valid object as early as possible in 
the constructor.  This way you can safely free the object, and you can check 
for whether the pointer is 0 later.

Similarly for simple types.  Booleans and numeric types should be set to false 
and 0 respectively, or another sensible value.  At some point they will be 
used in a test or assignment or as an argument, and if they haven't been 
initialized, then it's anybody's guess what will happen.  And unpredictable 
bugs are the worst ones to debug.

class Foo
{
	QString m_bar; // doesn't need explicitly initialising
	bool m_test;
	int m_number;
	float m_float;
	QWidget* m_widget;
	QWidget* m_anotherWidget;

	Foo( float input )
		: m_test( false ),
		  m_number( 0 ),
		  m_float( input ),
		  m_widget( 0 ),
		  m_anotherWidget( new QWidget )
	{
	}
}

Note that the above is my preferred style of member variable initialization.  
Putting assignments in the constructor body itself is perfectly valid, but by 
the time the constructor has finished, _every_ member variable should be 
initialized (whether implicitly by the compiler, in the case of inline 
objects, or explicitly, in the case of everything else).


Alex


-- 
Why have I got six monitors?  Because I haven't got room for eight.
  -- Terry Pratchett

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/amarok-devel/attachments/20090216/b8dfbfba/attachment.sig