CODEWITHSUNDEEP
cstructbinaryfilesfread
Nate Brennand
Nate Brennand

Reputation: 1508

Trouble using fread() to convert a binary file to a struct

I'm having trouble using fread() to convert a binary file into a linked list of structs.

The struct:

struct MdbRec {
    char name[16];
    char  msg[24];
};

Relevant code:

    FILE *file;
    file = fopen( argv[1], "rb" );

    struct List db;
    initList(&db);
    struct MdbRec *data = (struct MdbRec *)malloc(sizeof(struct MdbRec));
    struct Node *last;
    while( fread( data, 40, 1, file ) )
    {
            struct MdbRec *message = (struct MdbRec *)malloc(sizeof(struct MdbRec));
            message = data;
            if( !db.head )
            {
                    last = addFront( &db, message );
                    db.head = last;
            }
            else
                    last = addAfter( &db, last, message );
            if( fseek( file, 40, SEEK_CUR ) != 0)
                    break;
            printf("read\n");
    }
    free(data);
    removeAllNodes( &db );

addFront() and addAfter are methods of the linkedlist structure that mallocs space the data field.

When I run it with Valgrind it shows that I'm successfully having 2 allocations of memory. One is obviously the data variable. The other 568 bytes and it's very confusing to me. Valgrind says the error is coming from when I run fread().

Upvotes: 2

Views: 1567

Answers (2)

hmjd
hmjd

Reputation: 122011

This is a memory leak:

struct MdbRec *message = (struct MdbRec *)malloc(sizeof(struct MdbRec));
message = data;

as message is now pointing to data and no longer points to the just malloc()d memory, which is now unreachable. I suspect you actually meant to copy data to message:

*message = *data;

Other points:

  • Check result of fopen() before using it.
  • There appears to be no reason to not use a stack allocated object for data, which would avoid needless dynamic allocation management for it.
  • The fread() argument that specifies the size of the object to read, 40, is error prone. Any changes to struct MdbRec would break it: use sizeof(struct MdbRec) instead.
  • Casting the return value of malloc() is not necessary and possibly dangerous (see Do I cast the result of malloc?).

Upvotes: 1

quantum
quantum

Reputation: 3838

struct MdbRec *message = (struct MdbRec *)malloc(sizeof(struct MdbRec));
message = data;

You reallocated a struct MdbRec and discarded it.

Upvotes: 1

Related Questions