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, Apr 13 '15 at 23:38
I don't see how that would help. It makes it through the entire file just fine. But once its finished a segmentation fault occurs. — user3294542, Apr 13 '15 at 23:40
I don't see how ignoring good advice would help. Try it first. — chux - Reinstate Monica, Apr 13 '15 at 23:41
Also the second suggestion didn't help. I'm currently testing with a small test file that shouldn't even fill the buffer. — user3294542, Apr 13 '15 at 23:42
`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, Apr 13 '15 at 23:44
@BLUEPIXY comments do help. There likely exist other issues. — chux - Reinstate Monica, Apr 13 '15 at 23:49
Simplify your code. Get rid of the extra `malloc()/free()` calls. Instead of `int *outputSize = malloc()...`, just use `int outputSize;` and use the `int` directly. — Andrew Henle, Apr 13 '15 at 23:53
Note that because the `printf("FREEING...");` doesn't have a newline `'\n'` you may not see that output before the crash. — user3386109, Apr 13 '15 at 23:54
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, Apr 13 '15 at 23:54
About how _big_ is the file? Is the size exceeding `INT_MAX`? Note: Ideally the size variable should be type `size_t` and not `int`. Of course, checking the return values from your memory allocations is wise too - to insure they are not `NULL`. — chux - Reinstate Monica, Apr 14 '15 at 00:01
Maybe.... the issue is not here. But is the unposted calling code. You did say "after it reads the last line it crashes". Try `printf("FREEING...");` --> `printf("FREEING...\n");` (Add \n to flush buffer) — chux - Reinstate Monica, Apr 14 '15 at 00:06

Andrew Henle · Answer 1 · 2015-04-14T00:15:34.417

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.

If the file is opened in text mode, then `sb.st_size` may be more than `bytesRead`. `sb.st_size` represent the file's true byte size, whereas file input may translate line ending like `"\r\n"` to `"\n"`. — chux - Reinstate Monica, Apr 14 '15 at 00:08
@chux: Yep. I missed that. Arrrgh. I hate that translation - it makes it impossible to know you got every byte. Thanks, I updated the code I posted. — Andrew Henle, Apr 14 '15 at 00:14

fgets() causing segmentation fault when reading file

1 Answers1