Impossible to close a file after reading it - C programming

Question

I have written a function which reads a file full of text, and proceeds to concatenate all the lines into a single string. It works, but the fclose() instruction, when used, launches an error :

"* Error in `./main': double free or corruption (out): 0x00000000020fc330 *" followed by a backtrace and a memory map.

It also works badly for files with a single line.

What can I do ?

The code :

char* readFile(char* fileName){
FILE* myFile = fopen(fileName, "r");
if (myFile != NULL) {
    fseek(myFile, 0, SEEK_END);
    long l = ftell(myFile);
    fseek(myFile, 0, SEEK_SET);
    char* content = (char*)malloc((size_t)l * sizeof(char));
    if (content == NULL) return NULL;
    char * chain = (char*)malloc((size_t)l * sizeof(char));     
    while(fscanf(myFile, "%s\n", chain) != EOF || fscanf(myFile, "%s\t", chain) != EOF || fscanf(myFile, "%s ", chain) != EOF){
        strcat(content, chain);
    }
    free(chain);
    fclose(myFile);         
    return content;
} else return NULL;

}

Answer 1

Most likely what's happening here is that your calls to scanf() and/or strcat() are writing past the end of your allocated buffers (content or chain) and corrupting the heap. That corruption is noticed by free() when it tries to free 'chain', causing the error message and the crash.

In order to avoid writing past the end of the buffers, you'll need to know exactly how many bytes each of these calls is going to write into the buffer. That's difficult to know with fscanf() unless you have a guarantee on what's going to be in the file (and usually you do not), so you might want to use fread() instead, so that you can specify the maximum number of bytes to read. Also when calling strcat() you need to make sure that the buffer pointed to by the first argument has enough space for both its existing contents plus the contents of the string you want to add to the end of its existing contents. Consider calling strncat() instead if you want to make sure to avoid an overwrite (since strcat() has no way of knowing how big the buffers are, it will happily write past the end of a too-short buffer -- at least strncat() will stop writing after the number of bytes you specified in its third argument)

Answer 2

In addition to othjer comments, you should avoid using strcat() in the loop. Each time through the loop, strcat has to search the string for the nul character, and this search keeps getting longer and longer.

A more efficient way would be to keep a pointer to the nul character and just use strcpy to add to it, then increment the pointer by the number of characters that were added.

Answer 3

As others have stated, your content pointer points to memory that is not initialized, it may contain junk when you are trying to strcat() to it. Try using calloc() instead, this will set all the chars to '\0' and allow you to do what you want.

Consider following Jeremy's advice and create a more efficient function to do what you want. Right now you're allocating two buffers the size of the file and looking for tokens to fill in one of these huge buffers and concatenating them into the other. You'll definitely have plenty of space since you're dropping the delimiters, but that is a lot of wasted space.

Answer 4

the posted code, as indicated in the comments to the question, has a lot of problems.

The following posted code corrects these problems, cleanly compiles, and performs the desired function.

#include <stdio.h>   // fopen(), fclose(), fseek(), NULL, perror()
#include <stdlib.h>  // malloc(), free()
#include <string.h>  // strcat()

char* readFile( char* );

char* readFile( char* fileName )
{
    FILE* myFile = fopen(fileName, "r");

    if ( !myFile )
    {
        perror( "fopen for reading file failed" );
        return NULL;
    }

    // implied else, fopen successful

    if( 0 != fseek(myFile, 0, SEEK_END) )
    {
        perror( "fseek to end of file failed:" );
        return NULL;
    }

    // implied else, fseek successful

    long fileLength = ftell(myFile);
    if( -1 == fileLength )
    {
        perror( "ftell failed" );
        return NULL;
    }

    // implied else, ftell successful

    if( 0 != fseek(myFile, 0, SEEK_SET) )
    {
        perror( "fseek to start of file failed" );
        return NULL;
    }

    // implied else, fseek successful

    char* content = malloc((size_t)fileLength+1 );
    if (content == NULL)
        return NULL;

    // initialize the concatenated string to be empty
    content[0] = '\0';

    char * chain = malloc((size_t)fileLength+1);
    if ( chain == NULL)
        return NULL;

    while( fgets( chain, (int)fileLength, myFile ) )
    {
        strcat(content, chain);
    }

    free(chain);
    fclose(myFile);
    return content;
} // end function: readfile