Am I writing into memory I don't own?

This is a discussion on Am I writing into memory I don't own? within the C++ Programming forums, part of the General Programming Boards category; Here is a very abridged excerpt from a simple matrix manipulation library I'm writing, mostly as a practice project. Code: ...

  1. #1
    Registered User
    Join Date
    Dec 2010
    Posts
    14

    Am I writing into memory I don't own?

    Here is a very abridged excerpt from a simple matrix manipulation library I'm writing, mostly as a practice project.

    Code:
    class matrix {
    public:
    	double **a;
    	int row, col;
    
        matrix (){};
        matrix (double *,int,int);
    	matrix & operator = (const matrix &);
    };
    
    //constructor for matrix. 
    matrix::matrix(double *input, int inrow, int incol) {
    	row = inrow;
    	col = incol;
    	a = new double*[row];
    
    	for(int i=0;i<row;i++)
    		a[i] = new double[col];
    
    	if(input != NULL)
    		for(int i=0;i<row;i++)
    			for(int j=0;j<col;j++)
    				a[i][j] = *(input + (col*i) +j);
    	else
    		for(int i=0;i<row;i++)
    			for(int j=0;j<col;j++)
    				a[i][j] = 0;
    }
    
    //Overloaded = operator, simply checks matrices are of equal size and then 
    //copies elements from right to left.
    matrix & matrix::operator = (const matrix &rhs) {
    	if(this == &rhs)
    		return (*this); //not strictly necessary, I know.
    	if(row == rhs.row && col == rhs.col){
    		for(int i=0;i<row;i++)
    			for(int j=0;j<col;j++)
    				a[i][j]= rhs.a[i][j];
    		return (*this);
    	}
    }
    If I was to write this:
    Code:
    matrix test1 (some_array, rows, columns)
    matrix test2 = test1; //here
    matrix test3;
    test3 = test1;//here
    am I writing into memory I don't own?

    It seems to me the the default constructor is called, which includes no memory allocation, and then I'm assigning data //here into memory I don't own.

    However, I don't think I really understand the whole constructor business, and I don't get any errors at compile or runtime.

  2. #2
    Registered User
    Join Date
    Jun 2005
    Posts
    6,646
    Define "don't own".

    test1.a and test2.a will be the same value (i.e. pointing to the same memory). test3, being initialised with an empty constructor, will contain uninitialised data when it is created. So the assignment test3 = test1 (since it compares those uninitialised values with the values in test1) gives undefined behaviour.

    As a rough rule, if your class allocates and manages memory, it is a good idea to write an appropriate copy constructor, an assignment operator, and a destructor. It is also a good idea to ensure constructors and assignment operators all behave consistently with each other: i.e. if one allocates memory, all of them do. Mixing and matching (some do, some don't) is error prone.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Sunshine, and read this, this, and this before posting again.

  3. #3
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,308
    Your class does not follow "the rule of three" (look it up).
    This line of code actually calls the copy constructor:
    Code:
    matrix test2 = test1; //here
    But since you did not define one it will just make one by itself which would do the equivalent of a memcpy of the pointer value itself. That spell doom when the new object and the copied object are both destructed, when a double-free bug will occur.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  4. #4
    Registered User
    Join Date
    Dec 2010
    Posts
    14
    ok, here is the code with a copy constructor, a destructor and memory allocation in the = operator

    Code:
    class matrix {
    public:
    	double **a;
    	int row, col;
    
            matrix (){a=NULL;}; // is this good practice? I'm using it to identify to the = operator if memory needs allocating
            matrix (double *,int,int);
    	matrix (const matrix &);
    	~matrix();
    	
    	matrix & operator = (const matrix &);
    };
    
    //constructor for matrix. 
    matrix::matrix(double *input, int inrow, int incol) {
    	row = inrow;
    	col = incol;
    	a = new double*[row];
    
    	for(int i=0;i<row;i++)
    		a[i] = new double[col];
    
    	if(input != NULL)
    		for(int i=0;i<row;i++)
    			for(int j=0;j<col;j++)
    				a[i][j] = *(input + (col*i) +j);
    	else
    		for(int i=0;i<row;i++)
    			for(int j=0;j<col;j++)
    				a[i][j] = 0;
    }
    
    matrix::matrix (const matrix &in){
    	row = in.row;
    	col = in.col;
    
    	for(int i=0;i<row;i++)
    		a[i] = new double[col];
    
    	for(int i=0;i<row;i++)
    		for(int j=0;j<col;j++)
    			a[i][j] = in.a[i][j];
    }
    
    matrix::~matrix (){
            for(int i=0;i<row;i++)
    	delete[] a[i];
            delete[] a;
    }
    
    //Overoaded = operator, simply checks matrices are of equal size and then 
    //copies elements from right to left.
    matrix & matrix::operator = (const matrix &rhs) {
    	if(this == &rhs)
    	        return (*this);
            if(a == NULL){
    		row = rhs.row;
    		col = rhs.col;
    
    		for(int i=0;i<row;i++)
    			a[i] = new double[col];
    	}
    
    
    	if(row == rhs.row && col == rhs.col){
    		for(int i=0;i<row;i++)
    			for(int j=0;j<col;j++)
    				a[i][j]= rhs.a[i][j];
    		return (*this);
    	}
    }
    does this represent anything close to good c++ practice?
    Last edited by Skeksis; 07-12-2011 at 03:00 PM. Reason: just fixed a few stupid mistakes to do with **a and a

  5. #5
    Registered User
    Join Date
    Apr 2006
    Posts
    2,053
    Code:
    class matrix {
    public:
    	double **a;
    	int row, col;
    
            matrix (){**a=NULL;}; // is this good practice? I'm using it to identify to the = operator if memory needs allocating
    You're dereferncing a when it doesn't point to anything. Perhaps you want a=NULL? Also, in C++ it is better to use 0 or nullptr instead of NULL. (nullptr was/is being reciently introduced to the language, so support may be limited).

    Also, do you really want to allow default construction? It's not good practice to allow a partially built object. What is a matrix without data?

    Code:
    //Overoaded = operator, simply checks matrices are of equal size and then 
    //copies elements from right to left.
    matrix & matrix::operator = (const matrix &rhs) {
    	if(**a == NULL){
    		row = rhs.row;
    		col = rhs.col;
    
    		for(int i=0;i<row;i++)
    			a[i] = new double[col];
    	}
    	if(this == &rhs)
    		return (*this);
    
    	if(row == rhs.row && col == rhs.col){
    		for(int i=0;i<row;i++)
    			for(int j=0;j<col;j++)
    				a[i][j]= rhs.a[i][j];
    		return (*this);
    	}
    }
    What the two matrices are not the same size and the left is not empty?
    Last edited by King Mir; 07-12-2011 at 03:14 PM.
    It is too clear and so it is hard to see.
    A dunce once searched for fire with a lighted lantern.
    Had he known what fire was,
    He could have cooked his rice much sooner.

  6. #6
    Registered User
    Join Date
    Dec 2010
    Posts
    14
    Quote Originally Posted by King Mir View Post
    You're dereferncing a when it doesn't point to anything. Perhaps you want a=NULL? Also, in C++ it is better to use 0 or nullptr instead of NULL. (nullptr was/is being reciently introduced to the language, so support may be limited).
    fixed it in my last edit. Isn't NULL just #DEF NULL 0? Surely there's no problem?

    Quote Originally Posted by King Mir View Post
    What if the two matrices are not the same size and the left is not empty?
    I still haven't worked out what to do with that, I haven't got my head around exceptions yet which I hope will prove to be the solution.
    Last edited by Skeksis; 07-12-2011 at 03:25 PM.

  7. #7
    Registered User
    Join Date
    Apr 2006
    Posts
    2,053
    Quote Originally Posted by Skeksis View Post
    fixed it in my last edit. Isn't NULL just #DEF NULL 0? Surely there's no problem?
    Because when you see NULL you assume that the type must be a pointer type. But there are situations with templates where either a pointer or an integer could be valid, and in such cases the language defaults to integer. Using NULL in such cases would obscure the ambiguity. Salem or someone like him will probably jump in any moment and provide a link with examples.

    EDIT:Here's one
    Code:
    #include <iostream>
    template<typename T> int foo(T eg){
      return 1;
    }
    template<typename T> int foo(T *eg){
      return 0;
    }
    int main(){
      std::cout << foo(NULL);//prints 1;
      return 0;
    }
    I still haven't worked out what to do with that, I haven't got my head around exceptions yet which I hope will prove to be the solution.
    You could destroy the current data, and reallocate everything.
    Last edited by King Mir; 07-12-2011 at 04:08 PM.
    It is too clear and so it is hard to see.
    A dunce once searched for fire with a lighted lantern.
    Had he known what fire was,
    He could have cooked his rice much sooner.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. memory reading vs. writing
    By R.Stiltskin in forum C++ Programming
    Replies: 14
    Last Post: 08-20-2008, 05:57 AM
  2. Reading from and writing to memory with C
    By hpteenagewizkid in forum C Programming
    Replies: 7
    Last Post: 03-08-2006, 04:59 PM
  3. Memory Editing/Writing
    By Zero187 in forum C Programming
    Replies: 9
    Last Post: 08-04-2005, 06:59 PM
  4. writing to vdu memory
    By surdy in forum Linux Programming
    Replies: 2
    Last Post: 03-24-2004, 01:32 AM
  5. writing to vdu memory
    By surdy in forum C Programming
    Replies: 2
    Last Post: 03-23-2004, 04:44 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21