Write int, short int or any integer value in a file in binary form using fputc

Question

I am trying to write binary values in a file using fputc. In my case it is a short int. The file must be opened in "w" mode.

I'm using this function, which is working. I need to know if I'm doing it right or if there are cases that I hadn't anticipated.

void writeListPos(Compressor* comp, short int listPos){
 ++listPos;
 char* posChar = (char*)(&listPos);
 int i;
 for(i=0 ; i<(sizeof(short int)) ; ++i){
 fputc(posChar[i] , comp->outputFile);
 }
}

The function to read the file later:

short int readListPos(Compressor* comp){
 char* posChar = (char*)(malloc(sizeof(short int)));
 int i;
 for(i=0 ; i<sizeof(short int) ; ++i){
 posChar[i] = comp->lastRead;/* comp->lastRead initialised with a first fgetc*/
 comp->lastRead = fgetc(comp->inputFile);
 }
 short int* positionp = (short int*)posChar;
 short int position = (*positionp);
 free(posChar);
 --position;
 return position;
}

• BLUEPIXY: 1) must be opened in "wb" mode. 2)not x86 has alignment problem. short int* positionp = (short int*)posChar;short int position = (*positionp); 3) fwrite & fread is better.
• Ingo Leonhardt: plus: you must have read at least one byte and have stored it in comp->lastRead before calling readListPos() for the first time
• TTAK: The problem is that with this system I can write a -1 (which is EOF) in the file which is pretty bad...
• Tomáš Zato: You cannot write EOF into file and you really shouldn't directly copy bytes of any datatype to a file. You must instead account for endianity.

Answer 1

Using malloc() and free() in readListPos() seems like overkill. You could use:

short s;
char *posChar = (char *)&s;
for (int i = 0; i < sizeof(s); i++)
{
 int c;
 if ((c = fgetc(comp->inputFile)) == EOF)
 ...handle EOF...
 *posChar++ = c;
}
return s;

Note that this code handles EOF, unlike the original; you must always error check your inputs.

This code matches the code that writes the data. It is, however, tied to a particular endian-ness; the written data is not portable between, say, an Intel machine and a SPARC or PowerPC machine. If that doesn't matter to you, then you're more or less OK.

For non-portable reading and writing of binary data, the fread() and fwrite() functions are simpler.

Reading:

short s;
if (fread(&s, sizeof(s), 1, comp->inputFile) != 1)
 ...process error/EOF...
return s;

Writing:

if (fwrite(&listPos, sizeof(listPos), 1, comp->outputFile) != 1)
 ...process error...

If you're doing it portably, you define whether the number is written big endian or little endian and then write the bytes in that order on both big endian and little endian machines. (If you can demonstrate it's a performance problem, you can optimize one of the two platform types, but it complicates the code for what is usually negligible benefit. ('Premature optimization is the root of all evil.')