fgets() causing segmentation fault when reading file

Question

I'm trying to use fgets() to read text from a file and I keep getting a segmentation fault. The program reads in the entire file and then after it reads the last line it crashes. Any help would be appreciated.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
char *readFile(FILE *);

char *readFile(FILE *file){
    int *outputSize = (int *)malloc(sizeof(int));
    (*outputSize) = 1024;
    char *buf = (char *)malloc(sizeof(char)*1024);
    char *output = (char *)malloc(sizeof(char)*(*outputSize));
    *output='\0';
    while(fgets(buf,1024,file)){
        if(strlen(output)+strlen(buf)+1>(*outputSize)){
            printf("REALLOCATING...");
            (*outputSize) *=2;
            output = realloc(output,sizeof(char)*(*outputSize));
        }
        printf("BUFFER SIZE: %d\nBUFFER : %s\n",strlen(buf),buf);
        strcat(output,buf);
        printf("OUTPUT SIZE: %d\nOUTPUT: %s\n",strlen(output),output);

    }
    printf("FREEING...");
    free(outputSize);
    free(buf);
    return output;
}

char *output = (char *)malloc(sizeof(char)*(*outputSize));*output=0; — BLUEPIXY
malloc does not clear the memory, so putting a '\0' in the first character guarantees that the first strcat will work correctly. If you got lucky and had an initial 0 in output, then you won't have noticed the problem. If the last printf(...,output); looks right, then the crash isn't in the code you've posted. — user3386109
@BLUEPIXY comments do help. There likely exist other issues. — chux - Reinstate Monica
Re-allocating may still not be big enough: Suggest if(strlen(output)+strlen(buf)+1>(*outputSize)){ --> while(strlen(output)+strlen(buf)+1>(*outputSize)){ although given code's logic, 1 pass should be enough. Just adding some defensive coding. — chux - Reinstate Monica

Andrew Henle Andrew Henle · Accepted Answer · 2015-04-14T00:04:29

Your code is very hard to read, and therefore very hard to debug. Which is why you're asking for help debugging it.

You don't need to read a file line-by-line when you know you're reading the whole file. Simplify that code and just read the whole file - that makes it a lot easier to troubleshoot. (This code even has all error checking in less lines, and IMO is a LOT easier to understand, even without comments or debug statements that tell you what's going on)

char *readFile( FILE *file )
{
    struct stat sb;
    if ( !fstat( fileno( file ), &sb ) )
    {
        return( NULL );
    }

    if ( -1 == fseek( file, 0, SEEK_SET ) )
    {
        return( NULL );
    }

    char *data = malloc( sb.st_size + 1 );
    if ( data == NULL )
    {
        return( NULL );
    }

    /* this error check might not work in text mode because
       of \r\n translation */
    size_t bytesRead = fread( data, 1, sb.st_size, file );
    if ( bytesRead != sb.st_size )
    {
        free( data );
        return( NULL );
    }

    data[ sb.st_size ] = '\0';
    return( data );
}

Header files will need to be updated.

fgets() causing segmentation fault when reading file

1 Answers