11

I got the following question in an interview: "Write a C function that round up a number to next power of 2."

I wrote the following answer:

#include <stdio.h>

int next_pwr_of_2(int num)
{
    int tmp;

    do
    {
        num++;
        tmp=num-1;
    }
    while (tmp & num != 0);

    return num;
}

void main()
{
    int num=9;
    int next_pwr;
    next_pwr=next_pwr_of_2(num);
    printf(" %d \n",next_pwr);
}

The question is: why does the program go out of its do-while loop when getting to the values 11 and 10?

jeb
  • 78,592
  • 17
  • 171
  • 225
KBE
  • 379
  • 3
  • 4
  • 15

7 Answers7

28

Precedence my friend, precedence.

while ((tmp & num) != 0);

Will fix it. ( note the parenthesis around the expression tmp & num)

!= has higher precedence than &, so num != 0 is evaluated before tmp & num.

If you skip the parenthesis, the expression that is evaluated is : tmp & (num != 0)

  1. First time round, tmp = 9 (1001) and num != 0 is 1 (0001) so & evaluates to 1 (true), and the loop continues.

  2. Now at the end of second iteration, we have, tmp = 10 (1010). num != 0 is again 0001, so 1010 & 0001 evaluates to 0, hence the loop breaks.

Here is the table for reference.

The precedence order is quite unusual, as noted here. Happens all the time :).

Of course you don't have to remember any precedence order, which is just to help the compiler in deciding what is done first if the programmer does not make it clear. You can just correctly parenthesize the expression and avoid such situations.

axiom
  • 8,765
  • 3
  • 36
  • 38
  • 1
    Axiom, thanks a lot for the detailed answer, I will pay more attention next time in these cases. – KBE Feb 11 '13 at 13:51
20

The loop exits because you did not put parentheses around your condition. This should teach you not to put the unnecessary != 0 in your C/C++ conditions.

You can simplify your code quite a bit, though.

First, observe that temp equals the prior value of num, so you can change your loop to

int tmp;
do {
    tmp = mum++;
} while (tmp & num); // Don't put unnecessary "!= 0"

Second, the interviewer was probably looking to see if you are familiar with this little trick:

v--;
v |= v >> 1;
v |= v >> 2;
v |= v >> 4;
v |= v >> 8;
v |= v >> 16;
v++;

