When should I use free() in C?

Question

The code works as it is supposed to, though it never frees the memory allocated by malloc().

I have tried to free memory in any place that I can, but no matter where I do it, it breaks the program. Specifically, I get a "double free or corruption error." This is more of a question as to what free() and malloc() actually do? All of the problems with free are in the main:

int main(int argc,  char *argv[]){
if(argc!=2){
    exit(1);
}

printf("CSA WC version 1.0\n\n");

int length = strlen(argv[argc-1]);
char file_to_open[length];
strcpy(file_to_open, argv[argc-1]);

//printf("filename:%s\n",file_to_open);

//create counters for output
int count_number_of_lines = 0;
int count_number_of_words = 0;
int count_number_of_characters = 0;

//create int size of default array size
int current_array_size = pre_read(file_to_open);
//printf("number of lines: %i\n",current_array_size);

//create string array of default size
char *strings_array[current_array_size];

//create a pointer to catch incoming strings
char *incoming_string=NULL;

int done=0;
while(done==0){
    incoming_string=get_line_from_file(file_to_open, count_number_of_lines);
    if(incoming_string!=NULL){
        incoming_string=csestrcpy2(incoming_string);
        //printf("incoming line: %s\n",incoming_string);
        strings_array[count_number_of_lines]=(char*)malloc(strlen(incoming_string+1));
        strings_array[count_number_of_lines]=csestrcpy2(incoming_string);
        //printf("added to array:%s\n",strings_array[count_number_of_lines]);
        count_number_of_lines++;
        count_number_of_characters=(count_number_of_characters+(strlen(incoming_string)-1));
    }
    else{
        done=1;
    }

}
//all data is stored in a properly sized array


//count all words in array
int count=0;
int word_count=0;
char *readline;

while(count<current_array_size){
    readline = csestrcpy2(strings_array[count]);
    printf("line being checked: %s", readline);

    int i=0;
    int j=1;

    while( j< strlen(readline)+1 ){
        if(strcmp(readline,"\n")!=0){
            if( (readline[i] == ' ') && (readline[j] != ' ') ){
                word_count++;
            }
            if( (readline[i] != ' ') && (readline[j] == '\n') ){
                word_count++;
            }
        }
        i++;
        j++;
    }
    count++;
}
printf("current word count: %i", word_count);
return 0;
}



char* csestrcpy2(char* src){

int i = 0;
char *dest;
char t;
dest = (char*) malloc(MAX_LINE);

while( src[i] != '\0'){

    dest[i] = src[i];
    i++;

}

dest[i] = '\0';
//printf("length:%i\n",i);
free(dest);

return dest;
}

Answer 1

In general you only have to free memory that has been reserved for you dynamically. That means if you have a statement like this:

int *my_int_pointer;
my_int_pointer = malloc(sizeof(int));

than you need to free the memory that was allocated (reserved) by malloc. if you are unsure where to free it than just free it at the end of the program, by using free;

free(my_int_pointer);

In your file it looks like there will be memory allocated whenever there is a new line in the file you read (in the while(done==0) loop). so everytime after the if in the this loop you have to free the memory that was used by the variable.

Furthermore you need to free the memory that was allocated by for the readline variable. But as it was pointed out before you may have a memory leak there.

Hope this helps.

edit: Okay - I was already wondering about the csestrcpy function. Lets have a look at this function:

char* csestrcpy2(char* src){
    int i = 0;
    char *dest;
    char t;
    dest = (char*) malloc(MAX_LINE); /*<<- This allocates memory that has to be freed*/
    while( src[i] != '\0'){
        dest[i] = src[i];
        i++;
    }
    dest[i] = '\0';
    //printf("length:%i\n",i);
    free(dest);                  /* This frees the memory, but you return a pointer */
    return dest;                 /* to this memory. this is invalid.                */
}

What you could however free is the src pointer in that function. but remember: the pointer cannot hold information after the underlying memory is freed! It just points to a place in memory where it should not write or read anymore.

Furthermore the function copys the string as long as there is no '\0'. What happens if there is no terminator? The function keeps on copying from some memory adresses where it should not!

you should not use that function ;)

Answer 2

There needs to be a call to free() for each successful call to malloc().

That doesn't necessarily mean that you need to have equal numbers of malloc() and free() calls in your code; it means that for every malloc() call that's executed when your program runs, you should call free(), passing it the pointer value you got from malloc(). malloc() allocates memory; free() tells the system that you're done with the allocated memory.

(You can almost certainly get away with not free()ing allocated memory when your program terminates, since it will be reclaimed by the operating system, but just as a matter of style and good practice you should still match malloc()s with free()s.)

I'm ignoring calloc() and realloc() calls.

Answer 3

Dynamic memory allocation (malloc) allocates a memory block of requested size, and returns a pointer to the start of this block.Since we have taken this block from memory so its a good practice to return this back to memory after completion of task.

Now as answer of your question , To be always on safe side you can call free function before making return.

main{
    int *i;
    ..
    ..
    i=(int *)malloc(sizeof(int));
    ...//so something with i
    ..
    free(i);
    return 0;
}

Answer 4

Think of malloc and free as "begin" and "end". ANY time you call malloc, do what you need to and once you're done, always call free. Make sure to only free it once, double-free is a runtime error.

If you somehow lose the value returned by malloc (yes, this is what's happening with your code), then you have a memory leak (and the gates of hell open up, yada yada).

To re-iterate: free whatever malloc returns (except null).

Answer 5

As a general rule, for every malloc there should be a corresponding free. You cannot however free something twice (as you have noticed by now). I don't see any calls to free in your code, so it's impossible to say where your problem lies, but I noticed right away that you malloc some memory and assign it to readline inside of a loop, yet you don't call free on readline at the end of the loop, so you are leaking memory there.

Answer 6

        i++;
        j++;
        /// free here
        free(readline);
    }
    count++;

}
printf("current word count: %i", word_count);

//free here as well
for(int k = 0; k < count_number_of_lines; k++)
{
    free(strings_array[count_number_of_lines]);
}

return 0;
}

This should work.

In general - any memory allocated dynamically - using calloc/malloc/realloc needs to be freed using free() before the pointer goes out of scope.

If you allocate memory using 'new' then you need to free it using 'delete'.

Answer 7

This line:

strings_array[count_number_of_lines]=(char*)malloc(strlen(incoming_string+1));

is being overriden by the line next to it, so it can be removed.

You should also add free(readline) after count++ in the last loop to free the memory created by malloc.