Jump to content
  • Advertisement
Sign in to follow this  
BrianTheLlama

Releasing Memory

This topic is 4807 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I have the following templatised code:

// ----------------------------------------------------------------------------
// File:        MATRIX.H
// Synopsis:    Natus matrix arithmetic operations.
// Author:      Adam Chir
// ----------------------------------------------------------------------------

#ifndef MATRIX_H
#define MATRIX_H

#include "sysdeps.h"
#include <iostream.h>
namespace natus
{
    // Natus matrix arithmetic class
    template<class T> class Matrix
    {
    private:
        // attributes
        UINT32  m;          // row dimension
        UINT32  n;          // column dimension
        T      *element;    // matrix element data

        // private methods
        void allocate(const UINT32& m, const UINT32& n);
        void free(void);
        void reallocate(const UINT32& m, const UINT32& n);

    public:
        // constructors
        Matrix<T>(const UINT32& m, const UINT32& n);
        Matrix<T>(Matrix<T>& matrix);
        
        // destructors
        ~Matrix<T>(void); // possible virtual?

        // public operators
        T& operator()(const UINT32& i, const UINT32& j);

        Matrix<T> operator=(Matrix<T>& rightHandSide);  
        Matrix<T> operator+(Matrix<T>& rightHandSide);
        Matrix<T> operator-(Matrix<T>& rightHandSide);
        Matrix<T> operator*(Matrix<T>& rightHandSide);
        Matrix<T> operator~(void);
        Matrix<T> operator!(void);
    };
}

using namespace natus;

// ----------------------------------------------------------------------------
// Function:    template<class T> void Matrix<T>::allocate(const UINT32& m, 
//                                                         const UINT32& n)
// Synopsis:    Allocates sufficient memory for an m by n matrix.
// Author:      Adam Chir
// ----------------------------------------------------------------------------
template<class T> void Matrix<T>::allocate(const UINT32& m, const UINT32& n)
{
    // TO DO: exception handling
    this->element = new T [m * n];
    this->m       = m;
    this->n       = n;
}
// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------
// Function:    template<class T> void Matrix<T>::free(void)
// Synopsis:    Releases the memory used by a matrix.
// Author:      Adam Chir
// ----------------------------------------------------------------------------
template<class T> void Matrix<T>::free(void)
{
    // TO DO: exception handling
    if(this->element)
    {
        delete [] this->element;

        this->element = NULL;

        this->m = 0;
        this->n = 0;
    }
}
// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------
// Function:    template<class T> void Matrix<T>::reallocate(const UINT32& m, 
//                                                           const UINT32& n)
// Synopsis:    Releases then reallocates memory used by a matrix.
// Author:      Adam Chir
// ----------------------------------------------------------------------------
template<class T> void Matrix<T>::reallocate(const UINT32& m, const UINT32& n)
{
    // TO DO: exception handling
    free();             // release...
    allocate(m, n);     // and refresh!
}
// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------
// Function:    template<class T> Matrix<T>::Matrix<T>(const UINT32& m, 
//                                                     const UINT32& n)
// Synopsis:    Constructs a matrix of dimensions m by n.
// Author:      Adam Chir
// ----------------------------------------------------------------------------
template<class T> Matrix<T>::Matrix<T>(const UINT32& m, const UINT32& n)
{
    // TO DO: exception handling
    reallocate(m, n);
}
// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------
// Function:    template<class T> Matrix<T>::~Matrix<T>(void)
// Synopsis:    Default destructor
// Author:      Adam Chir
// ----------------------------------------------------------------------------
template<class T> Matrix<T>::~Matrix<T>(void)
{
    // TO DO: exception handling
    free();
}
// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------
// Function:    template<class T> Matrix<T>::Matrix<T>(const Matrix<T>& matrix)
// Synopsis:    Constructs a copy of a matrix.
// Author:      Adam Chir
// ----------------------------------------------------------------------------
template<class T> Matrix<T>::Matrix<T>(Matrix<T>& matrix)
{
    // TO DO: exception handling
    reallocate(matrix.m, matrix.n);

    // copy the elemental array from the left matrix to the right matrix
    for(int i = 1; i <= m; i++)
        for(int j = 1; j <= n; j++)
            (*this)(i, j) = matrix(i, j);
}
// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------
// Function:    template<class T> T& Matrix<T>::operator()(const UINT32& i, 
//                                                         const UINT32& j)
// Synopsis:    Retrieves a reference to the element in row i, column j.
// Author:      Adam Chir
// ----------------------------------------------------------------------------
template<class T> T& Matrix<T>::operator()(const UINT32& i, const UINT32& j)
{
    // TO DO: exception handling
    return(element[((j - 1) * m) + (i - 1)]);
}
// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------
// Function:    template<class T> Matrix<T> Matrix<T>::operator=
//                                          (Matrix<T>& rightHandSide)
// Synopsis:    Assignment operator.
// Author:      Adam Chir
// ----------------------------------------------------------------------------
template<class T> Matrix<T> Matrix<T>::operator=(Matrix<T>& rightHandSide)
{
    // TO DO: exception handling
    reallocate(rightHandSide.m, rightHandSide.n);

    // copy the elemental array from the left matrix to the right matrix
    for(int i = 1; i <= m; i++)
        for(int j = 1; j <= n; j++)
            (*this)(i, j) = rightHandSide(i, j);

    return(*this);
}
// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------
// Function:    template<class T> Matrix<T> Matrix<T>::operator+
//                                          (Matrix<T>& rightHandSide)
// Synopsis:    Returns a matrix that is the summation of the left and right
//              matrices.
// Author:      Adam Chir
// ----------------------------------------------------------------------------
template<class T> Matrix<T> Matrix<T>::operator+(Matrix<T>& rightHandSide)
{
    // TO DO: exception handling
    Matrix<T> sumResult(m, n);
  
    for(int i = 1; i <= m; i++)
        for(int j = 1; j <= n; j++)
            sumResult(i, j) = (*this)(i, j) + rightHandSide(i, j);

    return(sumResult);
}
// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------
// Function:    template<class T> Matrix<T> Matrix<T>::operator+
//                                          ( Matrix<T>& rightHandSide)
// Synopsis:    Returns a matrix that is the subtraction of the left and right
//              matrices.
// Author:      Adam Chir
// ----------------------------------------------------------------------------
template<class T> Matrix<T> Matrix<T>::operator-(Matrix<T>& rightHandSide)
{
    // TO DO: exception handling
    Matrix<T> subResult(m, n);
  
    for(int i = 1; i <= m; i++)
        for(int j = 1; j <= n; j++)
            subResult(i, j) = (*this)(i, j) - rightHandSide(i, j);

    return(subResult);
}
// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------
// Function:    template<class T> Matrix<T> Matrix<T>::operator*
//                                          (Matrix<T>& rightHandSide)
// Synopsis:    Returns a matrix that is the multiplication of the left and 
//              right matrices.
// Author:      Adam Chir
// ----------------------------------------------------------------------------
template<class T> Matrix<T> Matrix<T>::operator*(Matrix<T>& rightHandSide)
{
    // TO DO: exception handling
    // STUB
}
// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------
// Function:    template<class T> Matrix<T> Matrix<T>::operator~(void)
// Synopsis:    Returns a matrix that is the transverse of the calling matrix.
// Author:      Adam Chir
// ----------------------------------------------------------------------------
template<class T> Matrix<T> Matrix<T>::operator~(void)
{
    // TO DO: exception handling
    // STUB
}
// ----------------------------------------------------------------------------

