Freeing a pointer makes my program crash

Question

I'm having some troubles with pointers.

This is their declaration (in main):

int *dot_positions = (int *)malloc(sizeof(int));
char *str = (char *)malloc((MAX_N + 1) * sizeof(char));

Where MAX_N is 100, and is the limit of the string. dot_positions[0] = -1;

I add a value to the first position of 'dot_position', then, in runtime, I call the following function in order to add other values.

int add_dot_to_array(int *array, int position, int array_len) {
    array = (int *)realloc(array, array_len + 1);
    array[array_len] = position;
    return array_len + 1;
}

At the end of the main(), I free the pointers:

free(str); 
free(dot_positions); 

But this causes the crash of my program. I'm using Orwell Dev-C++ with Mingw on a Windows x64 machine. I'm also sure that those pointer are not NULL. What's wrong? Thanks in advance!

Answer 1

You're losing the address realloc() returns, since you're not re-setting the pointer in the caller's environment. This causes that pointer (from the original malloc()) to go stale, and freeing it crashes.

To fix this, make it add_dot_to_array(int **array, int position, int array_len) so you can change the caller's pointer. Also make sure you don't realloc() on each addition, as that will kill performance.

Also, don't cast the return value of malloc() in C, and don't "scale" string allocations by sizeof (char).

Answer 2

In add_dot_to_array, you're passing a pointer by value - so the function has its own local pointer (that points to the same array as the calling pointer). After calling realloc the local pointer is updated, but not the original pointer at the call site.

Try passing a pointer to a pointer (eg **, so that the original pointer gets updated too).

int add_dot_to_array(int **array, int position, int array_len) {

and call like:

add_dot_to_array(&dot_positions, ...

Answer 3

As unwind says,

int add_dot_to_array(int *array, int position, int array_len) {
    array = (int *)realloc(array, array_len + 1);
    array[array_len] = position;
    return array_len + 1;
}

works wrong - and does so in several ways.

1. You don't return the new array to the caller.
2. You don't check whether realloc() is successful.

Better:

int add_dot_to_array(int ** parray, int position, int * array_len) {
    int * new_array = realloc(*parray, (array_len + 1) * sizeof(*new_array)); // correct size!
    if (!new_array) return -1;
    *parray = new_array;
    new_array[(*array_len)++] = position;
    return 0;
}

which is called with

int result = add_dot_to_array(&my_pointer, pos, &my_saved_length);

and a proper result checking.

Note that it is not optimal to resize the buffer for every single element added.

Better way: maintain an alloced_size and a used_size. The used_size is incremented on every addition, and only if it reaches the alloced_size, the realloc() happens. In this case, it is advisable to realloc more than only one additional element.

Answer 4

For retaining the new address do this

int add_dot_to_array(int ** array, int position, int array_len) 
{
    *array = (int *)realloc(*array, array_len + 1);
    *array[array_len] = position;
    return array_len + 1;
}

//For calling the function 
add_dot_to_array(&dot_positions,1,1);