Sjoseph Sjoseph - 11 months ago 40
C Question

Struggling to understand how to correctly allocate memory using malloc()

Apologies if this seems like a re-post of a question I asked previously. I have an array of data,

arr
, and an array of indexes,
index
. My goal is to use a for loop to create new arrays of data partitioned at each of the indexes and to further find the minimum in each partitioned array. I am using
malloc
to create a dynamic array which I then free at the end of each loop.

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

int main(void)
{
int j;
int arr[] = {1,3,4,6,7,8,12,87,89,12,34,43,54,67,81,2,0,10,23,45,81,23,89,23,56,81,28,79};
int index[] = {1,5,10,13,19,24};
int h = 27;
int k;
int c;
int len;
for(j = 0;j < 4;++j)
{ printf("\n");
len = index[j+1] - index[j] - 1;
printf("len %d: ",len);
int *temp_arr = malloc(1000*sizeof(int));
for(k = index[j];k<(index[j+1]);++k )
{
printf("array val %d, ", arr[k]);
temp_arr[k] = arr[k];
}
int local_min ;
local_min = temp_arr[0];
for ( c = 1 ; c < len ; c++ )
{ printf("Temp array %d, ", temp_arr[c]);
if ( temp_arr[c] < local_min)
{
local_min = temp_arr[c];
printf("Local min in loop %d ", local_min );
}
}
free(temp_arr);
printf("\n");
}
return 0;
}


I do not understand how to correctly allocate the memory using
malloc
. I have been experimenting with different values; Too small a value and my program crashes, too large a value and some of my output is non-sensible. In the example I have given here, I get a mixture of correct values and nonsense values.

Is there an issue with creating this dynamic array for each
j
value? Am I correctly de-allocating the memory using
free(temp_arr)
?

Answer Source

You are allocating 1000 ints in every loop and then freeing. Again and again.

If you want same size everytime you can move mallocing and freeing out of loop, and even better, use automatic array.

int temp_arr[1000] = { 0, };   // Iam automatic array

But if you want adapt size of malloced array to how many ints you actually need in every loop, leave mallocing where it is and instead of 1000, calculate real size, which is in variable len.

int *temp_arr = malloc(len *sizeof(int));

You have to modify this snippet of code as well

for(k = index[j];k<(index[j+1]);++k )
{
    printf("array val %d, ", arr[k]);
    temp_arr[k] = arr[k];
}

to

int indexCounter = 0;
for(k = index[j];k<(index[j+1]);++k )
{
    printf("array val %d, ", arr[k]);
    temp_arr[indexCounter++] = arr[k];
}

becouse otherwise you would access outside of bounds which has undefined behavior, and program would probably crash.

Here is live demo. With this modification your code works well and is more effective (checked with valgrind).

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download