glibc detected : double free or corruption

Question

I will explain the brief coding steps I have done and area where I am facing the problem

main.cpp

int main()
{
    int cnt_map,i=1,value;

   /* My question is about this char pointer "key" */ 
    char *key =(char*)malloc(sizeof(char) * 25);

   if(key!=NULL)
   {
     printf("Key value is not NULL,its value is:%x\n",key) ;
     cout<< "Enter the number of elements required in container map"<<endl;
     cin >> cnt_map;
     for (i=1;i<=cnt_map;i++)
     {
       cout << "Enter the key : ";
       cin >>key;
       cout << "Enter the key value:" ;
       cin >>value;
       printf("value pointed by ptr key: %s, value in ptr: %x\n", key,key);
       c -> add_map1(key,value); //Function inserts value to map container
       key+=sizeof(key);
     }
     c -> size_map1();           //Function displays size of map container
     c -> display_map1();        //Function displays contents of map container
  if(key)
  {
    printf("FINALLY:value pointed by ptr key: %s, value in ptr: %x,size:%d\n",key, key, sizeof(key));
    free(key);
  } 
 }
return 0;
}

when tried compiling and running the above code, I am able to successfully compile the code but got "glibc detected : double free or corruption" when tried running the application.

Now my question is I created a char pointer(char *key =(char*)malloc(sizeof(char) * 25);) and successfully assigned memory to it using malloc. After completing my process when I tried freeing of that char pointer I am getting double free or corruption error. I learned that any variable assigned memory with malloc/calloc should be freed finally. Please tell why I am this getting error, why I should not do this? Please tell me how the memory operations are ongoing on char* key (if possible pictorially).

Note: The code presented above is not the complete code, I just explained where I am getting the problem and if I am not freeing the pointer variable, my application is running successfully.

Answer 1

By doing this:

key+=sizeof(key);

your key variable is no longer pointing to the start of the memory you allocated. You must pass the original pointer to free(). You need to store the original pointer in another variable so that you can correctly free() it at the end.

(You may be able to simply remove that line - I'm not sure what it's doing, given that sizeof(key) is either 4 or 8. I suspect it's redundant.)

Answer 2

That's because of this line : key+=sizeof(key); . key doen't contain the same address as the malloc returned address.

For example:

char *key =(char*)malloc(sizeof(char) * 25);

Let's say malloc returns the address 20000 (totally dumb address, it's just for the example).

Now you're doing key+=sizeof(key);, so key = 20000 + 4 = 20004. The problem is you're trying to free key, which points to the address 20004 instead of 20000.

In order to fix that, try this:

int main()
{
    int cnt_map,i=1,value;
    char *key_save;

   /* My question is about this char pointer "key" */ 
    char *key =(char*)malloc(sizeof(char) * 25);

    key_save = key;
   if(key!=NULL)
   {
     printf("Key value is not NULL,its value is:%x\n",key) ;
     cout<< "Enter the number of elements required in container map"<<endl;
     cin >> cnt_map;
     for (i=1;i<=cnt_map;i++)
     {
       cout << "Enter the key : ";
       cin >>key;
       cout << "Enter the key value:" ;
       cin >>value;
       printf("value pointed by ptr key: %s, value in ptr: %x\n", key,key);
       c -> add_map1(key,value); //Function inserts value to map container
       key+=sizeof(key);
     }
     c -> size_map1();           //Function displays size of map container
     c -> display_map1();        //Function displays contents of map container
  if(key)
  {
    printf("FINALLY:value pointed by ptr key: %s, value in ptr: %x,size:%d\n",key, key, sizeof(key));
    free(key_save);
  } 
 }
return 0;
}

Answer 3

Simply remove the line:

key+=sizeof(key);

key is not a pointer to an array of strings, it's a pointer to a single string. Every time you increment this, you're reducing the available space in the string. The first time you read a key, there's 25 bytes available. The next time, you've incremented key by 4 or 8 bytes, but the end of the allocated space hasn't changed, so now there's only 21 or 17 bytes available. The third time it's only 17 or 9 bytes, and so on. After a few iterations, you'll increment key past the end of the memory block that you allocated, and it will start writing into unallocated memory (or memory that's assigned to other data structures). This is undefined behavior, and will most likely cause unpredictable failures in your program.

Since you're using C++, you should use std::string instead of char[] for strings, and std::vector instead of ordinary arrays. These data structures automatically expand as needed, so you avoid buffer overflows like this.

Answer 4

this is not taking your code into consideration, but i had a same problem in Reader writer problem (operating systems) http://en.wikipedia.org/wiki/Readers%E2%80%93writers_problem.

It was due to file pointer being global so whenever any reader tried to read and in b/w another read reads and closes file pointer so when another reader which has not finished reading tried to close file pointer after reading. so what happened is file pointer was already closed it wasn't pointing to any file. Solution i used. Instead of declaring file pointer global i declared it local to reader function that it or else you can check file pointer for NULL and if NULL then don't close the file pointer.

#include<stdio.h>
#include<semaphore.h>
#include<pthread.h>
#include<string.h>
#include<stdlib.h>
sem_t x,wsem;
int rc=0;
char ch;
char str[20];
void *reader(void *);
void *writer(void *);


int main()
{
 int nw,nr,i=0,j=0;
 pthread_t w[10],r[10];
 sem_init(&x,0,1);
 sem_init(&wsem,0,1);
 rc=0;
 printf("Enter the no of readers:");
 scanf("%d",&nr);
 printf("Enter the no of writers");
 scanf("%d",&nw);
 while(i<nw || j<nr) 
 {

  if(i<nw)
  {
pthread_create(&w[i],NULL,writer,(void *)i);

i++;
  }
  if(j<nr)
  {
pthread_create(&r[j],NULL,reader,(void *)j);

    j++;
  }
 }
 for(i=0;i<nw;i++)
 {
pthread_join(w[i],NULL); 
 }
 for(j=0;j<nr;j++)
 {
pthread_join(r[j],NULL);
 }

 return 0;
}

void *reader(void *arg)
{
 FILE *fptr;
 sem_wait(&x);
 rc++;
 if(rc==1)
  sem_wait(&wsem);
 sem_post(&x);

 printf("\nreader %d:",arg);
 fptr=fopen("temp.txt","r+");
 while(fgets(str,10,fptr)!=NULL)
 {
  printf("%s",str);
 }
 printf("\n");
 fclose(fptr);
 sem_wait(&x);
 rc--;
 if(rc==0)
    sem_post(&wsem);
 sem_post(&x);
}


void *writer(void *arg)
{
 FILE *fptr1;
 sem_wait(&wsem);  
 printf("\nwriter-%d:\n",arg);
 fptr1=fopen("temp.txt","a+");
 printf("enter the string:");
 scanf("%s",str);
 fputs(str,fptr1);
 fclose(fptr1);  
 sem_post(&wsem);
}