// ----------------------------------------------------------------------------
// Function:    template<class T> Matrix<T> Matrix<T>::operator~(void)
// Synopsis:    Returns a matrix that is the inverse of the calling matrix.
// Author:      Adam Chir
// ----------------------------------------------------------------------------
template<class T> Matrix<T> Matrix<T>::operator!(void)
{
    // TO DO: exception handling
    // STUB
}
// ----------------------------------------------------------------------------

#endif


My problem is that MSVC 6.0 throws an assertion error when free() is called. Normally I wouldn't worry and try and locate my problem, but a perculiar thing occurs when I output to std::cout just before the memory block is deleted - the code runs normally! Anyone encountered this before, or know any way around the original problem of the assertion error? Regards, Adam

Share this post


Link to post
Share on other sites
Advertisement
What does your default c'tor do? Does it properly set the matrix' member array pointer to null? Because if not and your free method then tries to delete a block of memory where there is none .... BOOM. Just a thought.

P.S. Okay, I think I found a bug in your code.

One of your c'tors (namely this one)


template<class T> Matrix<T>::Matrix<T>(const UINT32& m, const UINT32& n)
{
// TO DO: exception handling
reallocate(m, n);
}




calls reallocate, which then calls free and tries to delete a block of memory because your array pointer is not null. Just edit your c'tor to this:


template<class T> Matrix<T>::Matrix<T>(const UINT32& m, const UINT32& n) : element( 0x00 )
{
// TO DO: exception handling
reallocate(m, n);
}




and it should work IMO.

Share this post


Link to post
Share on other sites
It looks like in the Matrix constructor you call reallocate (which calls free) before initializing element.

If you made element NULL before calling reallocate or called allocate instead of reallocate it shouldn't be a problem. I don't know if this is exactly what you were talking about though.

EDIT: dangit, beat me to it.

Share this post


Link to post
Share on other sites
[edit - Yikes... beaten to the punch [wink] ]

I haven't checked through the entire code yet to ensure there aren't any problems with writing past array bounds, but one thing did stand out to me. Your creation contructor looks suspicious:

template<class T> Matrix<T>::Matrix<T>(const UINT32& m, const UINT32& n)
{
// TO DO: exception handling
reallocate(m, n);
}


