1

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; } 
9
  • sorry *flag_holder |= *flag_holder << flag_position; Commented Jan 26, 2017 at 1:55
  • That depends on what you want. Do you understand the meaning of operators |=, &, << ? Because now "1" is gone from that line. Commented Jan 26, 2017 at 1:57
  • |=Checks if the values of the two are equal or not. & copies a bit to the result and << is left shift Commented Jan 26, 2017 at 2:00
  • @ZenJ if you could explain it a bit I would greatly appreciate that Commented Jan 26, 2017 at 2: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; Commented Jan 26, 2017 at 2:07

1 Answer 1

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.

Sign up to request clarification or add additional context in comments.

7 Comments

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
nevermind it works if you take out the unsigned part. Could you just explain the 1U part.
and i assume 1U is unsigned 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.
@christian "nevermind it works if you take out the unsigned part." --> Not a good idea, stay with unsigned.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.