fread can't read string value of a struct from a binary file in C

Question

I want to return a pointer to the head structure of linked list in following function but since my fread can't read struct's string, I have to malloc some variables and point newheader->name string to it. In following function if I free my temporary variables then returned new header only prints null values and If I don't then I get memory leaks but function works fine. I want to be able to return head of a complete linked list not null value.

struct head *read_strings(char *file) {
    FILE *my_file;
    struct head * newheader = NULL;
    char *str = NULL; 
    newheader = buildhead();

    int len, lenStr, lenTotal = 0;
    printf("\n\n");
    my_file = fopen(filename, "rb"); 
    if (my_file == NULL) {
        printf("error\n");
    }
    fread(&len,sizeof(int),1, my_file);
    newheader->name = malloc(len);
    fread(newheader->name,sizeof(char),len, my_file);
    fread(&newheader->length,sizeof(int),1, my_file);

    if (newheader->length != 0) {
        if (newheader->length == lenTotal) {
            fread(&lenStr,sizeof(int),1, my_file);
            str = malloc(lenStr); //char *str = malloc(lenStr);
            fread(str,sizeof(char),lenStr, my_file);
            create_string(newheader, str);
            lenTotal += lenStr;
            str = NULL; //free(str);
        }
        else {
            while (newheader->length != lenTotal) {
                fread(&lenStr,sizeof(int),1, my_file);
                str = malloc(lenStr); //char *str = malloc(lenStr);
                fread(str,sizeof(char),lenStr, my_file);
                create_string(newheader, str);
                lenTotal += lenStr;
                str = NULL; //free(str);
            }
        }
    }

    printString(newheader);
    fclose(my_file);
    free(str);
    //if i free newheader->name here then I get no memory leaks
    //free(newheader->name);
    freeStructure(newheader);
    return newheader;
}

I am required to read strings from binary file and store them into struct. But I can't store values directly to string variable of struct unless I malloc a new string and point to it. You can see in above code that I am able to read newheader->length from fread but not newheader->name. I tried to put fread results in a string array but got segmentation fault.

This is how my binary file looks like. It has null terminator after every string and int.

0000000 021  \0  \0  \0   i   t       i   s       a       g   o   o   d
0000020       d   a   y  \0   R  \0  \0  \0   #  \0  \0  \0   I   t    
0000040   w   a   s       t   h   e       b   e   s   t       o   f    
0000060   o   y   e       h   o   y   e       t   i   m   e   s   .  \0
0000100 034  \0  \0  \0   I   t       w   a   s       t   h   e       b
0000120   l   u   r   s   t       o   f       t   i   m   e   s   .  \0
0000140  \a  \0  \0  \0   m   o   n   k   e   y  \0 006  \0  \0  \0   p
0000160   a   n   d   a  \0 006  \0  \0  \0   s   h   i   f   u  \0
0000177

If I free newheader->name in the my freestruct() function then I get following error

*** Error in `./test': munmap_chunk(): invalid pointer:
0x000000000040133e *** Aborted.

if don't free it then I lose 17 bytes in 1 block.

==24169== HEAP SUMMARY:
==24169==     in use at exit: 0 bytes in 0 blocks
==24169==   total heap usage: 5 allocs, 6 frees, 1,201 bytes allocated
==24169== 
==24169== All heap blocks were freed -- no leaks are possible
==24169== 
==24169== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from0)
==24169== 
==24169== 1 errors in context 1 of 1:
==24169== Invalid free() / delete / delete[] / realloc()
==24169==    at 0x4C29E90: free (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==24169==    by 0x401298: freestruct (A1.c:267)
==24169==    by 0x400F8A: write_strings (A1.c:189)
==24169==    by 0x400840: main (test.c:25)
==24169==  Address 0x40133e is not stack'd, malloc'd or (recently)
free'd
==24169== 
==24169== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from0)

This is small piece of code from my freestruct() function

if (header->next == NULL) {
    header->length = 0;
    free(header->name);
    free(header);
}

After writing string to binary file, I freed the whole structure and then created again in read_strings function

Edit: If I free newheader->name inside readString function then I get no memory leaks but I want to be return newheader to main and call printString and freestruct after that.

I apologize for my bad grammar.

Use a debugger to step through the code line by line while monitoring variables and their values. Also, you use a function called setName, what does it do? And what does getName do? Do you need to set the structures name member twice? — Some programmer dude
And an unrelated query, do the string (and its length) in the file contain the string terminator? — Some programmer dude
@JoachimPileborg setname just assigns any string sent to it, to header->name and getname just returns that — samad bond

Some programmer dude Some programmer dude · Accepted Answer · 2016-09-25T05:30:27

If I understand your setName and getName functions correctly, then these three lines are the problem:

...
setName(newheader, name);
newheader->name = getName(newheader);
...
free(name);

First of all the two first lines do the same thing, but that's not problematic, just redundant. It's the last line there that causes the problem.

You make newheader->name point to the same memory that name points to. After the assignment both pointers are pointing to the same memory. Then you free the memory. That makes both pointers invalid.

You either need to duplicate (with e.g. the non-standard but common strdup function). Or not free the memory.

Lets see if I can make it clearer...

You have a variable name for which you allocate memory. It will look something like this:

+------+     +---------------------+
| name | --> | memory for the name |
+------+     +---------------------+

What the assignment

newheader->name = name;

actually does is just copy the pointer, not the memory it points to. So after the assignment you have something like this:

+------+
| name | ------------\
+------+              \    +---------------------+
                       >-> | memory for the name |
+-----------------+   /    +---------------------+
| newheader->name | -/
+-----------------+

You now have two variables pointing to the same memory.

Then you do

free(name);

This free's the memory, but remember that we have two variable pointing to the same memory. Now it looks like

+------+
| name | -------------> ?
+------+           

+-----------------+
| newheader->name | --> ?
+-----------------+

So whenever you try to access newheader->name you will try to access the now free'd memory, and you will have undefined behavior.

fread can't read string value of a struct from a binary file in C

1 Answers