0

The following is my code and I cannot figure out where I am going wrong to free my 2d array. I know that the error happens at this line: free(arr[i]); and I also know that I have to do this loop to free each integer before freeing the entire array later. Can anyone spot the bug here? I get no compilation errors, but once running my executable, there is a huge backtrace output for the free function.

#include <stdio.h>
#include <stdlib.h>

int main()
{
        int h = 4;
        int w = 2;
        int i, j;

        int **arr = (int**)malloc(sizeof(int*) * h);
        arr[0] = (int*)malloc(sizeof(int) * w * h);

        for (i=1; i<h; i++)
        {
                arr[i] = arr[0] + (w*i);
        }

        int count = 0;
        for (i=0; i<h; i++)
        {
                for (j=0; j<w; j++)
                {
                        arr[i][j] = count++;
                }
        }


        for (i=0; i<h; i++)
        {
                for (j=0; j<w; j++)
                {
                        printf("Array[%d][%d] = %d ", i, j, arr[i][j]);
                }
                printf("\n");
        }

        for (i=0; i<h; i++)
        {
                free(arr[i]);
        }

        free(arr);

        /*printf("\nAfter freeing the array it becomes:\n");
        for (i=0; i<h; i++)
        {
                for (j=0; j<w; j++)
                {
                        printf("Array[%d][%d] = %d ", i, j, arr[i][j]);
                }
                printf("\n");
        }*/

}
  • 4
    you call `malloc` twice but you call `free` more than twice ... they must match up – M.M Nov 04 '19 at 04:34
  • Are you saying I call free lots of times in the loop? How should I properly do it? In all the other places I looked online this loop is needed to free each integer – camillesmith99 Nov 04 '19 at 04:35
  • @camillesmith99 The master, who is M,M, means you don't call any of the `malloc`s in a loop while you call `free` in a loop. That is, the code part seems like just a facsimile without grasping its underlying logic. – Soner from The Ottoman Empire Nov 04 '19 at 04:40
  • In this part of my code I create the rows of the array ``` for (i=1; i – camillesmith99 Nov 04 '19 at 04:50
  • If you allocate at once then you free at once. They must *always* match. You can’t free a random part of allocated memory. It’s just like borrowing a book from a library: you don’t bring it back in chapters, you bring it all back. And if you borrow five books those you give back one at a time, not glued together as one. So every malloc gets a free with the pointer it returned. – Sami Kuhmonen Nov 04 '19 at 05:07
  • 1
    Thank you, that helped. Would the correct way of freeing the array the way I built it be: free(arr[0]); //freeing the inner layer free(arr); //freeing the outer layer One for each malloc I called? – camillesmith99 Nov 04 '19 at 05:14
  • With a single call to `free(array)` or you are doing it wrong. Duplicate of [Correctly allocating multi-dimensional arrays](https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays). – Lundin Nov 04 '19 at 16:34

4 Answers4

0

you are allocating memory for arr[0] = (int*)malloc(sizeof(int) * w * h); only but while freeing the allocated space you are freeing up to h count, for (i=0; i<h; i++){free(arr[i]);}. You should have allocated this way as well using the same loop.

mghh
  • 47
  • 1
  • 6
0

When you use your malloc's you allocate only the first position of the array

arr[0] = (int*)malloc(sizeof(int) * w * h);

This means than when you have to free your memory you need to call free(arr[0]) and then free(arr)

This will not give you an error.

Anyway, I don't think this is what you want to do. You probably want a solution more on the lines of:

int **arr = (int**)malloc(sizeof(int*) * h);

for (int i = 1; i < h ; ++i){
     arr[i] = (int*)malloc(sizeof(int) * w); // with the correct size you want
}    

This will allocate h pointers in the array for use, then you can free the memory like you did in your example

EDIT: Please note that like @AndrewHenle correctly pointed out in his comment this solution doesn't allocate a true 2D array but just an array of pointers to arrays. For a way to properly allocate a 2D array please see this related question

John Doe
  • 1,613
  • 1
  • 17
  • 35
  • *You probably want a solution more on the lines* No. The solution with only two `malloc()` calls is much more efficient. Calling `malloc()` many times in a loop is not a "2-dimensional array" - it's an array of pointers to 1-dimensional arrays. For a **true** dynamically-allocated multidimensional array, see https://stackoverflow.com/questions/42094465/correctly-allocating-multi-dimensional-arrays – Andrew Henle Nov 06 '19 at 13:11
  • @AndrewHenle I know. I never wrote anywhere in my answer that i used a 2D array, i wrote that my answer "will allocate ```h``` pointers in the array for use" which is exactly "an array of pointers to 1-dimensional arrays" like you wrote. Giving how he was trying to ```free``` the memory in his example i thought he wanted a solution more on the lines of what i wrote rather than a true 2D array. I will add an edit in my answer to better reflect that – John Doe Nov 06 '19 at 13:41
0

The memory you are allocating in the call arr[0] = (int*)malloc(sizeof(int) * w * h); is enough for the entire 2D array. If you want to use this approach, i.e., allocate memory for the 2D array in a single call to malloc, you can do so by casting the returned pointer to pointer-to-array of w elements. Then you can also free the memory in a single call to free. No need to use the for-loop. See the code below.

int main()
{
    int h = 4;
    int w = 2;
    int i, j;

    int (*arr)[w] = (int(*)[w])malloc(sizeof(int)*h*w); // pointer to array of w ints

    int count = 0;
    for (i=0; i<h; i++) {
        for (j=0; j<w; j++) {
            arr[i][j] = count++;
        }
    }

    for (i=0; i<h; i++) {
        for (j=0; j<w; j++) {
            printf("Array[%d][%d] = %d ", i, j, arr[i][j]);
        }
        printf("\n");
    }

    free(arr);  //<-- free the 2D array in a single free
    return 0;
}
AliA
  • 690
  • 6
  • 8
-1
//for (i=0; i<h; i++) { free(arr[i]); }//error
free(arr[0]);  //true

free(arr);