Dynamically allocating memory for 2d char array

Viewed 1117

I am using malloc to dynamically allocate a 2d char array. Unless I set every array index to NULL before free(), I get a segmentation fault when trying to free(). Why do I get a segmentation fault?

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

int main(void) {

    int nrows = 20; int ncolumns = 10;
    char ** arr = malloc(nrows * sizeof(char *));
    for(int i = 0; i < nrows; i++)
        arr[i] = malloc(ncolumns * sizeof(char));

    arr[0] = "string1";
    arr[1] = "string2";

    // does not work without the following code:
    // for(int i = 0; i < 20; i++)
    //     arr[i] = NULL;

    for(int i = 0; i < 20; i++)
        free(arr[i]);
    free(arr);

    return 0;
}
5 Answers

When you do this:

arr[0] = "string1";
arr[1] = "string2";

You overwrite the contents of arr[0] and arr[1], which contain the addresses of memory returned from malloc, with the address of two string constants. This causes a memory leak, as that memory is no longer accessible. This is also the reason you crash when you call free because these variables no longer hold the addresses of allocated memory.

What you probably want to do here instead is use strcpy, which will copy the contents of the string literal to the memory you allocated.

strcpy(arr[0], "string1");
strcpy(arr[1], "string2");

Now you can free the memory properly.

Your code is ok, the problem comes from the fact that you are assigning string literals to your array here: arr[0] = "string1";.

You are thus replacing the pointer at arr[0], which is pointing to your allocated memory, with the pointer to a string literal.

Pointers to literals are protected, you cannot free (nor write to them) them because you didn't allocate them.

To solve this problem, use strcpy to copy the value of your literal inside your allocated memory:

strcpy(arr[0], "string1");
strcpy(arr[1], "string2");

= operator does not copy the string it only assigns the pointer. So your malloced memory is not accessible anymore for those array elements and attempt to free it is an Undefined Behavoiur which may lead to the segfault.

You need to copy it using strcpy.

Two things to know:

  • You have two area in you memory (to make easy t understand) heap and stack
  • malloc, realloc, calloc allocate ressource from heap. I will say only malloc (but it is the same)
    • free can only free ressource from heap. Stack is reserver for the compiler (it store function call and other data)

The rule for each ressource you get from malloc you have to free it. to free simply call the free function (but we can optionally assigne null pointer to be sure it is freed).

char * a = malloc(255);

to free

free(a);/* this is mandatory */
a = NULL;/* we can add this to the first line */

In fact it you take the habit to assign NULL value and one time you access it's value you will have NULL deference error: so you will know where to find error

What you try to do: alloc a array char ** arr = malloc(nrows * sizeof(char *)); and you free it free(arr);

but you alloc 20 arrays of char arr[i] = malloc(ncolumns * sizeof(char)); you ignore it's value arr[0] = "string1"; (you loose the value returned by malloc so you can't free now arr[0]) we are not in C++. So "string1" is stocked on the stack (so malloc can't free it) and you call free on it.

what you can do

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

int main(void) {

    int nrows = 20; int ncolumns = 10;
    char ** arr = malloc(nrows * sizeof(char *));
    for(int i = 0; i < nrows; i++)
        arr[i] = malloc(ncolumns * sizeof(char));
    free(arr[0]);//know we can loose it value because it is freed
    arr[0] = NULL;// in fact we assign a value just after so this line is useless but is educationnal purpose
    free(arr[1]);//know we can loose it value because it is freed
    arr[1] = NULL;// in fact we assign a value just after so this line is useless but is educationnal purpose
    arr[0] = "string1";
    arr[1] = "string2";

    // does not work without the following code:
    // for(int i = 0; i < 20; i++)
    //     arr[i] = NULL;

    for(int i = 2; i < 20; i++)//we start at 2 because the first two value are on the stack
    {
        free(arr[i]);
        arr[i] = NULL;//this is useless because we will free arr just after the loop)
    }
    free(arr);
    arr = NULL;// this is useless because we exit the end of program

    return 0;
}

Crashing upon a call to free is a sign of incorrect memory management somewhere else in your code. When you set a pointer to NULL then free it, you are not going to crash, because free(NULL) is guaranteed to be benign by the C Standard § 7.22.3.3:

7.22.3.3 The free function

... If ptr is a null pointer, no action occurs. Otherwise, if the argument does not match a pointer earlier returned by a memory management function, or if the space has been deallocated by a call to free or realloc, the behavior is undefined.

Emphasis mine.

As other answers have noted, you are trying to call free on memory that you didn't explicitly allocate with malloc-family functions (since you overwrote arr[i] pointers with pointers to string literals)

Related