2

I got problem solving 'reverseBytes' in cs:app datalab.

i have to make code that returns reversed byte order.

example : input=0x123456, returns=0x563412

when i used my code, it can't take a score..

int reverseBytes(int x) { int mask=0xff; int byte1=x>>24; int byte2=(x>>16)&mask; int byte3=(x>>8)&mask; int byte4=x&mask; int result=(byte4<<24)|(byte3<<16)|(byte2<<8)|(byte1); return result; }

but, when i used other people's code, it takes a score.

int reverseBytes(int x) { int t2=~(0xff<<24); int s1=(0xff<<16)+0xff; int s2=0xff<<8; int s3=(s2<<16)+s2; int temp=(x&s1)<<8|((x&s3)>>8&t2); int q1=(0xff<<8)+0xff; int q2=q1<<16; int temp2=(temp&q1)<<16|((temp&q2)>>16&(~q2)); return temp2; }

i don't know why my code can't works.. i tested my code and other people's code. but i can't find difference between my code's result and another code's result. please help me..

Improve this question
5
  • 1
    Have you looked at the result of applying your code to the example data you provided? It won't be the result you say is expected, but if you print it in hexadecimal form then it should be pretty obvious what's wrong. Commented Sep 25, 2015 at 16:30
  • On the other hand, the code you say does work will not produce the given example output either. Commented Sep 25, 2015 at 16:38
  • Read about htonl(3) Commented Sep 25, 2015 at 16:45
  • 2
    @BasileStarynkevitch I am not sure about this as OP wants reverse bytes regardless if they are le or be. htonl(3) will do nothing in latter case Commented Sep 25, 2015 at 17:43
  • 1
    Right shifting a negative value is implementation defined. Use unsigned types and do not rely on int/unsigned having 32 bits. Use uint32_t instead. Left shifting a signed value chainging th sign is undefined behaviour even. Commented Sep 25, 2015 at 18:02

5 Answers 5

Reset to default
5

Simple right shift to get the byte needed will do:

#include <stdio.h> #include <stdint.h> uint32_t reverse_bytes(uint32_t bytes) { uint32_t aux = 0; uint8_t byte; int i; for(i = 0; i < 32; i+=8) { byte = (bytes >> i) & 0xff; aux |= byte << (32 - 8 - i); } return aux; }

Test:

int main(void) { uint32_t input = 0x123456; printf("input: 0x%08x\n", input); input = reverse_bytes(input); printf("input: 0x%08x\n", input); return 0; }

prints:

input: 0x00123456

input: 0x56341200

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

4

use this:

#define reverse_bytes_32(num) ( ((num & 0xFF000000) >> 24) | ((num & 0x00FF0000) >> 8) | ((num & 0x0000FF00) << 8) | ((num & 0x000000FF) << 24) )
Improve this answer

Comments

0

The following functions swap bytes-strings of whichever length you want. The swapbytes() swaps all the bytes contained in the inp parameter inside inp thus modifying the contents of inp. The swapbytesout() swaps the contents of the inp paramenter inside the out parameter, then it doesn't modify the contents of inp.

#include <stdio.h> #include <stdint.h> #include <inttypes.h> void * swapbytes(void *inp, size_t len); void * swapbytesout(void * inp, void *out, size_t len); /* This function swaps and modifies the content of inp * * Returns: a pointer to the output value (inp) */ void * swapbytes(void *inp, size_t len) { unsigned int i; unsigned char *in=(unsigned char *)inp,tmp; for(i=0;i<len/2;i++) { tmp=*(in+i); *(in+i)=*(in+len-i-1); *(in+len-i-1)=tmp; } return inp; } /* This function doesn't swap and doesn't modify the content * of inp, the output is computed on out. * * Returns: a pointer to the output value (out) */ void * swapbytesout(void *inp, void *out, size_t len) { unsigned int i; unsigned char *o=(unsigned char *)out; unsigned char *in=(unsigned char *)inp; for(i=0;i<len;i++) { *(o+len-i-1)=*(in+i); } return out; } int main(void) { uint32_t a0,a1; uint64_t b0,b1; a0=0x12345678; printf("%08X\n",*(uint32_t *)swapbytesout(&a0,&a1,sizeof(a0))); printf("%08X %08X\n",a0,a1); printf("%08X\n",*(uint32_t *)swapbytes(&a0,sizeof(a0))); printf("%08X\n",a0); puts(""); b0=0x123456789ABCDEF0ULL; printf("%016"PRIX64"\n",*(uint64_t *)swapbytesout(&b0,&b1,sizeof(b0))); printf("%016"PRIX64" %016"PRIX64"\n",b0,b1); printf("%016"PRIX64"\n",*(uint64_t *)swapbytes(&b0,sizeof(b0))); printf("%016"PRIX64"\n",b0); return 0; }
Improve this answer

Comments

0

The shift operator preserves signing. Try changing "int byte1=x>>24;" to " "int byte1=x>>24&mask;". Or just switch to using unsigned ints.

Improve this answer

2 Comments

Well, in fact the result of the right shift of a negative number is implementation-defined. Either arithmetic or logical shift can be provided, or, technically, every such shift could produce the value 42. The implementation just needs to document its choice.
With that said, yes, the failure to mask the result of the shift is a good candidate for the source of the problem.
0

Here is my implementation of reversing the byte order for various number of bytes.

#include <stdint.h> #include <stdio.h> #include <stdlib.h> #include <string.h> void reverse_byte_order(void *data, uint32_t num_bytes) { void *dest = malloc(num_bytes); for (int i = 0; i < num_bytes; ++i) { memcpy(dest + num_bytes - i - 1, data + i, 1); } memcpy(data, dest, num_bytes); free(dest); } int main() { uint32_t data1 = 0x12345678; printf("0x%08x\n", data1); reverse_byte_order(&data1, 4); printf("0x%08x\n", data1); uint32_t data2 = 0xabcdef; printf("0x%08x\n", data2); reverse_byte_order(&data2, 3); printf("0x%08x\n", data2); return 0; }

The output is:

0x12345678 0x78563412 0xabcdef 0xefcdab

This implementation does not require bit shifting or AND/OR operation. It basically just copy the memory from place to place one by one. It looks more straightforward in my eyes though.

Improve this answer

Comments