7
\$\begingroup\$

Could you suggest ways of making this more simple?

/*
 * Next write a function invert(x,p,n) that returns x with the n bits
 * that begin at position p inverted, leaving the others unchange
 */

void printbits(unsigned x) {
    size_t size_of_int = sizeof(int) << 3;
    unsigned mask = 1;
    int i = 0;
    for(i = 1; i <= size_of_int; ++i, x >>= 1) {
       ((x & mask) == 0) ? printf("0") : printf("1");
       ((i & 3)==0) ? printf(" ") : printf("%s","");
    }
    printf("\n");
}

void invert(unsigned x, unsigned p, unsigned n) {
    printbits(((~((((~(~0 << n) << p))) & x)) & (~(~0 << n) << p)) | (x & ~(~(~0 << n) << p)));
}

int main(int argc, char *argv[]) {
    unsigned input=3082676239, begin=15, nbits=5;
    invert(input, begin, nbits);
    return(0);
}

Before mushing the parts together this is what I get in output:

              x = 1111 0000 0001 0111 1011 1101 1110 1101 
===================================================================
          mask0 = 1111 0000 0001 0110 0000 1101 1110 1101 
          mask1 = 1111 1111 1111 1110 0100 1111 1111 1111 
          mask2 = 0000 0000 0000 0001 1111 0000 0000 0000 
===================================================================
         output = 1111 0000 0001 0110 0100 1101 1110 1101
\$\endgroup\$
1
\$\begingroup\$

Maybe:

void invert2(unsigned x, unsigned p, unsigned n) {
    printbits((~(~0 << n) << p) ^ x);
}
\$\endgroup\$
3
\$\begingroup\$

While @palacsint's code may work, it doesn't impress me as a model of clarity. I think I'd start from the fact that subtracting 1 from a number clears the least significant bit that was set, and sets all the less significant bits. If we start with a number that has only one bit set, that bit will be cleared, and all the less significant bits will be set. Based on that, getting a mask of N bits is pretty simple: take 1, shift it left N bits, and then subtract 1.

For example, consider creating a 5-bit mask in a 16-bit number:

0000 0000 0000 0001    // 1
0000 0000 0010 0000    // 1 << 5
0000 0000 0001 1111    // (1<<5)-1

Once we have that, we can shift it left p bits to get it to the right position, and xor with the input:

unsigned invert(unsigned x, unsigned p, unsigned n) { 
    unsigned mask = ((1u << n) - 1u) << p;
    return x ^ mask;
}

A couple minor points:

  1. I've removed printing from invert -- IMO, printing the result should be separate.
  2. This requires that n<size_of_int.

I think the printing can be simplified a bit as well. Although the "test for a multiple of 4" inside the loop does work, I think at least in this case a nested loop makes the intent clearer:

void print(unsigned x) { 
    static const int group_size = 4;
    int group, j;

    for (group = 0; group < size_of_int / group_size; group++) {
        for (j=0; j<group_size; j++, x >>= 1)
            printf("%c", (x & 1) + '0');
        printf(" ");
    }
}

Another possibility that might be worth considering would be a small lookup table to convert 4 bits at a time:

void print(unsigned x) { 
    static const int group_size = 4;
    // inverted order because you're printing the LSB first.
    static const char *outputs[] = {
        "0000", "1000", "0100", "1100", "0010", "1010", "0110", "1110", 
        "0001", "1001", "0101", "1101", "0011", "1011", "0111", "1111"
    };
    int group;

    for (group=0; group<size_of_int / group_size; group++, x >>= 4)
        printf("%s ", outputs[x & 0xf]);
}
\$\endgroup\$
  • \$\begingroup\$ +1 Jerry, I agree, your solution is much better and much more elaborated. To defend myself, my answer is only a quick note and it's still more simple than the one in the question. \$\endgroup\$ – palacsint Nov 18 '11 at 9:34
  • \$\begingroup\$ @palacsint: Yup -- it's definitely an improvement, and in fairness, it seems pretty clear that the code in the question is the source of most of the obfuscation. My remark was intended mostly as saying that what you posted was a big improvement, but I thought it was possible to do even a little better still. \$\endgroup\$ – Jerry Coffin Nov 18 '11 at 15:21

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.