3

I have a partially working function which involves writing to a file.

I have an array, arr, of type unsigned short int and each element must be written to a file in binary format.

My inital solution was:

for(i = 0; i < ROWS; i++) {
    fwrite(&arr[i], 1, sizeof(unsigned short int), source);
}

The code above works when writing unsigned short ints to the file. Also, source is a pointer to the file which is being written to in binary format. However, I need to swap the bytes, and am having trouble doing so. Essentially, what is written to the file as abcd should be cdab.

My attempt:

unsigned short int toWrite;
unsigned short int swapped;
for(i = 0; i < ROWS; i++) {
    toWrite = &arr[i];
    swapped = (toWrite >> 8) | (toWrite << 8);

    fwrite(swapped, 1, sizeof(unsigned short int), source);
}

However I get a segmentation fault core dump as a result. I read and used the upvoted answer to this question - convert big endian to little endian in C [without using provided func] - but it doesnt seem to be working. Any suggestions? Thanks!

Jonny Henly
  • 4,023
  • 4
  • 26
  • 43
rubyquartz
  • 319
  • 1
  • 2
  • 11
  • 3
    `fwrite(&swapped, 1 , sizeof(unsigned short int) , source);` fwrite needs an address. compiler has probably warned you. read the warnings. also `toWrite = arr[i];` or you swap the address... – Jean-François Fabre Mar 29 '18 at 14:03

2 Answers2

3

your attempt is very wrong (and the answers you copied from are okay, the problem isn't in the swapping itself)

First you're taking the address of the value to swap, then you're passing the value instead of the address to write. It should be:

unsigned short int toWrite;
unsigned short int swapped;
for(i = 0; i < ROWS; i++){
        toWrite = arr[i];
        swapped = (toWrite >>8) | (toWrite <<8); // that part is OK
        fwrite(&swapped, 1 , sizeof(unsigned short int) , source);  
}

I'm positive that the compiler warned you for this. Warnings are useful.

Jean-François Fabre
  • 137,073
  • 23
  • 153
  • 219
3

Welcome to the world of non portable binary formats.

Swapping the numbers is a hack that is error prone because it makes the program non portable: whether to swap the values or not depends on the endianness of the system where you compile and run the program. Your program has a very simple bug: you pass the value of swapped instead of its address. You can fix it with:

fwrite(&swapped, sizeof(swapped), 1, source);

Yet a better solution for your problem is to handle endianness explicitly in your program. This is a portable solution to write the numbers in big endian order:

/* writing 16 bit unsigned integers in big endian order */
for (i = 0; i < ROWS; i++) {
    putc(arr[i] >> 8, source);
    putc(arr[i] & 255, source);
}

This is the alternative version if you are expected to write in little endian order:

/* writing 16 bit unsigned integers in little endian order */
for (i = 0; i < ROWS; i++) {
    putc(arr[i] & 255, source);
    putc(arr[i] >> 8, source);
}

Note that it is somewhat confusing to name the stream variable for an output file source.

chqrlie
  • 131,814
  • 10
  • 121
  • 189