2

I have a matrix declared like int **matrix, and I know that the proper way to pass it to a function to allocate memory should be like this:

void AllocMat(int ***mat, int size);

But now I need to delete these memory in another function and am not sure about what to pass:

void DeallocMat(int **mat, int size);

or

void DeallocMat(int ***mat, int size);

I think the second one should be right, but neither way gives me segmentation fault as I tried.

Unihedron
  • 10,902
  • 13
  • 62
  • 72
Yang Chi
  • 432
  • 1
  • 3
  • 8
  • Not an answer but a suggestion. Use smart pointers, never do allocation and deallocation of memory yourself. – Vikas Aug 19 '11 at 18:51
  • You could use a `typedef int** MatrixType;` in order to make everything more readable. – trenki Aug 19 '11 at 18:56
  • 2
    @Vikas: **Never** is not true & misleading, **most of the times** is more appropriate. – Alok Save Aug 19 '11 at 19:03
  • 1
    You can reconsider your assumptions here: `int ***mat` is the C way of passing the pointer so that the function can modify it, `int **&mat` would be the C++ way. Then again, it might be better to redesign and allocate a single block `int *&mat`, even if that requires some more arithmetic... but again, being C++ it might be even better to encapsulate in a class, and then just allocate in the constructor, no need to pass anything around... – David Rodríguez - dribeas Aug 19 '11 at 19:20
  • Is this question supposed to be tagged C++? It looks like it should be C and most of the answers below are treating as if the OP wants a C solution. If we are really taking about C++ there should be no pointers here, it should be a class that encapsulates all the resources details from the user. – Martin York Aug 19 '11 at 19:49
  • Obligatory link to [Three Star Programmer](http://c2.com/cgi/wiki?ThreeStarProgrammer) – Bo Persson Aug 19 '11 at 23:59

7 Answers7

4

First is correct. But your real problem is that you are using pointers when there are better alternatives. For a 2d matrix you should use a vector of vectors

#include <vector>

typedef std::vector<std::vector<int> > Matrix;

Matix m;

Now there is no need to delete anything, so one less thing to go wrong.

john
  • 85,011
  • 4
  • 57
  • 81
  • 1
    That's also not really a good way, because the vectors in the matrix are not automatically synced to have all the same size. – leftaroundabout Aug 19 '11 at 19:06
  • The answer to that issue would be to wrap the vector of vectors in a class which could guarantee that all vectors would be the same size. There are many ways of solving this of course, and they're all better than using naked pointers. – john Aug 19 '11 at 19:37
  • @leftaroundabout: That's not a real problem (and as John points out easily solved), – Martin York Aug 19 '11 at 19:46
4

The question is tagged C++, and yet the answers only use the C subset...

Well, first of all, I would recommend against the whole thing. Create a class that encapsulates your matrix and allocate it in a single block, offer operator()(int,int) to gain access to the elements...

But back to the problem. In C++ you should use references rather than pointers to allow the function to change the argument, so your original allocate signature should be:

void AllocMat(int **&mat, int size);

And call it like:

int **matrix = 0;
AllocMat( matrix, 5 );

Or better, just return the pointer:

int **AllocMat( int size );
int **matrix = AllocMat( 5 );

For the deallocation function, since you don't need to modify the outer pointer, you can just use:

void DeallocMat( int**mat, int size ); // size might be required to release the 
                                       // internal pointers

Now, for a sketch of the C++ solution:

template <typename T>                   // no need to limit this to int
class square_matrix {
   const unsigned size;
   T * data;
public:
   square_matrix( unsigned size ) : size(size), data( new T[size*size]() ) {}
   square_matrix( matrix const & m ) : size( m.size ), data( new T[m.size*m.size] ) {
      std::copy( m.data, m.data+size*size, data );
   }
   ~matrix() {
      delete [] data;
   }
   T const & operator()( unsigned x, unsigned y ) const {
      // optional range check and throw exception
      return data[ x + y*size ];
   }
   void set( unsigned x, unsigned y, T const & value ) {
      // optional range check and throw exception
      data[ x + y*size ] = value;
   }
};
Martin York
  • 257,169
  • 86
  • 333
  • 562
David Rodríguez - dribeas
  • 204,818
  • 23
  • 294
  • 489
  • Thanks for this answer. Being a C programmer most time and only start using C++ recently, I always forget to use refs. Speaking of a new class to wrap up everything, well, it's really great but I only need this matrix in one function. Anyhow, great answers from everyone. I learned a lot. – Yang Chi Aug 21 '11 at 13:31
  • @Yang Chi: I would still advice you to encapsulate the matrix, the cost of doing so now is minimal, and the problem of not doing so is that if in the future you need to do it you will have to a) do it, b) fix the old code that used the raw resource. Also note that without encapsulating resources and using RAII, it will be tricky to get exception safety correctly... which means that under some circumstances your program might leak resources. Adding the enclosing type is not just *syntactic sugar* but enabling for better code. – David Rodríguez - dribeas Aug 22 '11 at 10:52