Unlike your code that may take up to 1,000,000,000 operations to complete, the above always completes after twelve operations (a decrement, an increment, five shifts, and five ORs).

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • Can you elaborate a little more on how the fedback from your while condition? – KBE Feb 11 '13 at 13:56
  • Tricks are cute, but most platforms provide the ability to determine the number of leading zeros in a word. Many C compilers provide access to those features of major chipsets. In an interview, I would expect an embedded programmer to mention that and not use pure C to solve the problem. – William Pursell Feb 11 '13 at 14:12
  • I have another interview and your second answer seems impressive, can you give some explanaition regarding it, like what is the sequence: >> first or | first in each line? – KBE Feb 11 '13 at 14:17
  • In C++ particularly, then != is not unnecessary. `tmp & num` is an int, `(tmp & num) != 0` is a bool. For that reason and many others, it is considered _good_ programming practice to keep all expressions in if statements in the correct type, "esstentially boolean". You are teaching the opposite of that. The _correct_ answer would be: use parenthesis whenever operator precedence isn't obvious. – Lundin Feb 11 '13 at 14:20
  • 1
    @KBE Unlike Java/C# and many other languages, C/C++ do not insist on conditions being boolean. Anything other than zero is interpreted as `true`, while zero is interpreted as `false`. That's why `!= 0` in `if`s and `while`s are rarely necessary and often omitted. You often find vehement disagreement on the subject, but your error is a perfect case at point for omitting the unnecessary `!= 0`: less bugs is always better, right? – Sergey Kalinichenko Feb 11 '13 at 14:21
  • 1
    @KBE On the subject of the bit twiddling trick, the `>>` is executed first, and then the result is OR-ed into the original value. By doing this for all powers of 2 up to 4 (for 64-bit you'd need to go up to 7) you ensure that all bits of the original number are "filled in" with ones before the increment. – Sergey Kalinichenko Feb 11 '13 at 14:26
  • 1
    @dasblinkenlight C/C++ may not insist on it, but human readers do. Not putting in the `!= 0` is just bad programming practice, which wouldn't have been condoned in most of the places I've worked. (And not putting bitwise operators in parentheses wouldn't be condoned either, because they have the "wrong" precedence.) – James Kanze Feb 11 '13 at 14:32
  • 3
    With regards to the bit twiddling trick: it depends on knowing the number of bits in an `int`. You'd only get slightly more operations (and possibly less) with the completely portable `v <<= 1; while ( (v & (v - 1)) != 0 ) { v &= v - 1; }` (except that this requires picking up the special case where the number is a power of two to being with). Which one should be prefered will depend on context (but of course, either is always better than the proposed solution). – James Kanze Feb 11 '13 at 14:36
  • @KBE I'm quite surprised you're getting a second interview with the code you gave in the question. You know that even after you fix the parentheses there's still a bug in it, right? – Mark Ransom Feb 11 '13 at 17:33
  • Actually it completes after twelve operations, not seven. – Benny Feb 14 '13 at 17:57
  • @becko No, it wouldn't. – Sergey Kalinichenko Aug 16 '15 at 21:55
2

Such questions always deserve counter questions to clarify requirements, if only to demonstrate your thinking and analytical skills and even creativity - that is what the interview should be about.

For example in the absence of any specification that the "number" in question is necessarily an integer, you might propose the following:

int nextPow2( double x )
{
    return (int)pow( 2, ceil(log10(x) / log10(2))) ;
}

But if you did you might also express concern about the applicability of such a solution to an embedded system with possibly no floating-point unit.

Clifford
  • 88,407
  • 13
  • 85
  • 165
1

I would answer by saying no one should write that in pure C. Especially in an embedded environment. If the chipset does not provide a feature to count the number of leading zeros in a word, then it's probably pretty old, and certainly not something you want to be using. If it does, you would want to use that feature.

As an example of a non-standard way to round an unsigned integer up to a power of two (you really need to clarify the type of the argument, as "number" is ambiguous) using gcc, you could do:

unsigned
round_up( unsigned x )
{
    if( x < 2 ) {
        return 1U;
    } else {
        return 1U << ( CHAR_BIT * sizeof x - __builtin_clz( x - 1 ));
    }
}
William Pursell
  • 204,365
  • 48
  • 270
  • 300
  • Some chipsets have such instructions; some don't. I don't know that age has much to do with it. More likely the question would simply one of whether such an instruction would represent the most beneficial use for the amount of silicon required. Such an instruction could accelerate software-based floating point, but I'm not sure how often it would otherwise be useful. – supercat Mar 04 '13 at 17:49
1

In contrast to what others have said, the bit-twiddling trick actually can be used on any number of bits portably. Just change it a bit:

unsigned int shift = 1;

for (v--; shift < 8 * sizeof v; shift <<= 1)
{
    v |= v >> shift;
}

return ++v;

I believe any compiler will optimize the loop away, so it should be the same performence-wise (plus I think it looks better).

Avidan Borisov
  • 3,235
  • 4
  • 24
  • 27
1

Yet another variant.

int rndup (int num)
{
    int tmp=1;
    while (tmp<num)
    {
        tmp*=2;

    }
    return tmp;

}
Leigh
  • 11
  • 1
0

Another variant using while loop and bit-wise operator

int next_pwr_of_2(unsigned &num)
{
    unsigned int number = 1;
    while (number < num)
    {
    number<<=1;
    }
    return number;
};
user3403531
  • 11
  • 1
  • 3