CODEWITHSUNDEEP
c
Adam S
Adam S

Reputation: 9235

Segmentation fault when reading from a file

I have the following function:

void read_file(char* path, char** data)
{
    FILE*   file = NULL;
    size_t  size, result = 0;

    *data = NULL;
    file = fopen(path, "rb");

    if (file == NULL)// error opening file
    {
        return;
    }

    fseek(file, 0, SEEK_END);
    size = ftell(file) + 1;
    rewind(file);

    *data = (char*)malloc(size);
    if(*data == NULL)
        return;

    result = fread(*data, 1, size, file);
    if (result != size - 1)// error reding file
    {
        *data = NULL;
    }
    printf("LINE=%u\n", __LINE__);
    (*data)[size-1] = '\0';

    printf("LINE=%u\n", __LINE__);
    fclose(file);
    return;
}

I am getting a Segmentation fault on the line right in between the two printf("LINE=%u\n", __LINE__); statements. I don't understand why this is. When I'm looking at this line, it seems (*data) would have a type of (char *) which should certainly be able to be used with the index operator [].

What am I missing?

Upvotes: 1

Views: 1573

Answers (4)

John Bode
John Bode

Reputation: 123458

Here's the probable source of the problem:

if (result != size - 1)// error reding file
{
    *data = NULL;
}
printf("LINE=%u\n", __LINE__);
(*data)[size-1] = '\0';

What happens if there is an error reading the file? You set *data to NULL, and then immediately try to dereference it - bad juju.

Note that this also results in a memory leak; you don't free the memory that *data points to.

Restructure your code so that (*data)[size-1] = '\0' is executed only if the read operation was successful:

if (result != size - 1)
{
  free(*data);
  *data = NULL;
}
else                    
{                       
  (*data)[size-1] = 0;  
}

Upvotes: 0

RushPL
RushPL

Reputation: 4805

I have tested your code and it works for me - I have added returning of file's size to properly pass the data to fwrite.

> ./a.out arm-2010.09-good.tar.bz2 | sha1sum && sha1sum arm-2010.09-good.tar.bz2 
alloc size of 37265592
6bdff517bcdd1d279fc84ab3a5fbbca34211a87c  -
6bdff517bcdd1d279fc84ab3a5fbbca34211a87c  arm-2010.09-good.tar.bz2

furthermore Valgrind reports no warning and errors so .. loooks OK!

#include <stdio.h>
#include <stdlib.h>

size_t read_file(char* path, char** data)
{
    FILE*   file = NULL;
    size_t  size, result = 0;

    *data = NULL;
    file = fopen(path, "rb");

    if (file == NULL)// error opening file
    {
        return 0;
    }

    fseek(file, 0, SEEK_END);
    size = ftell(file) + 1;
    rewind(file);

    fprintf(stderr, "alloc size of %i\n", size);
    *data = (char*)malloc(size);
    if(*data == NULL)
        return 0;

    result = fread(*data, 1, size, file);
    if (result != size - 1)// error reding file
        *data = NULL;
    (*data)[size-1] = '\0';
    size--; // report file size

    fclose(file);
    return size;
}

int main(int argc, char** argv)
{
    char* data;
    if(argc<2)
        return 0;
    size_t siz = read_file(argv[1], &data);
    if(data) {
        fwrite(data, 1, siz, stdout);
        free(data);
    }
    else {
        fprintf(stderr, "No data returned\n");
    }
    return 0;
}

Upvotes: 0

AndersK
AndersK

Reputation: 36082

some pointers:

ftell returns -1 on failure, so if that is the case this will be 0 size = ftell(file) + 1;

size_t on some platforms is unsigned int, it may be good to have that in mind.

doing *data = NULL; is not a good idea, free it first free( *data );

put some if statements in your code to catch errors, don't assume everything will work e.g. assert( size>0 );

Upvotes: 1

Paul R
Paul R

Reputation: 212929

Probably the if (result != size - 1) test is failing and then you reset *data to NULL (which is a memory leak, BTW), and then you try to write to (*data)[size-1] - oops !

Upvotes: 5

Related Questions