-1

I have a below sample file where I read the binary into structure and print the length of the string which is stored in structure. But I get a segmentation fault core dumped when I try to print the full string. I could not find the reason for it.

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

struct sample {
        unsigned int m;
        unsigned int v;
        unsigned int s[128];
        int t_length;
        char *t;
};

int main()
{
        int i=0;
        unsigned int t_len=0;
        FILE *fp;
        struct sample sam;
        fp =fopen("sample.bin", "rb");
        if (fp==NULL) {
                printf("File not created\n");
                return -1;
        }
        fread(&sam, sizeof(sam), 1, fp);
        printf("t_length is %d\n", sam.t_length);
        t_len=sam.t_length;
        sam.t=(char *)malloc(sizeof(char) * t_len);
        fseek(fp,0,SEEK_SET);
        fread(&sam, sizeof(sam)+t_len, 1, fp);
        printf("t_length is %d\n", t_len);
        printf("%s\n", sam.t);
        fclose(fp);
        free(sam.t);
        return 0;

}
arceus
  • 327
  • 4
  • 15
  • 2
    You also present the program that created the binary file. – BLUEPIXY Sep 12 '17 at 06:20
  • @BLUEPIXY Sorry I cannot give it – arceus Sep 12 '17 at 06:24
  • 2
    Why have you added the C++ tag? Are you programming in C or in C++? They are two very different languages, so please edit your question to remove the language tag that's not relevant (probably the C++ tag, since your program is a pure C program). – Some programmer dude Sep 12 '17 at 06:31
  • @Someprogrammerdude OK – arceus Sep 12 '17 at 06:33
  • 1
    Sidenote: Abusing structs for serialization is generally considered bad practice, because of limited control on things like endianness or padding. It would be better to serialize each member separately. – user694733 Sep 12 '17 at 06:37
  • @user694733 Could you please explain more about serialization as I am unaware of it – arceus Sep 12 '17 at 06:41
  • compile with all warnings and debug info (`gcc -Wall -Wextra -g`) and use the debugger (`gdb`) to run your program step by step and watch relevant variables. Check result of `fread` – Basile Starynkevitch Sep 12 '17 at 06:46
  • 1
    @arceus You can start by reading [this wikipedia article](https://en.wikipedia.org/wiki/Serialization) or [this SO question](https://stackoverflow.com/q/6002528/694733).There are plenty of information on this on the net. – user694733 Sep 12 '17 at 07:00

1 Answers1

3

Consider the following lines:

    fread(&sam, sizeof(sam), 1, fp);
    // ...
    sam.t=(char *)malloc(sizeof(char) * t_len);
    // ...
    fread(&sam, sizeof(sam)+t_len, 1, fp);

You read the structure. You assign the t member. Then you read the structure again, overwriting the t member.

This will lead to the t member no longer pointing to the memory you allocated, and lead to undefined behavior when you dereference the pointer.

I suggest you do some research about flexible array members as that might solve your problem. This of course requires that the program writing the file also be changed to use a flexible array member.

If you can't modify the program writing the file, then don't re-read the structure. Read the structure once, allocate memory, then read only the data to be put in the memory you allocated. Skip the seek, as the file should be at the correct position anyway.


In code something like

fread(&sam, sizeof sam , 1, fp);
sam.t = malloc(samr.t_length);
fread(sam.t, sam.t_length, 1, fp);
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • I could not understand. Could you please explain more with a sample program. – arceus Sep 12 '17 at 06:26
  • @arceus What you're doing is roughly equivalent to `int a; a = 10; a = 5;` and then wonder why `a` is not equal to `10`. If you step through the code, line by line, in a debugger you can see the value of the pointer change. – Some programmer dude Sep 12 '17 at 06:28
  • OK. But I removed the first fread and I am still facing "Segmentation fault (core dumped)" – arceus Sep 12 '17 at 06:32
  • @arceus Then you don't read the `t_length` member. Keep the first, remove the second, remove the seek, then read the data into `sam.t` instead. – Some programmer dude Sep 12 '17 at 06:33
  • 1
    @arceus Yes. The first `fread` read the structure, *and* positions the file-pointer at the correct place to read the string. Then you allocate the memory. Then you read (without any seeking) the data to be placed in the allocated memory. – Some programmer dude Sep 12 '17 at 06:41
  • I did that. Now there is no seg fault but full data is not printed. i.e., string (sam.t) is not printed. – arceus Sep 12 '17 at 06:45
  • 3
    @arceus Then you need to see how the data was written to the file. ***And*** add error checking for the `fread` calls. It is a different problem though, that should warrant a separate question if you can't figure it out yourself. – Some programmer dude Sep 12 '17 at 06:45
  • Ya now after I followed ur edited code, I am getting " Second t_length as 0" and some characters from beginning of string is getting truncated. Rest all is fine. I want second t_length to be same as first. – arceus Sep 12 '17 at 06:52
  • 1
    @arceus Again, you ***need to know how the file was written***. You need to know its format. Perhaps the pointer of the structure isn't written? Perhaps the writer used flexible array members? And since my code doesn't set `t_len`, you changed to print `sam.t_length` the second time as well? And it's *still* a subject for another question. – Some programmer dude Sep 12 '17 at 06:54