0

When deleting a multidimensional matrix with this destructor:

matrix::~matrix(){
     int i;
     for(i=0;i<n;i++){
         delete[] user_matrix[i];}
     delete[] user_matrix;}

I revive this error:

*** glibc detected *** ./a.out: free(): invalid pointer: 0x00007fdb33067778 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7fdb32d2db96]
./a.out[0x40157c]
./a.out[0x40172b]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x7fdb32cd076d]
./a.out[0x400b09]

Followed by a "memory map". When I run it through valgrind it suggests that there is an error in operator over load for *:

  matrix matrix::operator* (matrix param) {
  //n is the size of the square matrix
  if(n!=param.n){
    //returns an empty matrix if they are not of equal size
    matrix blah;
    return blah;}
  //initiates a nxn matrix that is empty
  matrix temp(n,0);
  temp.user_matrix=matrix_mult(user_matrix,param.user_matrix);
  return temp;}

This fits with the destructor being called, also with the destructor commented out it runs until the computer runs out of memory or the calculation is small enough and finishes.

#include"matrix.h"
using namespace std;

matrix::matrix(int n1,int initiate){
  srand(3534.34535);
  n=n1;
  //float** user_matrix
  user_matrix=new float* [n];
  int i;
  for(i=0;i<n;i++){
    user_matrix[i]=new float [n];}

  if(initiate==1){

  int j;
  for(i=0;i<n;i++){

    for(j=0;j<n;j++){

      cout<<"please ["<<i<<"]["<<j<<"]"<<endl;

      cin>>user_matrix[j][i];}
  }
  }else if(initiate==2){
    user_matrix=random_matrix(n);}

}

float** matrix::inverse(){
  int i,k;
  float sub_det,detin;

  detin=det();

  if(detin==0){cout<<"uninvertable"<<endl;};

  float** inverse = new float* [n];
  for(i=0;i<n;i++){
  inverse[i]=new float [n];}

  float invertdet=1.0/detin;

  for(i=0;i<n;i++){
    for(k=0;k<n;k++){
      inverse[k][i]=invertdet*pow(-1,i+k)*determinant(sub_matrix(user_matrix,i,k,n),n-1);
    }
  }

  return inverse;}  

void matrix::display(){
  //cout<<"lol"<<endl<<n<<endl;
  int i,j;
  cout.precision(5);
  for(j=0;j<n;j++){
    cout<<"|";
    for(i=0;i<n;i++){

      cout<<user_matrix[i][j]<<"   ";};cout<<"|";
    cout<<endl<<endl;}
  cout<<endl<<endl;
}

float matrix::determinant(float** matrix,int n1){
  if(n1==1){return matrix[0][0];}
  int i;
  float det1=0;
  i=0;
  for(i=0;i<n1;i++){    
    float** temp_matrix=sub_matrix(matrix,i,0,n1);
    det1 = det1 + pow(-1.0,i)*matrix[i][0]*determinant(temp_matrix,n1-1);
    int j=0;
    for(j=0;j<n1-1;j++){delete[] temp_matrix[j];}
    delete[] temp_matrix;
  }
  return det1;}

float matrix::det(){return determinant(user_matrix,n);}




float** matrix::sub_matrix(float** matrix,int colum,int row,int n){
  float **sub_matrix=new float *[n-1];
  int iter;

  for(iter=0;iter<(n-1);iter++){
    sub_matrix[iter]=new float [n-1];}
  int iter2;
  int placeholder1,placeholder2;

  placeholder1=placeholder2=0;

  for(iter=0;iter<n;iter++){

    if(iter==colum){continue;}
    placeholder2=0;
    for(iter2=0;iter2<n;iter2++){
      if(iter2==row){continue;}
      sub_matrix[placeholder1][placeholder2]=matrix[iter][iter2];
      placeholder2++;
    }
    placeholder1++;

  }

  return sub_matrix;}

float** matrix::random_matrix(int n){

  int i,j;
  float** temp_mat=new float* [n];
  for(i=0;i<n;i++){
    temp_mat[i]=new float [n];}

  for(i=0;i<n;i++){
    for(j=0;j<n;j++){
      temp_mat[i][j]=rand()%10 +1;}
  }
  return temp_mat;}

float** matrix::matrix_mult(float** matrix1,float** matrix2){
  int i,j,k;
  float subresult;
  float** ret_mat;
  ret_mat=new float* [n];
  for(i=0;i<n;i++){
    ret_mat[i]=new float [n];}
  for(i=0;i<n;i++){
    for(j=0;j<n;j++){

      for(k=0;k<n;k++){
    subresult=subresult + matrix1[k][i]*matrix2[j][k];
      }
      ret_mat[i][j]=subresult;}
  }
  return ret_mat;}
matrix::~matrix(){
     int i;for(i=0;i<n;i++){delete[] user_matrix[i];};delete[] user_matrix;}

matrix matrix::operator* (matrix param) {
  if(n!=param.n){
    matrix blah;
    return blah;}

  matrix temp(n,0);
  temp.user_matrix=matrix_mult(user_matrix,param.user_matrix);
  return temp;}

