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
– Some programmer dude, Commented Sep 25, 2016 at 5:12
And an unrelated query, do the string (and its length) in the file contain the string terminator? — Some programmer dude
– Some programmer dude, Commented Sep 25, 2016 at 5:13
@JoachimPileborg setname just assigns any string sent to it, to header->name and getname just returns that — samad bond
– samad bond, Commented Sep 25, 2016 at 5:23

Some programmer dude · Accepted Answer · 2016-09-25 13:02:40Z

0

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.

edited Sep 25, 2016 at 13:02

answered Sep 25, 2016 at 5:30

Some programmer dude

411k36 gold badges420 silver badges655 bronze badges

Sign up to request clarification or add additional context in comments.

10 Comments

samad bond Over a year ago

setName and getName are just help functions in linked list. I can replace them with newheader->name = name; I tried using strcpy(newheader->name, name); but that also gave me seg fault

Some programmer dude Over a year ago

@samadbond Updated my answer. Hope it makes it clearer.

samad bond Over a year ago

thanks for detailed explanation. I understand whats happening but I can't come up with any other way to read string from binary file using fread. At the end of the function I am required to return header to the linked list. Whats any other solution for adding string to newheader->name

Some programmer dude Over a year ago

@samadbond You probably read the string fine (use a debugger to find out, or just print it after you read it). But what you should not do is free the memory the string is stored in, you need to keep it and then free it when you free the whole structure.

samad bond Over a year ago

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

|

Collectives™ on Stack Overflow

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

1 Answer 1

10 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

10 Comments

Your Answer

Sign up or log in

Post as a guest

Related