Weird leak at operator=

Discussion in 'C++' started by Lucas Kanebley Tavares, Jul 24, 2008.

  1. Hello all,

    I have a templatized class which has an attribute as: "T *data", all
    constructors initialize it to zero, and then allocate memory for the
    array (and that IS done correctly, I've checked).

    What I find it strange, is that at one point I have to reallocate the
    data, but if I put this in my operator= overload:

    if (data != 0) delete data; //mem leak here
    data = new T[get_data_size()];

    valgrind acuses a memory leak, but I've watched my destructor and
    unless I do this there, that memory block is NEVER deallocated (and no
    it's not a self-assignment issue, that's taken care of, and the other
    object is not the same).

    I wonder what is going on. Any thoughts?

    Thanks!

    Lucas Kanebley Tavares

    PS: This is more of a curiosity, to me the program seems fine, and it
    has never crashed before, so I take it the deallocation is correct...
    just weird...
    Lucas Kanebley Tavares, Jul 24, 2008
    #1
    1. Advertising

  2. On Jul 24, 2:32 pm, Lucas Kanebley Tavares <> wrote:
    > Hello all,
    >
    > I have a templatized class which has an attribute as: "T *data", all
    > constructors initialize it to zero, and then allocate memory for the
    > array (and that IS done correctly, I've checked).
    >
    > What I find it strange, is that at one point I have to reallocate the
    > data, but if I put this in my operator= overload:
    >
    > if (data != 0) delete data; //mem leak here
    > data = new T[get_data_size()];
    >
    > valgrind acuses a memory leak, but I've watched my destructor and
    > unless I do this there, that memory block is NEVER deallocated (and no
    > it's not a self-assignment issue, that's taken care of, and the other
    > object is not the same).
    >
    > I wonder what is going on. Any thoughts?
    >
    > Thanks!
    >
    > Lucas Kanebley Tavares
    >
    > PS: This is more of a curiosity, to me the program seems fine, and it
    > has never crashed before, so I take it the deallocation is correct...
    > just weird...


    Nevermind, sorry to bother you guys, after hours looking at it, I
    stepped away for a minute and saw the missing brackets.. *sigh*
    Thank you all, and sorry about the inconvenience.

    Lucas Kanebley Tavares
    Lucas Kanebley Tavares, Jul 24, 2008
    #2
    1. Advertising

  3. Lucas Kanebley Tavares schrieb:
    > On Jul 24, 2:32 pm, Lucas Kanebley Tavares <> wrote:
    >> I have a templatized class which has an attribute as: "T *data", all
    >> constructors initialize it to zero, and then allocate memory for the
    >> array (and that IS done correctly, I've checked).
    >>
    >> What I find it strange, is that at one point I have to reallocate the
    >> data, but if I put this in my operator= overload:
    >>
    >> if (data != 0) delete data; //mem leak here


    You don't need to check for 0, because deletion of a null-pointer is a
    no-op.

    >> data = new T[get_data_size()];
    >>
    >> valgrind acuses a memory leak, [...]

    >
    > Nevermind, sorry to bother you guys, after hours looking at it, I
    > stepped away for a minute and saw the missing brackets.. *sigh*
    > Thank you all, and sorry about the inconvenience.


    You could use a std::vector<T> instead of a T*, so your class would
    automatically be exception safe, copyable and assignable etc. without a
    need to implement operator= and the copy constructor yourself.

    --
    Thomas
    Thomas J. Gritzan, Jul 24, 2008
    #3
  4. On Jul 24, 3:45 pm, "Thomas J. Gritzan" <>
    wrote:
    > Lucas Kanebley Tavares schrieb:
    >
    > > On Jul 24, 2:32 pm, Lucas Kanebley Tavares <> wrote:
    > >> I have a templatized class which has an attribute as: "T *data", all
    > >> constructors initialize it to zero, and then allocate memory for the
    > >> array (and that IS done correctly, I've checked).

    >
    > >> What I find it strange, is that at one point I have to reallocate the
    > >> data, but if I put this in my operator= overload:

    >
    > >> if (data != 0) delete data; //mem leak here

    >
    > You don't need to check for 0, because deletion of a null-pointer is a
    > no-op.
    >
    > >> data = new T[get_data_size()];

    >
    > >> valgrind acuses a memory leak, [...]

    >
    > > Nevermind, sorry to bother you guys, after hours looking at it, I
    > > stepped away for a minute and saw the missing brackets.. *sigh*
    > > Thank you all, and sorry about the inconvenience.

    >
    > You could use a std::vector<T> instead of a T*, so your class would
    > automatically be exception safe, copyable and assignable etc. without a
    > need to implement operator= and the copy constructor yourself.
    >
    > --
    > Thomas


    Thanks for the tip, but it's actually an implementation of a "low-
    diagonal" matrix, with a weird growth rate, so that wouldn't work.
    Well, I guess it would... but still... =/

    Let me clarify. This is what the matrix is supposed to look like:
    0
    0 0
    0 0 0
    0 0 0 0
    Only the value below the main diagonal are used. But it's not only
    that, it could grow at a rate of 2 (or any number of) columns per row
    (determined upon allocation), as such:
    0
    0 0 0
    0 0 0 0 0
    0 0 0 0 0 0 0
    ...
    So, an implementation using vector would get boring FAST. heh

    Thanks for the tip, though!

    Lucas
    Lucas Kanebley Tavares, Jul 24, 2008
    #4
  5. Lucas Kanebley Tavares

    Guest

    On Jul 24, 11:45 am, "Thomas J. Gritzan" <>
    wrote:

    > You could use a std::vector<T> instead of a T*, so your class would
    > automatically be exception safe, copyable and assignable etc. without a
    > need to implement operator= and the copy constructor yourself.


    If that vector is the only member, yes it would be exception safe.
    Introduce a new member and now the compiler generated assignment
    operator is not exception safe. :(

    The reason is, it recursively assigns bases and members and can stop
    early due to an exception, at which point we would have a partially-
    assigned object at hand.

    Guideline #N - Observe The Rule of The Big One: Either write the
    canonical form of the assignment operator (the one that uses the swap
    idiom), or declare the assignment operator private.

    Ali
    , Jul 25, 2008
    #5
  6. Lucas Kanebley Tavares

    Lionel B Guest

    On Thu, 24 Jul 2008 12:35:25 -0700, Lucas Kanebley Tavares wrote:

    > On Jul 24, 3:45 pm, "Thomas J. Gritzan" <> wrote:
    >> Lucas Kanebley Tavares schrieb:
    >>
    >> > On Jul 24, 2:32 pm, Lucas Kanebley Tavares <> wrote:
    >> >> I have a templatized class which has an attribute as: "T *data", all
    >> >> constructors initialize it to zero, and then allocate memory for the
    >> >> array (and that IS done correctly, I've checked).

    >>
    >> >> What I find it strange, is that at one point I have to reallocate
    >> >> the data, but if I put this in my operator= overload:

    >>
    >> >> if (data != 0) delete data; //mem leak here

    >>
    >> You don't need to check for 0, because deletion of a null-pointer is a
    >> no-op.
    >>
    >> >> data = new T[get_data_size()];

    >>
    >> >> valgrind acuses a memory leak, [...]

    >>
    >> > Nevermind, sorry to bother you guys, after hours looking at it, I
    >> > stepped away for a minute and saw the missing brackets.. *sigh* Thank
    >> > you all, and sorry about the inconvenience.

    >>
    >> You could use a std::vector<T> instead of a T*, so your class would
    >> automatically be exception safe, copyable and assignable etc. without a
    >> need to implement operator= and the copy constructor yourself.

    >
    > Thanks for the tip, but it's actually an implementation of a "low-
    > diagonal" matrix, with a weird growth rate, so that wouldn't work. Well,
    > I guess it would... but still... =/


    Are you aware that std::vector has a `resize()' member that will, if
    necessary, do pretty much what your code does: delete and reallocate if
    the vector has to grow? It probably also has a much cleverer memory
    allocation scheme than you are likely to implement yourself.

    > Let me clarify. This is what the matrix is supposed to look like: 0
    > 0 0
    > 0 0 0
    > 0 0 0 0
    > Only the value below the main diagonal are used. But it's not only that,
    > it could grow at a rate of 2 (or any number of) columns per row
    > (determined upon allocation), as such: 0
    > 0 0 0
    > 0 0 0 0 0
    > 0 0 0 0 0 0 0
    > ...
    > So, an implementation using vector would get boring FAST. heh


    Boring?!? If boring == safer, easier to implement/maintain and quite
    possibly more efficient, then yes. If you prefer "interesting"* then
    carry on as you were ;-)

    * http://en.wikipedia.org/wiki/May_you_live_in_interesting_times

    --
    Lionel B
    Lionel B, Jul 25, 2008
    #6
  7. On Jul 25, 6:02 am, Lionel B <> wrote:
    > On Thu, 24 Jul 2008 12:35:25 -0700, Lucas Kanebley Tavares wrote:
    > > On Jul 24, 3:45 pm, "Thomas J. Gritzan" <> wrote:
    > >> Lucas Kanebley Tavares schrieb:

    >
    > >> > On Jul 24, 2:32 pm, Lucas Kanebley Tavares <> wrote:
    > >> >> I have a templatized class which has an attribute as: "T *data", all
    > >> >> constructors initialize it to zero, and then allocate memory for the
    > >> >> array (and that IS done correctly, I've checked).

    >
    > >> >> What I find it strange, is that at one point I have to reallocate
    > >> >> the data, but if I put this in my operator= overload:

    >
    > >> >> if (data != 0) delete data; //mem leak here

    >
    > >> You don't need to check for 0, because deletion of a null-pointer is a
    > >> no-op.

    >
    > >> >> data = new T[get_data_size()];

    >
    > >> >> valgrind acuses a memory leak, [...]

    >
    > >> > Nevermind, sorry to bother you guys, after hours looking at it, I
    > >> > stepped away for a minute and saw the missing brackets.. *sigh* Thank
    > >> > you all, and sorry about the inconvenience.

    >
    > >> You could use a std::vector<T> instead of a T*, so your class would
    > >> automatically be exception safe, copyable and assignable etc. without a
    > >> need to implement operator= and the copy constructor yourself.

    >
    > > Thanks for the tip, but it's actually an implementation of a "low-
    > > diagonal" matrix, with a weird growth rate, so that wouldn't work. Well,
    > > I guess it would... but still... =/

    >
    > Are you aware that std::vector has a `resize()' member that will, if
    > necessary, do pretty much what your code does: delete and reallocate if
    > the vector has to grow? It probably also has a much cleverer memory
    > allocation scheme than you are likely to implement yourself.
    >
    > > Let me clarify. This is what the matrix is supposed to look like: 0
    > > 0 0
    > > 0 0 0
    > > 0 0 0 0
    > > Only the value below the main diagonal are used. But it's not only that,
    > > it could grow at a rate of 2 (or any number of) columns per row
    > > (determined upon allocation), as such: 0
    > > 0 0 0
    > > 0 0 0 0 0
    > > 0 0 0 0 0 0 0
    > > ...
    > > So, an implementation using vector would get boring FAST. heh

    >
    > Boring?!? If boring == safer, easier to implement/maintain and quite
    > possibly more efficient, then yes. If you prefer "interesting"* then
    > carry on as you were ;-)
    >
    > *http://en.wikipedia.org/wiki/May_you_live_in_interesting_times
    >
    > --
    > Lionel B


    LOL @ interesting life ;)

    Yes, I'm familiar with resize(), I perhaps should've been clearer in
    what I said.

    What I meant from "boring" is that debugging STL is CRAP. I work with
    it a lot when I need fast prototypes for things, or when I'm building
    something that I believe won't cause me trouble. But the classes that
    are to be "within" the matrix are the core of my program
    (implementation and prototypes for several dynamic programming
    algorithms for lot sizing problems) and I need to have a debugger
    handy, which STL just can't deliver.

    Thanks
    Lucas Kanebley Tavares
    Lucas Kanebley Tavares, Jul 26, 2008
    #7
    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. Richard Heathfield

    Leak or no leak ??

    Richard Heathfield, Jul 10, 2006, in forum: C Programming
    Replies:
    4
    Views:
    341
    Richard Heathfield
    Jul 10, 2006
  2. dorayme
    Replies:
    1
    Views:
    605
    richard
    Jan 21, 2011
  3. richard
    Replies:
    0
    Views:
    572
    richard
    Jan 21, 2011
  4. richard
    Replies:
    0
    Views:
    608
    richard
    Jan 21, 2011
  5. Jeff
    Replies:
    2
    Views:
    97
Loading...

Share This Page