pointer initialization in ctor

Discussion in 'C++' started by Grahamo@nospam.com, Dec 8, 2005.

  1. Guest

    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
    , Dec 8, 2005
    #1
    1. Advertising

  2. * :
    >
    > 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.

    --
    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?
    Alf P. Steinbach, Dec 8, 2005
    #2
    1. Advertising

  3. Kai-Uwe Bux Guest

    wrote:

    > 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
    Kai-Uwe Bux, Dec 8, 2005
    #3
  4. On Thu, 08 Dec 2005 06:40:54 -0500, Kai-Uwe Bux <>
    wrote:

    >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.

    --
    Bob Hairgrove
    Bob Hairgrove, Dec 8, 2005
    #4
  5. mlimber Guest

    Bob Hairgrove wrote:
    > On Thu, 08 Dec 2005 06:40:54 -0500, Kai-Uwe Bux <>
    > wrote:
    >
    > >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.


    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
    mlimber, Dec 8, 2005
    #5
  6. Kai-Uwe Bux Guest

    mlimber wrote:

    >
    > Bob Hairgrove wrote:
    >> On Thu, 08 Dec 2005 06:40:54 -0500, Kai-Uwe Bux <>
    >> wrote:
    >>
    >> >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.

    >
    > 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
    Kai-Uwe Bux, Dec 8, 2005
    #6
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Apricot
    Replies:
    4
    Views:
    510
    velthuijsen
    Apr 16, 2004
  2. Matthias Kaeppler
    Replies:
    2
    Views:
    424
    Victor Bazarov
    Jul 18, 2005
  3. NVH
    Replies:
    8
    Views:
    477
    mlimber
    Jul 6, 2006
  4. Anonymous
    Replies:
    2
    Views:
    370
    Victor Bazarov
    Aug 28, 2007
  5. Anonymous
    Replies:
    2
    Views:
    332
    Barry
    Aug 29, 2007
Loading...

Share This Page