4
\$\begingroup\$

The comment at the top was not followed because I printed the contents. I just want an opinion on coding style.

    /*
     * Write a function setbits(x,p,n,y) that returns x with the n bits
     * that begin at position p set to the rightmost n bits of y, leaving
     * the other bits unchanged in the least number of lines.
     *
     * Note: function signatures and curly brackets dont count towards 
     * the number of lines. You must also declare your variables, and 
     * call the function
     *
     * build with:
     *   gcc -o bit_twiddle -Wall -g -lm ./bit_twiddle.c
     */

    #include <stdio.h>
    #include <math.h>
    #include <limits.h>

    unsigned setbits(unsigned x, unsigned p, unsigned n, unsigned y) {
        x |= (y & ~(~0 << n)) << p;
        size_t s = (int)(log(INT_MAX)/log(2)) + 1;
        printf("An int is %d bits\n", s);
        int mask = pow(2.0, (int)s);
        do {
            ((x & mask) == 0) ? printf("0") : printf("1");
            ((s%4)==0) ? printf(" ") : printf("");
            x <<= 1;
        } while (s--);
        printf("\n");
    }

    void main( ) {
        unsigned retrn=1, begin=3, nbits=3, input=7;
        unsigned x = setbits(retrn, begin, nbits, input);
    }

UPDATE

x |= (y & ~(0 << n)) << p --> x |= (y & ~(~0 << n)) << p
\$\endgroup\$
  • 2
    \$\begingroup\$ Not sure this does anything: (0 << n) \$\endgroup\$ – Martin York Nov 16 '11 at 2:59
7
\$\begingroup\$

I think your code does not exactly what the comment wants. In line:

x |= (y & ~(0 << n)) << p;

0 << n is 0. If you want n rightmost bits of y you may use a mask like (1<<n)-1 (which is n 1 bits). Now on your code style, when you are coding a bit manipulating task, you'd better do it completely with bitwise operators (at least use other functions very few). Here in this code, using log and pow can be avoided. If you want to get number of bits in an integer type, you can use sizeof, like below:

size_t s = sizeof(int) << 3;

here << 3 is equal to * 8, because each byte definitely has 8 bits (it works on integer types of any size, just replace int with any other type like short or long long).

Now instead of using pow(2, s) you can write 1 << s :)

And about your printing, you can replace s%4 with s&3 (think about it). And I prefer shifting down mask instead of shifting up x (here it doesn't make any difference, but when you work with signed values it does, because sign will be preserved in left shifts). And for reducing number of lines for printing, you may use for loop.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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