Reputation: 23
I have a 32-bit unsigned int and I need to extract bits at given positions and make a new number out of those bits. For example, if I have a 0xFFFFFFFF and want bits 0,10,11 my result will be 7 (111b).
This is my attempt, it extracts the bits correctly but doesn't create the correct result. I'm shifting the result 1 place left and ANDing it with my extracted bit, apparenlty this is incorrect though?
I'm also sure there is probably a much more elegant way to do this?
#define TEST 0xFFFFFFFF
unsigned int extractBits(unsigned short positions[], unsigned short count, unsigned int bytes)
{
unsigned int result = 0;
unsigned int bitmask = 0;
unsigned short bit = 0;
int i = 0;
for(i = 0; i < count; i++) {
bitmask = 2 << (positions[i] -1);
if(bitmask == 0) bitmask = 1;
bit = bytes & bitmask;
bit = bit >> positions[i];
result = result << 1;
result = result & bit;
}
if(result != 31) {
printf("FAIL");
}
return result;
}
int main(void)
{
unsigned short positions[5] = {8, 6, 4, 2, 0};
unsigned int result = extractBits(positions, 5, TEST);
printf("Result: %d\n", result);
return 0;
}
Upvotes: 2
Views: 1965
Reputation: 8701
Since you are picking off individual bits, there's no reason to make the bit mask a variable; just shift the desired bit into the units bit, and use a mask of 1. E.g.:
...
result = (2*result) | ((bytes >> positions[i]) & 1);
...
Many compilers generate the same code for 2*result
and result<<1
, so use whichever you like.
Note, if you are designing the interface and don't have good reasons for using short
integers for positions[]
and count
as you do, then don't. Be consistent and specify all the integers the same way.
Upvotes: 1
Reputation: 20044
Beware, untested code:
for(i = 0; i < count; i++)
{
bitmask = 1 << positions[i];
bit = (bytes & bitmask)!=0;
result = (result << 1)|bit;
}
Upvotes: 2