Assigning memory in C++

Question

I am attempting to learn how to assign memory in C++. I followed this answer to allocating 2-D arrays: Copy 2D array using memcpy?

However, when I attempt to combine this and copying memory using std::copy I get a corruption.

#include <iostream>

class Matrix
{

  private:

    int nrows;
    int mcols;
    double **entry;


  public:

  Matrix(int n, int m);

  ~Matrix();

  Matrix& operator=(const Matrix& mat_in);

};

Matrix::Matrix(int n, int m)
{
  nrows = n;
  mcols = m;

  std::cout << "Using regular constructor" << std::endl;

  entry    = new double*[nrows];
  entry[0] = new double[nrows * mcols];
  for(int i=1;i<nrows;i++)
    entry[i] = entry[i-1] + mcols;

  for(int i=0;i<nrows;i++)
    for(int j=0;j<mcols;j++)
      entry[i][j] = 0.0;
}

Matrix::~Matrix()
{
  delete[] entry[0];
  delete[] entry;
}



Matrix& Matrix::operator=(const Matrix& mat_in)
{
  std::cout << "using deep copy constructor" << std::endl;

  if(this == &mat_in)
    return *this;

  double **p = new double*[mat_in.nrows];
  p[0] = new double[mat_in.nrows * mat_in.mcols];

  std::copy(mat_in.entry, mat_in.entry + (mat_in.nrows * mat_in.mcols), p);

  delete[] this -> entry;

  entry = p;
  nrows = mat_in.nrows;
  mcols = mat_in.mcols;

  return *this;
}
int main()

{
  Matrix A(3,3);

  Matrix B(3,3);

  B = A;

  return 0;
}

I think the problem is that I don't fully understand what is happening when I free the memory and I am freeing it twice.

Why do you feel that you need an array of pointers ("2D array")? Why will a single array of `double`s (of size width × height) not suffice? It would certainly be a _lot_ simpler and then you probably wouldn't have this problem. — Lightness Races in Orbit, Mar 04 '18 at 20:19
Consider learning from a [book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list/388282#388282) instead. — nwp, Mar 04 '18 at 20:19
Consider using `std::array` or `std::vector` instead of C arrays so that assignment works the same as for integers. It makes things a lot easier. — nwp, Mar 04 '18 at 20:21
Don't use manual memory management in modern C++ - please. Use containers and smart pointers and RAII.. — Jesper Juhl, Mar 04 '18 at 20:21
@LightnessRacesinOrbit I guess I'm just trying to do what I would usually do in C. Probably not the best approach. — John Meighan, Mar 04 '18 at 20:23
@John Meighan - C++ is *not* C. And many things are done differently. — Jesper Juhl, Mar 04 '18 at 20:28
A vector implementation `std::vector>` would be much less grief to handle. — David C. Rankin, Mar 04 '18 at 20:34
@JohnMeighan I use a 1D array and mapping functions to get more dimensions in C as well as C++. You need a larger block of memory, but modern CPUs prefer large continuous runs of data to hopping around chasing pointers. If you got the RAM, use it. — user4581301, Mar 04 '18 at 20:45

score 3 · Answer 1 · answered Mar 04 '18 at 20:32

You're mixing the two basic kinds of 2-D array allocation.

In one style, one allocates an array (size nRows) of row pointers, and then allocates an array (size nCols) of elements for each row. Addressing an element is rows[r][c].

In the other style, one allocates an array (size nRows*nCols) of elements for the whole thing. Addressing an element is elements[(r * nCols) + c].

john · Accepted Answer · 2018-03-04T20:34:53.733

2

Your copy assignment operator is bugged (despite your comment it's not a copy constructor). Specifically this

std::copy(mat_in.entry, mat_in.entry + (mat_in.nrows * mat_in.mcols), p);
delete[] this -> entry;

should be this

for(int i=1;i<mat_in.nrows;i++)
    p[i] = p[i-1] + mat_in.mcols;
std::copy(mat_in.entry[0], mat_in.entry[0] + (mat_in.nrows * mat_in.mcols), p[0]);
delete[] this -> entry[0];
delete[] this -> entry;

There's a lot of code duplication here, which you should clean up. You should also look into the copy and swap idiom which is a common way of implementing a copy assignment operator.

edited Mar 04 '18 at 20:34

answered Mar 04 '18 at 20:26

john

85,011
4
57
81

OK thanks, can you explain the difference between entry[0] and entry I would have thought they both point to the same place? – John Meighan Mar 04 '18 at 20:28
From your constructor you have `entry = new double*[nrows]; entry[0] = new double[nrows * mcols];`. Clearly they don't point to the same place. – john Mar 04 '18 at 20:30
`entry[0]` is the same as `*entry` or specifically `*(entry + 0)`. – jtbandes Mar 04 '18 at 20:30
Hmm, my code is closer but still not correct. I'll edit to fix it. – john Mar 04 '18 at 20:33
@JohnMeighan I think my code is correct now, but you can probably tell I'm not testing anything. – john Mar 04 '18 at 20:37

Assigning memory in C++

2 Answers2