CODEWITHSUNDEEP
c
Doug Skinner
Doug Skinner

Reputation: 47

Misunderstanding of binary file writing/reading in C

Can someone please explain why the following code segfaults whenever I attempt to do anything with either of the character arrays inside of the struct after I read it in from the binary file? The code is as follows:

struct my_struct {
    int filename_len;
    char *filename;
    int size;
    char *contents;
};

int main() {
    FILE *fp = fopen("test.bin", "wb");
    char *one = "list1";
    char *two = "test file";
    int one_len = strlen(one);
    int two_len = strlen(two);

    fwrite(&one_len, sizeof(int), 1, fp);
    fwrite(&one, one_len, 1, fp);
    fwrite(&two_len, sizeof(int), 1, fp);
    fwrite(&two, two_len, 1, fp);
    fclose(fp);



    struct x *temp = malloc(sizeof(struct x));
    fp = fopen("test.bin", "rb");

    fread(&(temp->filename_len), sizeof(int), 1, fp);
    fread(&(temp->filename), sizeof(char), temp->filename_len, fp);
    fread(&(temp->size), sizeof(int), 1, fp);
    fread(&(temp->contents), sizeof(char), temp->size, fp);

    // This  does not segfault
    printf("%d\n", temp->filename_len);

    // This does
   printf("%s\n", temp->filename);
   fclose(fp);

   return 0;
}

Thank you!

Upvotes: 0

Views: 56

Answers (2)

MFisherKDX
MFisherKDX

Reputation: 2866

1) You are calling fread with the address of temp->filename. 

fread(&(temp->filename), sizeof(char), temp->filename_len, fp)

You don't want to do this. temp->filename is already a pointer to a char. Instead pass simply temp->filename.

2) You aren't allocating any space for temp->filename. You need to do this after you read temp->filename_len.

fread(&(temp->filename_len), sizeof(int), 1, fp);
temp->filename = malloc((temp->filename_len + 1) * sizeof(char))
fread(temp->filename, sizeof(char), temp->filename_len, fp);
temp->filename[temp->filename_len] = '\0';

Similarly for temp->contents

Upvotes: 1

user149341
user149341

Reputation: 

fwrite(&one, one_len, 1, fp);

&one is the address of the pointer one on the stack, not the location it points to. This means that, instead of writing the string pointed to by one, you're writing the bytes that make up its address, potentially along with whatever happens to come after that in memory.

fread(&(temp->filename), sizeof(char), temp->filename_len, fp);

Similarly, this reads a pointer (or part of a pointer) into temp->filename, and may additionally overwrite whatever happens to be after it.

What you need to do is:

fwrite(one, one_len, 1, fp);

Write the actual contents of the string, not its address.

temp->filename = calloc(temp->filename_len + 1, sizeof(char));
fread(temp->filename, sizeof(char), temp->filename_len, fp);

Allocate a buffer sized appropriately for the filename, then read data into that buffer.

Upvotes: 2

Related Questions