Memory Leaking - C

Question

In the same file I have two routines. The first will store some bytes from one file. The other will give this information to routines that will process that information.

boolean
adin_memory(char* buffer, int size_chunck, int end_flag){
    real_data=(SP16 *)malloc(size_chunck); //real_data -->global
    memcpy(&(real_data[0]),&(buffer[0]),size_chunck);

    pos_write += size_chunck;
    global_size = size_chunck;
    global_end_flag = end_flag;
    //end_flag = 1 --> end of Stream
    //end_flag = 0 --> Streaming
    return TRUE;
}

To prevent the possibility of leaking I am using malloc. But this routine is called several times. So, after some repetitions of adin_memory and adin_read (where will be free), I think the memory starts to fragment (I can see a leak with the size of the input file in task manager - increment of RAM). Is that right? How can I prevent this? To see this leak I put one breakpoint at the beginning and at the end of adin_memory an look at task manager.

int
adin_read(SP16 *buf, int sampnum)
{
  FILE *fp;
  int cnt = 0;
  fp = gfp;

  //(.......)
    if(global_end_flag == 1 || pos_write == pos_read){ return -1;}

    for(i = pos_read/sizeof(SP16); i <= sampnum; i++){
         if(i >= pos_write/sizeof(SP16)) {              
             cnt = i;
             //(....)
             break;
         }
         buf[i] = real_data[i];
     }
     pos_write = 0; 
     //(....)
     free(real_data);
     return cnt;
}

This can help to avoid heap fragmentation -http://stackoverflow.com/questions/150753/how-to-avoid-heap-fragmentation — ameyCU, Oct 27 '15 at 17:46
Do you call `adin_memory` again before `adin_read` frees `real_data`? Because that would leak the memory pointed to by the current value of `real_data`. — Ian Abbott, Oct 27 '15 at 17:49
@IanAbbott Do you mean a "double free leak"? Now you saying this I think is possible. Let me check.. — carduh, Oct 27 '15 at 17:51
I think Ian refers to the case where you overwrite the global handle by assigning freshly allocated memory. Also, if you free a global variable, you should probably set it to `NULL` to avoid double `free`s. — M Oehm, Oct 27 '15 at 17:53
@MOehm The program in the way it is, will not overwrite memory. I check that in the debugger. — carduh, Oct 27 '15 at 18:01
It is at this line where the leak happens: `real_data=(SP16 *)malloc(size_chunck);`. The leak occurs every third\fourth times that the function is executed. — carduh, Oct 27 '15 at 18:06
@carduh What is a "double free leak"? Double free isn't a leak, it's undefined behavior. — PC Luddite, Oct 27 '15 at 18:35
[link](http://www.cprogramming.com/tutorial/memory_debugging_parallel_inspector.html) is what you saying. — carduh, Oct 27 '15 at 21:43

score 2 · Answer 1 · answered Oct 27 '15 at 17:54

int
adin_read(SP16 *buf, int sampnum)
{
  FILE *fp;
  int cnt = 0;
  fp = gfp;

  //(.......)
    if(global_end_flag == 1 || pos_write == pos_read){
       /* Leak is possibly here. You return without freeing.
          Ensure free is called here also. And it is good practice to
          make the freed pointer point to NULL so you can check and
          avoid double free problems. */       
       return -1;
    }

    for(i = pos_read/sizeof(SP16); i <= sampnum; i++){
         if(i >= pos_write/sizeof(SP16)) {              
             cnt = i;
             //(....)
             break;
         }
         buf[i] = real_data[i];
     }
     pos_write = 0; 
     //(....)
     free(real_data);
     return cnt;
}

thanks for the tip, but the leak persists. I will try to look somewhere in the code — carduh, Oct 28 '15 at 11:58

score 1 · Answer 2 · answered Oct 27 '15 at 18:03

1

Difficult to say without further context describing how you use these functions but...

Every time you call your adin_memory() function it will allocate some memory (via a call to malloc) and then set real_data to point to that newly allocated memory.

If real_data was already pointing to some allocated memory then you just leaked it.

So if your main program calls adin_memory() three times and then calls adin_read() then you will leak two blocks of memory and only free the last one.

answered Oct 27 '15 at 18:03

GrahamS

9,980
9
49
63

The sequence is... `adin_memory()` , `adin_read()`, `adin_memory()` ... and never other way. – carduh Oct 27 '15 at 18:28
If you ALWAYS call `adin_read()` after a call to `adin_memory()` then your leak is likely due to the early return in `adin_read()` as @cenouro correctly identified. – GrahamS Oct 27 '15 at 18:35
I already tested that option. But without success. The free process is going right (I guess). But in the adin_memory() at every 5/6 executions I will have a leak. I am trying to find other way of allocate the memory. – carduh Oct 28 '15 at 14:07
@carduh Do you have any hard evidence of an actual leak? RAM usage can increase for many reasons. Is your debugger reporting a leak (if it is capable of that)? – GrahamS Oct 28 '15 at 14:12
Apparently I have solved the problem. And there are no more increments. @GrahamS – carduh Oct 28 '15 at 14:37

score 0 · Answer 3 · answered Oct 28 '15 at 08:28

0

Change

if(global_end_flag == 1 || pos_write == pos_read){ return -1;}

To

if(global_end_flag == 1 || pos_write == pos_read)
{
    free(real_data);
    return -1;
}

answered Oct 28 '15 at 08:28

toxiicdev.net

1
1

carduh · Answer 4 · 2015-10-28T14:43:13.647

The problem was indeed in the successive malloc/free (wich cause memory fragmentation). After removed I create a global pointer to the incoming bytes. And make the memcpy in the adin_read()

int adin_read(SP16 *buf, int sampnum)
{
    int i;
    int cnt = 0; 
    if(global_end_flag == 1 || pos_write == pos_read){return -1}
    memcpy(buf,global_buffer,global_size);
    cnt = global_size/sizeof(SP16);
    pos_write = 0;
    pos_read = 0; 
    //(....) 
    return cnt;
}

And then:

boolean
adin_memory(char* buffer, int size_chunck, int end_flag){
    pos_write += size_chunck;
    global_size = size_chunck;
    global_end_flag = end_flag;
    global_buffer = buffer;
    return TRUE;
}

Memory Leaking - C

4 Answers4