2

I am getting this error:

Error in `./sorter': double free or corruption (!prev): 0x0000000000685010

and then a bunch of numbers which is the memory map. My program reads a CSV file of movies and their attributes from stdin and tokenizes it. The titles of the movies with commas in them are surrounded in quotes, so I split each line into 3 tokens and tokenize the front and back token again using the comma as the delimeter. I free all my mallocs at the end of the code but I still get this error. The csv is scanned until the end but I get an the error message. If I don't free the mallocs at all I don't get an error message but I highly doubt it is right. This is my main() :

char* CSV = (char*)malloc(sizeof(char)*500);
char* fronttoken = (char*)malloc(sizeof(char)*500);
char* token = (char*)malloc(sizeof(char)*500);
char* backtoken = (char*)malloc(sizeof(char)*500);
char* title = (char*)malloc(sizeof(char)*100);
while(fgets(CSV, sizeof(CSV)*500,stdin))
{   
    fronttoken = strtok(CSV, "\"");     //store token until first quote, if no quote, store whole line
    title = strtok(NULL,"\"");          //store token after first quote until 2nd quote
    if(title != NULL)                   //if quotes in line, consume comma meant to delim title
    {
        token = strtok(NULL, ",");      //eat comma
    }
    backtoken = strtok(NULL,"\n");  //tokenize from second quote to \n character (remainder of line)

    printf("Front : %s\nTitle: %s\nBack: %s\n", fronttoken, title, backtoken);  //temp print statement to see front,back,title components

    token = strtok(fronttoken, ",");    //tokenizing front using comma delim
    while (token != NULL)
    {
        printf("%s\n", token);
        token = strtok(NULL, ",");
    }
    if (title != NULL)      //print if there is a title with comma
    {
        printf("%s\n",title);
    }   
    token = strtok(backtoken,",");  //tokenizing back using comma delim
    while (token != NULL)
    {
        printf("%s\n", token);
        token = strtok(NULL, ",");
    }
}

free(CSV);
free(token);    
free(fronttoken);
free(backtoken);
free(title);

return 0;
gsamaras
  • 71,951
  • 46
  • 188
  • 305
codemonkey
  • 58
  • 1
  • 8
  • 2
    `while(fgets(CSV, sizeof(CSV)*500,stdin))` --> `while(fgets(CSV, sizeof(*CSV)*500,stdin))` – chux - Reinstate Monica Sep 30 '17 at 03:34
  • 4
    `strtok` doesn't allocate memory, you shouldn't free all the pointers returned by it. – Jack Sep 30 '17 at 03:34
  • 2
    In addition to what @Jack said, the memory you created with `malloc` is leaked once you assign the output of `strtok` to the pointer, since you no longer have a pointer to it to free it. – Charles Srstka Sep 30 '17 at 03:48
  • 1
    There is no need to cast the return of `malloc`, it is unnecessary. See: [**Do I cast the result of malloc?**](http://stackoverflow.com/q/605845/995714) – David C. Rankin Sep 30 '17 at 05:48
  • You have sizeof on the brain... `char *CSV = malloc(500); fgets(CSV, 500, stdin);` – M.M Sep 30 '17 at 06:37

1 Answers1

1

Focus here:

char* title = (char*)malloc(sizeof(char)*100);
title = strtok(NULL,"\"");
  1. You dynamically allocate memory that title points to.
  2. You assign the return value of strtok to title, losing any reference to the memory dynamically allocated with malloc()! This means that you will definetely have a memory leak, since you will never be able to de-allocate the memory you dynamically allocated before.

The ref's example of strtok() has a very informative example:

/* strtok example */
#include <stdio.h>
#include <string.h>

int main ()
{
  char str[] ="- This, a sample string.";
  char * pch;
  printf ("Splitting string \"%s\" into tokens:\n",str);
  pch = strtok (str," ,.-");
  while (pch != NULL)
  {
    printf ("%s\n",pch);
    pch = strtok (NULL, " ,.-");
  }
  return 0;
}

As a result, there is no need to allocate memory for what strtok() returns - it's actually bad as I explained before.

Back to your code:

free(title);

does nothing, since title is NULL at that point (because of the while loop after strtok().

Same with token.


Furthermore, fronttoken and backtoken also result in memory leaks, since they are assigned the return value of strtok(), after malloc() has been called. But their free() is problematic too (in contrast with the other de-allocations of title and token), since they point within the original memory allocated for CSV.

So, when free(backtoken); is called, double-free or memory corruption occurs.


Moreover, change this:

while(fgets(CSV, sizeof(CSV)*500,stdin))

to this:

while(fgets(CSV, sizeof(*CSV)*500,stdin))

since you want the size of where CSV points to (that's the size of the memory you dynamically allocated).

gsamaras
  • 71,951
  • 46
  • 188
  • 305
  • 1
    `free` of `fronttoken` and `backtoken` are also problematic as they point somewhere within the original memory allocated for `CSV` leading to the double-free or corruption when `free(backtoken)` is called. – David C. Rankin Sep 30 '17 at 05:53
  • Thank you so much. I am still a beginner at C and don't know when to use the malloc on pointers. So I should only use malloc when making an array? Just making a char* pointer is the same as making a string in java and can be changed? I'm still confused but hopefully I will learn eventually. – codemonkey Sep 30 '17 at 21:22
  • @codemonkey you are welcome. You should dynamically allocate memory only when you have to. Check the example of `strtok()` and study about pointers. BTW, if the answer helped you, please *accept* it by clicking the tick mark on the left of my answer. – gsamaras Oct 01 '17 at 08:29