int main(){
  int i;
  /*for(i=1;i<20;i++){
  matrix m1(i,2);
  cout<<i<<" "<<m1.det()<<endl;}*/
  matrix m1(16,2),m2(16,2),m3(16,0);
  for(i=0;i<100000;i++){m3=m1*m2;}


return 0;}

Code without non-error causing functions.

#include"matrix.h"
using namespace std;

matrix::matrix(int n1,int initiate){
  srand(3534.34535);
  n=n1;
  //float** user_matrix
  user_matrix=new float* [n];
  int i;
  for(i=0;i<n;i++){
    user_matrix[i]=new float [n];}

  if(initiate==1){

  int j;
  for(i=0;i<n;i++){

    for(j=0;j<n;j++){

      cout<<"please ["<<i<<"]["<<j<<"]"<<endl;

      cin>>user_matrix[j][i];}
  }
  }else if(initiate==2){
    user_matrix=random_matrix(n);}

}

float** matrix::random_matrix(int n){

  int i,j;
  float** temp_mat=new float* [n];
  for(i=0;i<n;i++){
    temp_mat[i]=new float [n];}

  for(i=0;i<n;i++){
    for(j=0;j<n;j++){
      temp_mat[i][j]=rand()%10 +1;}
  }
  return temp_mat;}

float** matrix::matrix_mult(float** matrix1,float** matrix2){
  int i,j,k;
  float subresult;
  float** ret_mat;
  ret_mat=new float* [n];
  for(i=0;i<n;i++){
    ret_mat[i]=new float [n];}
  for(i=0;i<n;i++){
    for(j=0;j<n;j++){

      for(k=0;k<n;k++){
    subresult=subresult + matrix1[k][i]*matrix2[j][k];
      }
      ret_mat[i][j]=subresult;}
  }
  return ret_mat;}
matrix::~matrix(){
     int i;for(i=0;i<n;i++){delete[] user_matrix[i];};delete[] user_matrix;}

matrix matrix::operator* (matrix param) {
  if(n!=param.n){
    matrix blah;
    return blah;}

  matrix temp(n,0);
  temp.user_matrix=matrix_mult(user_matrix,param.user_matrix);
  return temp;}

matrix & matrix::operator= (const matrix & param)
{
  int i,j;
  float** new_array=new float* [param.n];
  for(i=0;i<param.n;i++){
    new_array[i]=new float [param.n];}
  for(i=0;i<param.n;i++){
    for(j=0;j<param.n;j++){
      new_array[i][j]=param.user_matrix[i][j];}
  }

  for(i=0;i<param.n;i++){
    delete[] user_matrix[i];}
  delete[] user_matrix;
  user_matrix=new_array;
  n=param.n;
  return *this;

}

int main(){
  int i;
  matrix m1(16,2),m2(16,2),m3(16,0);
  for(i=0;i<100000;i++){m3=m1*m2;}


return 0;}
Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
user1750289
  • 17
  • 1
  • 7
  • Show the full code for the class or at least the data members and any overloaded operator[] etc. Have you defined a copy constructor and assignment operator for this class? – Pete Oct 20 '12 at 09:46
  • Incorrectly defined copy constructor is the most likely explanation for this error. But you didn't show us the copy constructor. Please post the whole class. – john Oct 20 '12 at 09:48

2 Answers2

1

When you have an object which allocates memory, just need to be careful about how that object is copied. You need to define a copy constructor and an assignment operator that do the right thing in terms of the memory your object allocates. Otherwise you get error when you delete objects, usually because you end up deleting the same memory twice.

You don't seem to have defined a copy constructor or an assignment operator. When you define a destructor you almost always need to define a copy constructor and an assignment operator as well. This is known as the 'rule of three'. You can have a look here for some guidelines on how to do this (or just read a good C++ book).

What is The Rule of Three?

Community
  • 1
  • 1
john
  • 85,011
  • 4
  • 57
  • 81
  • After defining a copy constructor I get the same error but valgrind now says the error is in the destructor. – user1750289 Oct 20 '12 at 10:16
  • Well to help with this problem I would really need to see a complete program. Can you cut down your code to the **smallest possible version** that still has the bug and then post the entire code. Or if your not sure whether you got your copy constructor right then you could just post that. If sure you've been told this already, but the reason people say to use std::vector instead of doing your own memory allocation is because, as you're finding out, doing your own memory allocation is hard. – john Oct 20 '12 at 10:17
  • There is no copy constructor even in the new code you posted. – john Oct 20 '12 at 11:07
  • What you've added is an assignment operator. You need a copy constructor as well. Your operator* function calls the copy constructor when it returns a matrix. I expect it's the copy constructor (or lack of it) that's causing your crash. – john Oct 20 '12 at 13:43
0

Without seeing more code I can only guess that you may

  • not have a proper copy constructor, so the user_matrix pointer gets copied, or
  • the default constructor does not initialize the member user_matrix, or
  • matrix_mult returns an invalid pointer.

But at least one thing is wrong. temp.user_matrix is deleted before temp.user_matrix=...

Yurim
  • 923
  • 1
  • 17
  • 34