pointer initialization in ctor

[Grahamo@nospam.com]

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]

Re: pointer initialization in ctor

* Grahamo@nospam. com:[color=blue]
>
> I have a basic question regarding some legacy code I'm working with;
>[/color]
[snip][color=blue]
> 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.[/color]

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

[color=blue]
> 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[/color]

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.

[color=blue]
> but it doesn't leak
> and it does add memory resource concerns to the class, does it?[/color]

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.

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

[Kai-Uwe Bux]

Re: pointer initialization in ctor

Grahamo@nospam. com wrote:
[color=blue]
> 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.[/color]

I would go for initialization:

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

[color=blue]
> 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?[/color]

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]

Re: pointer initialization in ctor

On Thu, 08 Dec 2005 06:40:54 -0500, Kai-Uwe Bux <jkherciueh@gmx .net>
wrote:
[color=blue]
>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.[/color]

You are quite correct.

--
Bob Hairgrove
NoSpamPlease@Ho me.com

[mlimber]

Re: pointer initialization in ctor


Bob Hairgrove wrote:[color=blue]
> On Thu, 08 Dec 2005 06:40:54 -0500, Kai-Uwe Bux <jkherciueh@gmx .net>
> wrote:
>[color=green]
> >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.[/color]
>
> You are quite correct.[/color]

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]

Re: pointer initialization in ctor

mlimber wrote:
[color=blue]
>
> Bob Hairgrove wrote:[color=green]
>> On Thu, 08 Dec 2005 06:40:54 -0500, Kai-Uwe Bux <jkherciueh@gmx .net>
>> wrote:
>>[color=darkred]
>> >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.[/color]
>>
>> You are quite correct.[/color]
>
> 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.[/color]

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