K&R 2.6 Exercise setbits(x,p,n,y) function 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.


#include &lt;stdio.h&gt;

unsigned setbits(unsigned x, int p, int n,unsigned y);

int main(void){
    unsigned result=setbits(0, 4, 3, ~0);
    printf("%d\n",result);
    return 0;
}


unsigned setbits(unsigned x, int p, int n,unsigned y){
    unsigned ybits = y &amp; ~(~0 &lt;&lt; n);
    unsigned helper = ~(~0 &lt;&lt; n);
    unsigned result = (x | (helper &lt;&lt; (p+1-n))) &amp; ((ybits &lt;&lt; (p+1-n)) | (~(helper &lt;&lt; (p+1-n))));
    return result;
}


Output:


  28
Your example, which uses x = 0 and y = ~0, is not a very useful demonstration, as those degenerate cases would tend to let bugs remain undetected.



You have some common expressions that reappear several times, which should be given variable names:


(p+1-n) appears three times in result.  I would name it shift.
~(~0 &lt;&lt; n) appears in ybits, but it is the same as helper.  I would name it mask.

I would write it as (1 &lt;&lt; n) - 1 instead.  It's probably not any more or less efficient than ~(~0 &lt;&lt; n).  However, you use bitwise negation a lot.  When you write (~(helper &lt;&lt; (p+1-n))), that's a negation of a negation of a negation, which I find headache-inducing.


Conversely, you gave variable names to ybits and result, neither of which provide any benefit.



Your strategy is, in summary:


(x | ForceRelevantBitsOn) &amp; ((ybits &lt;&lt; ShiftToRelevantPlace) | OnesEverywhereElse)



However, that strategy requires you to construct OnesEverywhereElse.  I recommend the following simpler technique instead:

(x &amp; ForceRelevantBitsOff) | (ybits &lt;&lt; ShiftToRelevantPlace)




Here's how I would write it:

unsigned setbits(unsigned x, int p, int n, unsigned y) {
    int shift = p - n + 1;
    unsigned mask = (1 &lt;&lt; n) - 1;
    return (x &amp; ~(mask &lt;&lt; shift)) | ((y &amp; mask) &lt;&lt; shift);
}


The last line could be replaced with this expression, which might be more efficient (using five operations instead of six), but is more cryptic:

    return ((((x &gt;&gt; shift) ^ y) &amp; mask) &lt;&lt; shift) ^ x;


That works by determining which bits in x need to be toggled.