bitwise operators and shifts issues in c

Question

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;
}

That depends on what you want. Do you understand the meaning of operators |=, &, << ? Because now "1" is gone from that line. — ZenJ, Jan 26 '17 at 01:57
|=Checks if the values of the two are equal or not. & copies a bit to the result and << is left shift — John, Jan 26 '17 at 02:00
@ZenJ if you could explain it a bit I would greatly appreciate that — John, Jan 26 '17 at 02:03
So you got a left shift correctly. As for the rest: Writing `a |= b` is the same as writing `a = a | b`. Operator & does not copy anything, it only compares each and every bit of two numbers and for returns the result of such comparison. Anyways, I guess what you want is this: `*flag_holder |= 1 << flag_position;` — ZenJ, Jan 26 '17 at 02:07
Here is a tutorial for you: https://www.programiz.com/c-programming/bitwise-operators — ZenJ, Jan 26 '17 at 02:10
@zenJ my code originally said *flag_holder = *flag_holder&1 << flag_position; is that correct or... — John, Jan 26 '17 at 02:25
Probably not. Anything &1 won't change at all. The result for bit&0 is 0 for that bit (so it is used to set bits to 0). The result of Bit | 1 is 1 for the bit (so it is used to set bit to 1). Since I don't know what your program should do, I can't advise you. Go on, read the tutorial, then you'll understand. — ZenJ, Jan 26 '17 at 02:34
Thanks @ZenJ you helped my through the understanding process. Instead of giving me the answer. I read the tutorial and understand it now thanks. — John, Jan 26 '17 at 02:44

ad absurdum · Accepted Answer · 2017-01-26T02:59:56.333

2

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.

edited Jan 26 '17 at 02:59

answered Jan 26 '17 at 02:33

ad absurdum

19,498
5
37
60

Thanks it works but could you explain the *flag_holder |= (1U << flag_position); so I know how it works. Also is there a way to do this with out using unsigned ints. Thanks again – John Jan 26 '17 at 02:39
nevermind it works if you take out the unsigned part. Could you just explain the 1U part. – John Jan 26 '17 at 02:40
and i assume 1U is unsigned 1? – John Jan 26 '17 at 02:43
1

The `U` makes the constant `1` `unsigned`; this is a good habit to get into when you are doing bitwise operations. You must use `unsigned` for `flag_holder`. This is because you are setting the high-order bit (bit number 31) to `1`. This is the sign bit, and bit-shifting a negative value to the right is implementation-defined, to the left is undefined behavior. If it appears to "work", consider yourself lucky (for now), and fix it before something unpleasant happens. – ad absurdum Jan 26 '17 at 02:44
1

@christian "nevermind it works if you take out the unsigned part." --> Not a good idea, stay with `unsigned`. – chux - Reinstate Monica Jan 26 '17 at 02:46
The instructor provided the driver which has the function declarations so i cannot change it to unsigned. However i have jotted it down for future reference to use the unsigned. – John Jan 26 '17 at 02:49
If you must use `signed int`s, I would suggest at least changing to `set_flag(&flag_holder, 30);`, and initializing: `int flag_holder = 0U;` to be sure that the sign bit is not set. – ad absurdum Jan 26 '17 at 02:52

bitwise operators and shifts issues in c

1 Answers1