pointer initialization in ctor

[Grahamo]

Hi,

I have a basic question regarding some legacy code I'm working with;

Basically the code looks something like this. I'd like to know if there
are any reasons why a particular approach is taken.

Given a type X we have a class something like this;

class foo
{
public:

foo (X* x)
{
m_x = new X(); /// my question is about these 2 lines.
*m_x = *x;
}

~foo()
{
delete m_x;
}

protected:

X* m_x;

};



Now in the foo ctor above, I would have personally done something like;

foo(X* x)
{
m_x = new X(*x);
}

thereby avoiding the overhead of the default ctor call and then the
assignment, we would just use the copy ctor.

I am asking this question in the context of memory leaks. There's
nothing wrong, in the non-purist sense of the word, with the original
code. I mean it's not as efficient as it could be but it doesn't leak
and it does add memory resource concerns to the class, does it?


thanks and have a nice day

Graham


thanks and have a nice day

G

 

[Alf P. Steinbach]

* (e-mail address removed):

I have a basic question regarding some legacy code I'm working with;
[snip]
class foo
{
public:

foo (X* x)
{
m_x = new X(); /// my question is about these 2 lines.
*m_x = *x;
}

~foo()
{
delete m_x;
}

protected:

X* m_x;

};



Now in the foo ctor above, I would have personally done something like;

foo(X* x)
{
m_x = new X(*x);
}

thereby avoiding the overhead of the default ctor call and then the
assignment, we would just use the copy ctor.

There's probably no other reason than that the programmer didn't understand
constructors.

I am asking this question in the context of memory leaks. There's
nothing wrong, in the non-purist sense of the word, with the original
code. I mean it's not as efficient as it could be

Nope; unless there is some compelling reason to use dynamic allocation
the X object should just be a direct member, not accessed via a pointer.

but it doesn't leak
and it does add memory resource concerns to the class, does it?

To avoid dangling pointers the class needs a user-defined copy constructor
and an assignment operator, or alternatively, disabling these operations, or,
replace the raw pointer member with a smart pointer.

 

[Kai-Uwe Bux]

Hi,

I have a basic question regarding some legacy code I'm working with;

Basically the code looks something like this. I'd like to know if there
are any reasons why a particular approach is taken.

Given a type X we have a class something like this;

class foo
{
public:

foo (X* x)
{
m_x = new X(); /// my question is about these 2 lines.
*m_x = *x;
}

~foo()
{
delete m_x;
}

protected:

X* m_x;

};



Now in the foo ctor above, I would have personally done something like;

foo(X* x)
{
m_x = new X(*x);
}

thereby avoiding the overhead of the default ctor call and then the
assignment, we would just use the copy ctor.

I would go for initialization:

foo ( X* x )
: m_x ( new X ( *x ) )
{}

I am asking this question in the context of memory leaks. There's
nothing wrong, in the non-purist sense of the word, with the original
code. I mean it's not as efficient as it could be but it doesn't leak
and it does add memory resource concerns to the class, does it?

Well, it could leak. Look at the code:

m_x = new X(); // line 1
*m_x = *x; // line 2

Now, if the assignment in line 2 throws an exception, the m_x pointer will
not be deleted since (I might be wrong and I am too lazy to check the
standard right now) the destructor will not be called upon disposal of a
not completely constructed object but only the destructors for the already
constructed members. Thus, when a constructor throws, it has to clean up
the mess by itself.


Best

Kai-Uwe Bux

 

[Bob Hairgrove]

Well, it could leak. Look at the code:

m_x = new X(); // line 1
*m_x = *x; // line 2

Now, if the assignment in line 2 throws an exception, the m_x pointer will
not be deleted since (I might be wrong and I am too lazy to check the
standard right now) the destructor will not be called upon disposal of a
not completely constructed object but only the destructors for the already
constructed members. Thus, when a constructor throws, it has to clean up
the mess by itself.

You are quite correct.

 

[mlimber]

You are quite correct.

And that's where Alf's suggestion of using a smart pointer becomes not
just a good idea but an absolute necessity to avoid memory leaks. (Of
course, using a non-pointer member of type X would also do the trick.)

Cheers! --M

 

[Kai-Uwe Bux]

And that's where Alf's suggestion of using a smart pointer becomes not
just a good idea but an absolute necessity to avoid memory leaks.

Actually this is not *where* smart pointers become a necessity. The
constructor

foo ( X* x )
: m_x ( new X ( *x ) )
{}

is exception safe whether m_x is a smart or a dumb pointer. Life (aside from
the operator= and copy constructor issues) becomes tricky, when you have
*two* members that might throw:

foo ( whatever )
: m_ptr_one ( new X ( something ) )
, m_ptr_two ( new Y ( something_else ) ) // bad line
{}

will leak m_ptr_one if the bad line throws.


However, it might be worthwile to note that an innocent looking line like

*ptr = some_value;

may throw and can leak memory if the pointer object ptr is destroyed during
stack unwinding.


Anyway, I am with you folks that raw pointers should not rear their ugly
head into code without reason.


Best

Kai-Uwe Bux