1
\$\begingroup\$

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 <stdio.h>

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 & ~(~0 << n);
    unsigned helper = ~(~0 << n);
    unsigned result = (x | (helper << (p+1-n))) & ((ybits << (p+1-n)) | (~(helper << (p+1-n))));
    return result;
}

Output:

28

\$\endgroup\$
  • \$\begingroup\$ Are the 3 bits that start at bit 4 bits 4,5,6 or bits 4,3,2? \$\endgroup\$ – JS1 Nov 7 '16 at 6:17
3
\$\begingroup\$

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 << n) appears in ybits, but it is the same as helper. I would name it mask.

    I would write it as (1 << n) - 1 instead. It's probably not any more or less efficient than ~(~0 << n). However, you use bitwise negation a lot. When you write (~(helper << (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) & ((ybits << ShiftToRelevantPlace) | OnesEverywhereElse)

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

(x & ForceRelevantBitsOff) | (ybits << 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 << n) - 1;
    return (x & ~(mask << shift)) | ((y & mask) << 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 >> shift) ^ y) & mask) << shift) ^ x;

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

\$\endgroup\$
  • \$\begingroup\$ Nice improvement.Just there is a part i do not understand.You said that ~(~0 << n) is equal to (1 << n) - 1 but why ? Firstly 1 is not equal to ~0 right ? And secondly why do you substract it from 1 ? \$\endgroup\$ – Muhamed Cicak Nov 8 '16 at 17:41

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.