0

So, let's say I have a number N that's guaranteed to be a power of 2 and is always greater than 0. Now, I wrote two C methods to find what power of 2 N is, based on bitwise operators -

Method A - 

int whichPowerOf2(long long num) {
    int ret = -1;
    while (num) {
        num >>= 1;
        ret += 1;
    }

    return ret;
}

Method B - 

int whichPowerOf2(long long num) {
    int idx = 0;
    while (!(num & (1<<idx))) idx += 1;
    return idx;
}

Intuitively, the two methods seem one and the same and also return the same values for different (smaller) values of N. However, Method B doesn't work for me when I try to submit my solution to a coding problem.

Can anyone tell me what's going on here? Why is Method A right and Method B wrong?

gravetii
  • 9,273
  • 9
  • 56
  • 75
  • 2
    Probable hint: I believe the type of this expression: `1< – Mat Jan 14 '20 at 21:50
  • 1
    Wouldn't the second one loop forever for `num=0` ? – Eugene Sh. Jan 14 '20 at 21:51
  • @EugeneSh. You're right, forgot to mention num's always >= 2. Edited. – gravetii Jan 14 '20 at 21:51
  • 2
    Change `(1< – Lxer Lx Jan 14 '20 at 21:51
  • 2
    I would also change `num` to be `unsigned` – Eugene Sh. Jan 14 '20 at 21:53
  • @EugeneSh. Bounds of the problem allow `num` in the range of long long. But, thanks anyway, found the issue after your comments. – gravetii Jan 14 '20 at 21:57
  • 2
    @gravetii You work with positive numbers, so there is no reason to have signed types. Moreover, you really don't want to deal with undefined/implementation defined behaviors when you shift something into the sign bit. – Eugene Sh. Jan 14 '20 at 21:58
  • Yeah, I understand what you're saying. It makes perfect sense. I was actually just solving a problem on an online judge and I happened to use long long for it. That's all. Point taken though - when only doing bitwise operations like this, better use unsigned. – gravetii Jan 14 '20 at 22:00

2 Answers2

4

The problem is with this subexpression:

1<<idx

The constant 1 has type int. If idx becomes larger than the bit width of an int, you invoked undefined behavior. This is specified in section 6.5.7p3 of the C standard regarding bitwise shift operators:

The integer promotions are performed on each of the operands. The type of the result is that of the promoted left operand. If the value of the right operand is negative or is greater than or equal to the width of the promoted left operand, the behavior is undefined.

Change the constant to 1LL to give it type long long, matching the type of num.

while (!(num & (1LL<<idx))) idx += 1;
dbush
  • 205,898
  • 23
  • 218
  • 273
2

In your Method B, the following line can cause undefined behaviour:

while (!(num & (1<<idx))) idx += 1;

Why? Well, the expression 1<<idx is evaluated as an int because the constant 1 is an int. Further, as num is a long long (which we'll assume has more bits than an int), then you could end up left-shifting by more than the number of bits in an int.

To fix the issue, use the LL suffix on the constant:

while (!(num & (1LL<<idx))) idx += 1;
Adrian Mole
  • 49,934
  • 160
  • 51
  • 83