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

Question

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); 

Answer 1

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 ....

Answer 2

You don't write the terminating NUL, you don't need to but then you have to think about adding it when reading. ie malloc stringLength + 1 char, read stringLength chars and add a \0 at the end of what has been read.

Now the usual warning: if you are writing binary file the way you are doing here, you have lots of unstated assumptions which make your format difficult to port, sometimes even to another version of the same compiler -- I've seen default alignment in struct changes between compiler versions.

Answer 3

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);

Answer 4

Some more to add to paxdiablo and AProgrammer - if you are going to use malloc in the future, just do it from the get go. It's better form and means you won't have to debug when switch over.

Additionally I'm not fully seeing the use of the unsigned short, if you are planning on writing a binary file, consider that the unsigned char type is generally of size byte, making it very convenient for that purpose.

Answer 5

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;
}

Answer 6

You Just remove your &drumReadString in the fread function.You simply use drumReadString in that function as ganesh mentioned.Because,drumReadString is an array.Array is similar to pointers which point to the memory location directly.