I have a long list of single bits uint32 numbers. Sometimes they are || together to form a new number. For each set bit in this new number, I need to perform a certain operation. So basictly I'm writing a loop to iterat through each bit of the number.
I know that I can do this easily with a while loop. But since I need to this same thing in many parts of hte code. I figure I can make a function for it (just like the json_object_foreach() function).
For the function arguments, I'm parsing in the original number, the starting bit's position (assuming starting from the 0th bit, but if it doesn't make sense, I can also starting from 1), and what the numerical value this bit is.
I have the following so far. The code does compile. However the code is not iterating from 0-31. I'm pretty sure the get_next_bit_position function is not right bit I don't know how to fix it.
#include <stdio.h>
#include <stdbool.h>
#include <stdint.h>
uint32_t get_value_of_kth_bit(uint32_t original_number, int bit_position)
{
int result = 0;
// if the bit_position is set to 1
printf("original_number is: %u\n", original_number);
printf("bit_position is: %d\n", bit_position);
if (original_number & (1 << (bit_position))){
// if the bit_position is set to 1
// calculate the numerical value
if(bit_position == 0){
result = 1;
}
while (bit_position!=0){
result*=2;
--bit_position;
}
printf("result: %d\n", result);
return result;
} else {
return 0;
}
}
uint32_t get_next_bit_position(uint32_t bit_position)
{
printf("bit_position input is : %u\n", bit_position);
if(bit_position < 32){
bit_position++;
}
printf("new bit_position is : %u\n", bit_position);
return bit_position;
}
#define foreach_bit(original_number, bit_position, current_bit_value) \
for(current_bit = 0; \
current_bit_value = get_value_of_kth_bit(original_number, bit_position); \
current_bit = get_next_bit_position(current_bit))
#define CLASS_A 0x00000001 // 0001
#define CLASS_B 0x00000002
#define CLASS_C 0x00000010 // 1010
#define CLASS_D 0x00000400
//...
// the list goes on
int main(){
uint32_t num = CLASS_A | CLASS_C; // 1011
uint32_t current_bit = 0;
uint32_t current_bit_value = 0;
foreach_bit(num, current_bit, current_bit_value){
printf("Entered function\n");
if(current_bit_value = CLASS_A){
printf("Found class_a bit\n");
// do something else
}
else if (current_bit_value = CLASS_B){
printf("Found class_b bit\n");
// do something else
}
else if (current_bit_value = CLASS_D) {
printf("Found class_c bit\n");
// do something else
}
}
}
The code outputs the following, which it's weird. Since I think the function should enter the foreach_bit function before running get_value_of_kth_bit
original_number is: 17
bit_position is: 0
result: 1
Entered function
Found class_a bit
bit_position input is : 0
new bit_position is : 1
original_number is: 17
bit_position is: 1
Update about requirements:
#define CLASS_A 0x00000001 // 0001
#define CLASS_B 0x00000002
#define CLASS_C 0x00000004 // 0100
#deifne CLASS_D 0x00000008
#define CLASS_E 0x00000010
#define CLASS_F 0x00000020
....
get_class_name_str(uint32 class_number)For example, the original_number is 5 (binary: 0101). I check the first bit (right most) which is 1 (0001). And 0001 is the value for CLASS_A. So I see that the CLASS_A is in this original_number, and I can parse it to the helper funciton to do somethine. Then, the second bit it set to 0, which means CLASS_B is not in this original_number. The third bit is 1, which means CLASS_C is a part of original_number.
I can do this in a while loop, to check each bits. However, I would like this to be a funcytion itself since it's used in mutiple places.
So basictly I'm writing a loop to iterat through each bit of the number.
I think you are overcomplicating it. Just literally iterate over the bits, and exclude bits that are not set:
for (unsigned i = 0; i < 32; ++i) {
uint32_t bit = 1u << i;
if (!(num & bit)) continue;
// use bit
}
Which we can squeeze in that macro:
#include <stdio.h>
#include <stdint.h>
#define foreach_bit(BIT, ITER, NUM) \
for (uint32_t BIT, ITER = 0; ITER < 32 && (BIT = 1u << ITER, 1); ++ITER) \
if (!(NUM & BIT)) \
continue; \
else
int main() {
uint32_t val = 0b1011;
foreach_bit(i, _in, val) {
printf("%x\n", i);
}
}
The code snippets outputs:
1
2
8
Then we could even remove the need for bit position by checking if the mask will be zero when shifted - it will only be, when the last bit is taken/ The following looks quite nice, and I think I would prefer it:
#include <stdio.h>
#include <stdint.h>
#define foreach_bit(BIT, NUM) \
for (uint32_t BIT = 1; BIT << 1; BIT <<= 1) \
if (!(NUM & BIT)) \
continue; \
else
int main() {
uint32_t val = 0b1011;
foreach_bit(i, val) {
printf("%x\n", i);
}
}
Another idea, you do not need really "bit position" - you could remove the visited bits from the input instead, calculating the mask from the first bit set in the input:
#include <stdio.h>
#include <stdint.h>
#include <limits.h>
#include <strings.h>
#define foreach_bit(IDX, STATE, VAL) \
for (uint32_t STATE = VAL, IDX; \
STATE ? (IDX = 1 << (ffsl((long)STATE) - 1)) : 0; \
STATE &= ~IDX)
int main(){
uint32_t num = 0b1011;
foreach_bit(i, _in, num) {
printf("%x\n", i);
}
}
In your code, you check only once for if (original_number & (1 << (bit_position))){ - you have to check for that inside while (bit_position!=0){ loop and start checking from the bit last visited.
Anyway, there is very little point in writing any loop in the presented code. Just check the bits:
int main(){
uint32_t num = CLASS_A | CLASS_C
if (num & CLASS_A){
printf("Found class_a bit\n");
// do something else
}
else if (num & CLASS_B){
printf("Found class_b bit\n");
// do something else
}
else if (num & CLASS_D) {
printf("Found class_c bit\n");
// do something else
}
}