CODEWITHSUNDEEP
cmemory-leaksmallocfree
Flank
Flank

Reputation: 53

C- Memory leak, why?

I found the following functions with valgrind me that tells me:

  LEAK SUMMARY:
 ==3120==    definitely lost: 7,968 bytes in 377 blocks
 ==3120==    indirectly lost: 0 bytes in 0 blocks
 ==3120==      possibly lost: 1,904 bytes in 7 blocks
 ==3120==    still reachable: 224 bytes in 7 blocks
 ==3120==         suppressed: 0 bytes in 0 blocks
 ==3120== Reachable blocks (those to which a pointer was found) are not shown.
 ==3120== To see them, rerun with: --leak-check=full --show-leak-kinds=all
 ==3120== 
 ==3120== For counts of detected and suppressed errors, rerun with: -v
 ==3120== Use --track-origins=yes to see where uninitialised values come from
 ==3120== ERROR SUMMARY: 644 errors from 6 contexts (suppressed: 0 from 0)

the functions are:

 void put(message_t dati, op_t *err){
     message_data_t payload=dati.data;
     message_t *key1=malloc(sizeof(message_t));
    *key1=dati.hdr;
     message_data_t *malldati=(message_data_t *)malloc(sizeof(message_data_t));
     malldati->len=payload.len;
     malldati->buf=payload.buf;
     if((icl_hash_insert(hash,(void *)&key1->key,malldati))==NULL){
         *err=13;
         free(chiave);
     }else{

          *err=11;
       }
  }

If I add the free in the else the program don't work.

 void update(message_t dati, op_t *err){

     message_data_t *exist;
     message_data_t *payload=(message_data_t *) malloc(sizeof(message_data_t));
     payload=&dati.data;
     message_hdr_t key1=dati.hdr;

     exist=icl_hash_find(hash, &key1.key);

     if(exist == NULL){
         *err=20;
     }else{
        if(exist->len!=payload->len){
            *err=19;

         }else{
            exist=payload;
            *err=11;

         }
    }
 }


prototypes icl_hash_insert icl_hash_find and functions are:

icl_entry_t * icl_hash_insert(icl_hash_t *, void*, void *);

void * icl_hash_find(icl_hash_t *, void* );

The problem are in the key1 (put) and payload (update) How I can fix the code? Thanks for help!

UPDATE: Thanks for the answer, now I have another memory leak caused by the thread. Valgrind report:

 ==6111== HEAP SUMMARY:
 ==6111==     in use at exit: 1,904 bytes in 7 blocks
 ==6111==   total heap usage: 1,168 allocs, 1,161 frees, 8,042,496 bytes allocated
 ==6111== 
 ==6111== Thread 1:
 ==6111== 1,904 bytes in 7 blocks are possibly lost in loss record 1 of 1
 ==6111==    at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
 ==6111==    by 0x40136D4: allocate_dtv (dl-tls.c:322)
 ==6111==    by 0x40136D4: _dl_allocate_tls (dl-tls.c:539)
 ==6111==    by 0x4E422AE: allocate_stack (allocatestack.c:588)
 ==6111==    by 0x4E422AE: pthread_create@@GLIBC_2.2.5 (pthread_create.c:539)
 ==6111==    by 0x40243D: main (membox.c:642)
 ==6111== 
 ==6111== LEAK SUMMARY:
 ==6111==    definitely lost: 0 bytes in 0 blocks
 ==6111==    indirectly lost: 0 bytes in 0 blocks
 ==6111==      possibly lost: 1,904 bytes in 7 blocks
 ==6111==    still reachable: 0 bytes in 0 blocks
 ==6111==         suppressed: 0 bytes in 0 blocks
 ==6111== 
 ==6111== For counts of detected and suppressed errors, rerun with: -v
 ==6111== Use --track-origins=yes to see where uninitialised values come from
 ==6111== ERROR SUMMARY: 641 errors from 3 contexts (suppressed: 0 from 0)

What's the problem?

Upvotes: 0

Views: 166

Answers (2)

John Bode
John Bode

Reputation: 123568

This has me concerned:

message_t *key1=malloc(sizeof(message_t));
*key1=dati.hdr;
...
if((icl_hash_insert(hash,(void *)&key1->key,malldati))==NULL){

You're dynamically allocating a whole new object of message_t, but only saving the address of one of its members. Unless hdr is the first member of message_t and key is the first member of hdr, the address you save in the hash is not the same as the address of the dynamically allocated object. That means you won't be able to free that memory later on.

Based on these snippets, it seems to me like you'd only want to make a copy of the key to save in the hash, not the entire message_t struct, like so:

T *key1 = malloc( sizeof *key1 ); // replace T with the actual key type
*key1 = dati.hdr.key;
...
if(( icl_hash_insert( hash, key1, malldati )) == NULL ){

I'm assuming that keys and data items are freed when they are removed from the hash.

Upvotes: 1

user6499716
user6499716

Reputation:

For every malloc you need to free once.

In put: You need to free key1 just before put ends.

In update: You need to free payload just before updates ends.

Also: payload=&dati.data; // you set payload to point to a new address, memory will be lost 

void put(message_t dati, op_t *err){
     message_data_t payload=dati.data;
     message_t *key1=malloc(sizeof(message_t));
    *key1=dati.hdr;
     message_data_t *malldati=(message_data_t *)malloc(sizeof(message_data_t));
     malldati->len=payload.len;
     malldati->buf=payload.buf;
     if((icl_hash_insert(hash,(void *)&key1->key,malldati))==NULL){
         *err=13;
         free(chiave);
     }else{

          *err=11;
       }
       free(key1);
       free(malldati);
  }


void update(message_t dati, op_t *err){

     message_data_t *exist;
     message_data_t *payload=(message_data_t *) malloc(sizeof(message_data_t));
     payload=&dati.data; // you set payload to point to a new address, memory will be lost 
     message_hdr_t key1=dati.hdr;

     exist=icl_hash_find(hash, &key1.key);

     if(exist == NULL){
         *err=20;
     }else{
        if(exist->len!=payload->len){
            *err=19;

         }else{
            exist=payload;
            *err=11;

         }
    }
    free(payload);
 }

Upvotes: 0

Related Questions