C Programming: Dynamic malloc in another function

Question

So I'm trying to dynamically malloc memory in other function. Here is my simplified code, I couldn't get it right.

The first malloc initialized memory.

The second malloc (realloc) expand it, my original code use it in a loop, so the memory can keep expanding.

I got the following result:

size=2
data=0.000000
data=2.500000

But I'm expecting

size=2
data=1.500000
data=2.500000

Any Suggestions? Here is my code.

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

typedef struct
{
   time_t time_stamp;
   double data;
} sensor_data;


void Get_Data(sensor_data **raw_data,int *size){

    (*size) = 0;
    *raw_data = (sensor_data *)malloc(sizeof(sensor_data) * ((*size) + 1));

    (*raw_data)[(*size)].data = 1.5;

    (*size)++;
    *raw_data = (sensor_data *)malloc(sizeof(sensor_data) * ((*size) + 1));

    (*raw_data)[(*size)].data = 2.5;

    (*size)++;

}

int main() {

    sensor_data *raw_data;
    int size = 0;

    Get_Data(&raw_data,&size);

    printf("size=%d\n",size);
    int i = 0;
    for( i = 0; i < size; i++)
    {
        printf("data=%f\n",raw_data[i].data);
    }

    free(raw_data);

    return 0;
}

Aside from the obvious memory leak, in what way does it not work? — Chris Turner, Apr 21 '17 at 13:53
nitpicking: either malloc, or dynamically allocate. pick one. — Karoly Horvath, Apr 21 '17 at 13:57
Several issues. You should not type cast `malloc` return. Your function is a bit odd. Why are you doing `malloc` twice? On the second `malloc`, you lost the pointer to the data you saved on the first `malloc`. You are setting `size` to the actual size less 1. So your `for` loop will do one less than you intended. — lurker, Apr 21 '17 at 13:58
`*raw_data = (sensor_data *)malloc(sizeof(sensor_data) * ((*size) + 1));` → `*raw_data = realloc(*raw_data, sizeof(sensor_data) * ((*size) + 1));` — Spikatrix, Apr 21 '17 at 14:05
Yes, that solves my problem, I toally forgot realloc. I shouldn't use malloc more than once. @CoolGuy — xkimi, Apr 21 '17 at 14:10
Could you elaborate the memory leak? I really appreciate your help. @ChrisTurner — xkimi, Apr 21 '17 at 14:11
The function `Get_Data` makes no sense what-so-ever. It will have to be rewritten from scratch. Why do you need heap allocation to begin with, since you only allocate 2 objects, always? — Lundin, Apr 21 '17 at 14:22
@Lundin It's a simplified version. The "realloc" part is actually in a loop. Depending on the configurations, that could be a lot of data. — xkimi, Apr 21 '17 at 14:25
@KangminXu Why would calling it from a loop justify the use? Using realloc from inside loops is usually a very bad idea. — Lundin, Apr 21 '17 at 14:33
@ChrisTurner I got the whole figured out with realloc. I use valgrind to make sure it's leak free. Thanks for the help. — xkimi, Apr 21 '17 at 15:34

score 1 · Answer 1 · edited May 23 '17 at 11:47

1

In your GetData function, you start by allocating space for an array of 1 sendor_data and write to it, which is fine. But you then allocate space for a new array, this time with space for 2 elements, and overwrite the pointer that pointed to the original allocated memory. So you loose the original memory block and write to only the second element of the new block.

What you want in the second case is not malloc but realloc. This function allows you to resize a memory block already allocated and retain any data from the old block in the new block. If the new block is larger, additional bytes will be uninitialized. If the new block is smaller, the data from the old block will be truncated to the size of the new.

So change your second malloc call to realloc:

*raw_data = realloc(*raw_data, sizeof(sensor_data) * ((*size) + 1));

Also, you should test the result of malloc and realloc to see if they returned NULL, and don't cast the return value of malloc.

edited May 23 '17 at 11:47

Community

1
1

answered Apr 21 '17 at 14:10

dbush

205,898
23
218
273

It really doesn't make sense at all to call realloc 2 lines below a malloc call. Instead calculate how much memory that's needed in advance and then call malloc once. – Lundin Apr 21 '17 at 14:19
As it turns out, the size is always 2 here so it makes little sense to use dynamic memory in the first place. – Lundin Apr 21 '17 at 14:21
1

@Lundin In this specific case yes, however the code does demonstrate a simplified method of allocating some initial amount of space and then expanding that space later as needed. If the `realloc` was inside of a loop whose controlling condition contains an external parameter, that would make it more concrete. – dbush Apr 21 '17 at 14:22
Thank you. Another note is when using C++ compiler, if not casting the return value, compiler will complain. Plain C environment works perfect, thanks for the help. @dbush – xkimi Apr 21 '17 at 14:27
1

@KangminXu If you are using malloc in C++, you are doing it terribly wrong. It can't be mixed with new/delete and it won't call constructors/destructors. – Lundin Apr 21 '17 at 14:31

C Programming: Dynamic malloc in another function

1 Answers1