Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm studing C on K&R and I solved exercise 2.08 ("Write a function rightrot(x,n) that returns the value of the integer x rotated to the right by n positions") with the code below. I've tested my code with some bit patterns and it seems to work, but I'm not sure that this solution covers all possible cases. What do you think about this code?

unsigned rightrot(unsigned x, int n)
{   
    int size;
    unsigned y;

    size = 0;
    y = x;

    while (y != 0) {
    y = y << BYTESIZE;
    size++;
    }
    size = size * BYTESIZE;

    return (x << (size-n)) | (x >> n);
}

Here is the main

#include <stdio.h>

#define BYTESIZE 8

unsigned rightrot(unsigned x, int n);

int main(void)
{
     unsigned x;
     int n;

     x = 0x23acb;
     n = 2;

     printf("%x\n", rightrot(x, n));

     return 0;
}
share|improve this question
add comment

2 Answers

up vote 1 down vote accepted

@William has the optimum solution.

But some comments on your code:

// There is already a macro that defines the number of bits
// in a byte it is called CHAR_BIT
#define BYTESIZE 8

    // Initialize variables as you declare them   
    int size;
    unsigned y;

    // You can find the number of bytes in an object using
    // sizeof(<expression>)    
    while (y != 0) {
    y = y << BYTESIZE;
    size++;
    }

    // Thus the number of bits is:
    // sizeof(y) * CHAR_BIT
    size = size * BYTESIZE;

    // The result is the same.
    return (x << (size-n)) | (x >> n);
share|improve this answer
add comment

Isn't it something like this?

#include <limits.h> /* for CHAR_BIT */

unsigned
rotate_right(unsigned x, int n)
{
    int left_shift = ((sizeof x) * CHAR_BIT) - n;
    return x<<left_shift | x>>n;
}
share|improve this answer
 
I'm aware of the existence of built in sizeof function but I can not use it because formally in the book it has not yet been explained. :-) Is it correct to calculate the size of x as I did or am I missing something? –  cimere Jan 28 '12 at 22:27
1  
No. In your solution, if x==0, size is 0. Try for(size=0, y=~0; y; y<<=1, size++); (note the ; at the end of that) –  William Morris Jan 30 '12 at 13:13
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.