CODEWITHSUNDEEP
cbit-manipulationbit
John
John

Reputation: 49

bitwise operators and shifts issues in c

Hello I am having some trouble with the bitwise operators and shifts. I believe check_flag() is working however set_flag() is not. Could someone explain what's wrong with it?

#include <stdio.h>

void set_flag(int* flag_holder, int flag_position);
int check_flag(int flag_holder, int flag_position);

int main(int argc, char* argv[])
{
    int flag_holder = 0;
    int i;
    set_flag(&flag_holder, 3);
    set_flag(&flag_holder, 16);
    set_flag(&flag_holder, 31);
    for(i=31; i>=0; i--)
    {
        printf("%d", check_flag(flag_holder, i));
        if(i%4 == 0)
        {
            printf(" ");
        }
    }
    printf("\n");
    return 0;
}

void set_flag(int* flag_holder, int flag_position)
{
    *flag_holder = *flag_holder |= 1 << flag_position;
}

int check_flag(int flag_holder, int flag_position)
{
    int bit = (flag_holder << flag_position) & 1;
    if(bit == 0)
        return 0;
    else
        return 1;

    return bit;
}

Upvotes: 1

Views: 281

Answers (1)

ad absurdum
ad absurdum

Reputation: 21318

You need to change the type of flag_holder to unsigned. Assuming that your ints are 32 bits wide, when you set the high-order bit (position 31), you are setting the sign bit. This causes implementation-defined behavior for right bit-shifts, and undefined behavior for left bit-shifts. The set_flag() function should be changed to:

void set_flag(unsigned* flag_holder, int flag_position)
{
    *flag_holder |= (1U << flag_position);
}

This shifts a bit into position before setting the bit at flag_position in flag_holder. 1U is an unsigned int with only the lowest-order bit set; (1U << flag_position) shifts the single set bit flag_position places to the left. The |= operator is equivalent to assigning the result of a bitwise or, of *flag_holder and the shifted bit, to *flag_holder. This could also have been written as:

*flag_holder = *flag_holder | (1U << flag_position);

There is also a problem in check_flag(). The bit-shifting code needs to change to:

int bit = (flag_holder >> flag_position) & 1U;

This shifts the bit of interest to the lowest-order position before extraction with the & operator. The way that it was written, with a left-shift, the & was always comparing with a 0 bit.

Upvotes: 2

Related Questions