0

So I have created and allocated memory for a double pointer using the following function:

void mallocDoubleArr(double ***arr, int size)
{
    printf("Here: %d", size);
    int i, j;

    *arr = malloc(size * sizeof(double*));

    for(i = 0; i < size; i++)
    {
        (*arr)[i]= malloc(size*sizeof(double));
        for (j = 0; j < size; j++)
        {
            (*arr)[i][j] = 0;
        }
    }
}

And I called the function using:

    double **G; //Create double pointer to hold 2d matrix

    mallocDoubleArr(&G, numNodes);

Now my question is how would I write a function to free the memory?

I tried something like this:

void freeDoubleArr(double ***arr, int size)
{
    int i, j;

    for (i = 0; i < size; i++)
        for (j = 0; j < size; j++)
            free((arr)[i]);
    free(arr);
}
Ace
  • 462
  • 4
  • 10
  • 17
  • No, `free(arr[i][j])` is wrong -trying to `free` a `double` value. – Basile Starynkevitch Oct 22 '13 at 19:31
  • @BasileStarynkevitch I'm not sure what you are trying to say. Please tell me how I should do it if my way is wrong. – Ace Oct 22 '13 at 19:33
  • You make your live twice as hard by storing immediately into arr instead of using a temporary variable. Actually, it would be much easier to write a function that returns double**, just like malloc returns a void* and doesn't take a void** parameter. And just like free () takes a void*, freeDoubleArr should take a double** parameter and should be passed a pointer, not the address of the pointer. – gnasher729 May 24 '14 at 21:21

2 Answers2

3

It seems that you want to pass the address of a pointer to your freeDoubleArr like freeDoubleArr(&G, numnodes) (which I would rather call deleteDoubleArr). Then you need to have

void freeDoubleArr(double ***arrptr, int size)
{
   double** arr = *arrptr;
   for (int i = 0; i < size; i++)
      free(arr[i]);
   free (arr);
   *arrptr = NULL;
}

However, you could decide that your square matrix is not represented as an array of pointers to arrays, but just as a plain array. Perhaps using flexible array members (of C99 and later) like

struct matrix_st {
   unsigned size;
   double arr[]; /* flexible array of size*size elements */
};

could be useful, with the convention that  arr is really an array of size*size elements (each being a double).

Then you can define quick access and mutator inline functions.

inline double get_element(struct matrix_st *m, int i, int j) {
   assert (m != NULL);
   unsigned s = m->size;
   assert (i>=0 && i<s && j>=0 && j<s);
   return m->arr[s*i+j];
}

inline void put_element(struct matrix_st* m, int i, int j, double x) {
   assert (m != NULL);
   unsigned s = m->size;
   assert (i>=0 && i<s && j>=0 && j<s);
   m->arr[i*s+j] = x;
}

When optimizing and with <assert.h> (see assert(3) ...) and compiling with -DNDEBUG the above accessor get_element and mutator put_element would be perhaps faster than your code.

And the matrix creation is just (to create a zero-ed matrix):

struct matrix_st* make_matrix (unsigned size) {
   struct matrix_st* m = malloc(sizeof (struct matrix_st)
                                + size*size*sizeof(double);
   if (!m) { perror("malloc"); exit(EXIT_FAILURE); };
   m->size = size;
   memset(m->arr, 0, sizeof(double)*size*size);
   return m;
 }

Then the user could just use one single call to free to free such a matrix.

BTW, if coding on Linux, compile with gcc -Wall -g and use valgrind memory leak detector and gdb debugger.

Basile Starynkevitch
  • 223,805
  • 18
  • 296
  • 547
  • Program just hangs if it try it the way I did. I specifically need to use a double pointer but thanx for your code anyway. I'll try it a little later on something else. – Ace Oct 22 '13 at 19:31
  • I improved my answer, because I did not guess that you want to pass the address of a pointer to `freeDoubleArr` which is unusual. – Basile Starynkevitch Oct 22 '13 at 19:36
  • thanx for the help. I posted an answer to how I did it now. Would that work properly? The program compiles fine but I never know if I properly freed everything. – Ace Oct 22 '13 at 19:41
  • On Linux, `valgrind` will tell you about memory leaks. It is alone a good reason to switch to Linux (and there are tons of other good reasons). – Basile Starynkevitch Oct 22 '13 at 19:45
  • I was actually reading about valgrind earlier today. Is it easy to use? I'll probably use a virtual machine to run linux though. Is this still on topic? – Ace Oct 22 '13 at 19:48
  • 1
    Yes, you would compile your single-file program with `gcc -Wall -g mycode.c -o myprog` and run it with  `valgrind myprog`; BTW, I would suggest installing Linux on your laptop (not just in a VM). – Basile Starynkevitch Oct 22 '13 at 19:51
1

Shouldn't your free code be more like the following:

void freeDoubleArr(double ***arr, int size)
{
    int i;

    for (i = 0; i < size; i++)
        free((*arr)[i]);
    free(*arr);
}

You're not even using the 'j' parameter anywhere so the inner for loop is going to cause it to attempt to free the same area of memory size times. You also need to dereference the pointer that's passed in to get to the one that's malloc'd, in the same way you do when assigning to result of malloc into it.

Also, it would be useful when saying something doesn't work to include what specific errors you are seeing or what makes you think it isn't working properly.

Sean Burton
  • 907
  • 7
  • 15
  • I tried that. It Doesnt give any errors but when I run it just hangs, saying "Project has stopped working" – Ace Oct 22 '13 at 19:36
  • 1
    You should always try this sort of thing in debug mode, it will give you more useful errors. "Project has stopped working" is windows' way of saying something failed, most likely because of a segfault or something, which is exactly what I would expect with your original code. If you run the code under a debugger it should tell you on exactly which line the segfault is occurring. – Sean Burton Oct 22 '13 at 19:53