3

void DeallocMat(int **mat, int size) - allows you to deallocate memory (since you have passed the value of mat only allowing to deallocate memory but not change mat)

void DeallocMat(int ***mat, int size) - allows you to deallocate memory and change the value of mat to NULL (since you have now passed a pointer to mat allowing you to change its value)

Jacob
  • 34,255
  • 14
  • 110
  • 165
  • 1
    @Stevehb: It hides more errors than it fixes in the long run. I want my bugs to be non hidden and cause a crash during testing rather than hide and come out in production. Anyway this is a silly recommendation for a C+++ question. The Matrix should be wrapped in a class for correct resource management. – Martin York Aug 19 '11 at 19:45
  • @stevenhb: Martin is right, this has actually been [asked](http://stackoverflow.com/questions/7114465/is-delete-p-p-nullnullptr-an-antipattern) today, but you can find other discussions over the internet. There is no clear advantage of setting it to null other than hiding double deletes, and setting to null will not solve the issue just pushes it under the carpet... – David Rodríguez - dribeas Aug 19 '11 at 20:02
1

The extra "*" just handles the pointer to be behaved as call by reference. If you want to get the output from your function, you need an extra "*" in your declaration. In this case, you should pass the reference of your pointer (using &) to these functions.

TonySalimi
  • 8,257
  • 4
  • 33
  • 62
0

Either way works, but if you pass a pointer to the pointer you need to dereference it first. And the size parameter is redundant.

void DeallocMat(int **mat)
{
    delete[] mat;
}

void DeallocMat(int ***mat)
{
    delete[] *mat;
    *mat = NULL;
}
Mark Ransom
  • 299,747
  • 42
  • 398
  • 622
0

The reason why you required to pass a pointer to double pointer because your local variable must required to reflect with the new updated memory

void Foo(int * a)
{
   a = new int[10];
}
int main()
{
   int *a = 0;
   Foo( a );
}

Now the memory will be allocated but the pointer A will not be update because the value of pointer A is simply copied to another pointer variable which is parameter of Foo. Once the Foo is returned, a will remain 0. To make it refect that, you should write code like follows

void Foo(int ** a)
{
   *a = new int[10];
}
int main()
{
   int *a = 0;
   Foo( &a );
}

Here you're passing the address of a pointer. The which means that, the value which contains in the pointer will be updated from the Foo function.You can debug through and see how it works.

If you're sure that you will not access the pointer anymore, please use the first type. Otherwise use the second one. Make sure that you set the pointer to NULL to avoid further memory corruptions or dangling pointers.

sarat
  • 10,512
  • 7
  • 43
  • 74
  • `If you're sure that you will not access the pointer anymore, please use the first type.` doesn't make any sense. If you want to allocate dynamic memory to an pointer inside a function being passed as parameter to the function you simply **cannot** use #1. Period. – Alok Save Aug 19 '11 at 19:09
0

The thing that confuses me about your question is that most people would not declare a matrix as an int **. The reason for this is that you would be forced to then allocate it in a loop. Your allocation function would require two parameters, which are the dimensions of the array like this:

void AllocMat(int *** mat, int n, int m) {
    int ** result = new int * [ n ];
    for (int x=0; x<n; x++) {
        result[x] = new int [ m ];
    }
    *mat = result;
}

If this were the case, the corresponding deallocation function would require knowledge of the size of n as follows:

void DeallocMat(int *** mat, int n) {
    if (mat == NULL || *mat == NULL) return;
    int ** tmp = *mat;
    for (int x=0; x<n; x++) {
        if (tmp[x] != NULL) delete [] tmp[x];
    }
    delete [] tmp;
    *mat = NULL;
}

With this approach, you could access your matrix like this:

int ** mat = NULL;
AllocMat(&mat, n, m);

for (int x=0; x<n; x++) {
    for (int y=0; y<m; y++) {
        mat[x][y] = 1;
    }
}

DeallocMat(&mat, n);

Usually, people allocate matrices as a single buffer of memory to avoid extra allocations and pointer indirections, which is how I recommend you do it. In that case, you allocation function would look like this:

void AllocMat2(int ** mat, int n, int m) {
        *mat = new int [ n * m ];
}

And the corresponding deallocation function like this:

void DeallocMat2(int ** mat) {
    if (mat != NULL && *mat != NULL) {
        delete [] *mat;
        *mat = NULL;
    }
}

And you would access it follows:

int * mat2 = NULL;
AllocMat2(&mat2, n, m);

for (int x=0; x<n; x++) {
    for (int y=0; y<m; y++) {
        mat2[x * n + y] = 1;
    }
}

DeallocMat2(&mat2);
Ivan P
  • 56
  • 3