C: Writing and Reading a string to and from a binary file

26,616

Solution 1

You are doing wrong while reading. you have put the & for the pointer variable that's why it gives segmentation fault.

I removed that it works fine and it returns Hello correctly.

int count = fread(drumReadString, sizeof(char), stringLength, fp);

Solution 2

I see a couple of issues, some problematic, some stylistic.

  • You should really test the return values from malloc, fread and fwrite since it's possible that the allocation can fail, and no data may be read or written.
  • sizeof(char) is always 1, there's no need to multiply by it.
  • The character array "Hello\0" is actually 7 bytes long. You don't need to add a superfluous null terminator.
  • I prefer the idiom char x[] = "xxx"; rather than specifying a definite length (unless you want an array longer than the string of course).
  • When you fread(&drumReadString ..., you're actually overwriting the pointer, not the memory it points to. This is the cause of your crash. It should be fread(drumReadString ....

Solution 3

A couple of tips:

1

A terminating \0 is implicit in any double quote string, and by adding an additional at the end you end up with two. The following two initializations are identical:

char str1[6] = "Hello\0";
char str2[6] = { 'H', 'e', 'l', 'l', 'o', '\0', '\0'};

So

char drumReadString[] = "Hello";

is enough, and specifying the size of the array is optional when it is initialized like this, the compiler will figure out the required size (6 bytes).

2

When writing a string, you might just as well just write all characters in one go (instead of writing one by one character sizeOfString times):

fwrite(drumCString, sizeOfString, 1, fp);

3

Even though not so common for a normal desktop pc scenario, malloc can return NULL and you will benefit from developing a habbit of always checking the result because in embedded environments, getting NULL is not an unlikely outcome.

char *drumReadString = malloc(sizeof(char) * stringLength);
if (drumReadString == NULL) {
        fprintf(stderr, "drumReadString allocation failed\n");
        return;
}
Share:
26,616
Aran Mulholland
Author by

Aran Mulholland

Full stack, Javascript, HTML, .NET, .NET Core, ASP.NET MVC, Project Coding Infrastructure, iOS, neo4j development, analogue synthesis, toilet photographer.

Updated on March 10, 2020

Comments

  • Aran Mulholland
    Aran Mulholland over 3 years

    I want to store strings in a binary file, along with a lot of other data, im using the code below (when i use it for real the strings will be malloc'd) I can write to the file. Ive looked at it in a hex editor. Im not sure im writing the null terminator correctly (or if i need to). when i read back out i get the same string length that i stored, but not the string. what am i doing wrong?

    FILE *fp = fopen("mybinfile.ttt", "wb");
    
    char drumCString[6] = "Hello\0";
    printf("%s\n", drumCString);    
    //the string length + 1 for the null terminator
    unsigned short sizeOfString = strlen(drumCString) + 1;
    fwrite(&sizeOfString, sizeof(unsigned short), 1, fp);
    
    //write the string
    fwrite(drumCString, sizeof(char), sizeOfString, fp);
    
    fclose(fp);
    
    fp = fopen("mybinfile.ttt", "rb");  
    
    unsigned short stringLength = 0;
    fread(&stringLength, sizeof(unsigned short), 1, fp);
    
    char *drumReadString = malloc(sizeof(char) * stringLength);
    int count = fread(&drumReadString, sizeof(char), stringLength, fp);
    
    //CRASH POINT
    printf("%s\n", drumReadString);
    
    fclose(fp); 
    
  • Aran Mulholland
    Aran Mulholland over 13 years
    thank you, its been a while, ive been living in managed code land. comments much appreciated.