Calling reallocate() invokes free() which dereferences this and checks the value of element. First of all it is not guaranteed that element will be set to 0 at this point, so it is possible that the check will pass and you will attempt to delete [] some random piece of memory. This is clearly a bad thing. Secondly, calling functions that dereference this in the constructor is dangerous if you start using inheritance. In your particular case you should be fine, but it's a habit that I'd personally recommend not getting into for as long as possible, as it can make life miserable if you subclass something down the road and forget to adjust things accordingly. See this FAQ for more details.

IMHO it makes absolutely no sense whatsoever to call reallocate in the constructor - even if the issue with the randomly-initialized element pointer is ignored, the logical choice is to allocate here. Changing to allocate will band-aid the code and fix the problem with reallocate() releasing a bogus pointer during construction, but it'd also be a good idea to initialize the pointer for certainty, especially if you plan to introduce exception handling.


template<class T> Matrix<T>::Matrix<T>(const UINT32& m, const UINT32& n)
: element(0)
{
// TO DO: exception handling
allocate(m, n);
}



Optionally you can also initialize m and n here, but this doesn't seem to have any apparent benefit in code usability or safety.


See if that helps out for you. If not, even a rough call stack would be helpful - i.e. what happened just before free() got called? Is it the constructor calling reallocate() or the destructor, or some other place in code invoking the allocation functions manually?


[Another edit - to add on to what snk_kid said, ideally you should use the VC.Net C++ compiler pack with Visual Studio 6. You lose a few of the configuration options in the GUI, but the increased standards compliance and general niceness of the compiler is well worth it IMO.]

[Yargh! Looks like snk_kid pulled his post out [and reposted it below]. Are we confused yet? [grin] ]

Share this post


Link to post
Share on other sites
Just something i want to add.

  • This Matrix<T>(Matrix<T>& matrix); is not a declaration of a copy constructor, a copy constructor takes a constant reference

    This Matrix<T> operator=(Matrix<T>& rightHandSide); is not a declaration of an assignment operator, an assignment operator takes a constant reference and returns a non constant reference.

    If VC++ 6.0 follows the correct standard behaviour (as it mostly doesn't) then you still have an assignment operator & copy constructor implicitly defined (compiler generated) which does a default member-wise copy/assign. In this case this means you still have shallow copies and eventually invoke delete[] twice or more on the same area of memory.


  • Any member function which doesn't modify state should be a constant member functions, and the general arithmetic operators should also take constant references, e.g:

    This Matrix<T> operator*(Matrix<T>& rhs);

    would prefferably be this: Matrix<T> operator*(const Matrix<T>& rhs) const;.


  • Prefer building the general arithmetic operators out of arithmetic assignment operators e.g.


    foo& operator+=(const foo) { /*...* return *this; }

    foo operator+(const foo& f) const { return foo(*this) += f; }



  • Use std::size_t to refer to sizes its in header cstddef and generally pass/return it by value not reference its not worth the indirection as its small in size.


  • I would change this:

    T& operator()(const UINT32& i, const UINT32& j);

    to this:

    //...
    typedef std::size_t size_type;
    //...
    const T& operator()(size_type row, size_type col) const;
    T& operator()(size_type row, size_type col);



  • There is no such header as iostream.h, that predates standardization of C++ so its deprecated, all C++ headers have no extension and C headers in C++ also append a 'c' on the front, i.e math.h becomes cmath.


  • Checking for NULL before invoking operator delete/delete[] is redundant, a pointer is permitted to be null when passed to operator delete/delete[].


  • Lastly don't use the VC++ 6.0's compiler any more, its offically defunct product. It pre-dates standardization of C++ therefore it has poor standard compliance and does not support or incorrectly supports certain features of standard C++ which leads to borked/poorly implementated C++ standard libraries. Check my sig for legal freebie.



[Edited by - snk_kid on June 20, 2005 12:10:21 PM]

Share this post


Link to post
Share on other sites
Quote:
Lastly don't use the VC++ 6.0's compiler any more, its offically defunct product. It pre-dates standardization of C++ therefore it has poor standard compliance and does not support or incorrectly supports certain features of standard C++ which leads to borked/poorly implementated C++ standard libraries. Check my sig for legal freebie.


BTW the Microsoft Visual C++ Toolkit 2003, as linked to in snk_kid's sig. can be integrated with the Visual C++ 6 IDE.

If you're planning to use the standard library or any kind of templates, then you should really look at upgrading from VC++6.

Share this post


Link to post
Share on other sites
It's probably a complete fluke - things like that tend to happen a lot with goofy pointer behavior. That's part of what makes dynamic memory allocation such a complicated affair, and why the std containers and idioms like RAII were developed.

At a low level, when you create a new object, it is given a block of memory to hold all of the member data (and the vtabl if the class has virtual members). In most cases, this memory is not initialized to anything specific unless the constructor says for it to be; so when the object is created, any members which are not initialized will have unpredictable and potentially harmful contents. The most likely explanation for your case is that invoking the << operator on std::cout rearranged the stack (or heap, depending on how the Matrix object was instantiated) in such a way that the memory for the element pointer in the new object was effectively 0. Such things happen.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!