Getting unexpected results when using macros with arguments

Question

#include <stdio.h>
#define big(a, b) a > b ? a : b
#define swap(a, b) temp = a; a = b; b = temp;

int main() {
    int a = 3, b = 5, temp = 0;
    if ((3 + big(a, b)) > b)
        swap(a, b);
    printf("%d %d", a, b);
}

Above code was given in a multiple choice question. I expected the answer to be 5 3. On running the code, output comes out to be 5 0. I've tried to make sense of this but my efforts have gone in vain. What's the working of this code?

Answer 1

Just expand the macros in the code and you will get

int main() {
    int a = 3, b = 5, temp = 0;
    if ((3 + a > b ? a : b ) > b)
        temp = a;
    a = b;
    b = temp;    
    printf("%d %d", a, b);
}

As the first expression 3 + a in the conditional operator is equal to 6 then the result of the conditional operator is the value of a that is less than b.

So the condition of the if statement evaluates to logical false and the statement

temp = a;

is skipped.

As a result a will be equal to 5 and b will be equal to 0.

You need to use parentheses. For example

#define big(a, b) ( ( a ) > ( b ) ? ( a ) : ( b ) )
#define swap(a, b) do { temp = ( a ); ( a ) = ( b ); ( b ) = temp; } while ( 0 )

Answer 2

gcc -E (preprocessing only) renders the following output:

int main() {
    int a = 3, b = 5, temp = 0;
    if ((3 + a > b ? a : b) > b)
        temp = a; a = b; b = temp;;
    printf("%d %d", a, b);
}

From this we can see that the body of the if statement is comprised only of the first expression statement (temp = a;), terminating with the first semi-colon.

a = b; b = temp;; fall outside the if body, to be unconditionally executed. Note the extra semi-colon as well.

---

The typical pattern to have multiple expression statements contained in a macro is to make use of do { ... } while (0), which groups the statements to a new block scope, and allows the use of a trailing semi-colon to be consistent.

The other thing to do is to aggressively parenthesize macro arguments, and often entire macros, to ensure operator precedence remains as consistent as possible.

Note that swap relying on an implicit temp variable to be defined is a code smell. One possibility for a better approach would be to pass the type instead.

#include <stdio.h>
#define big(a, b) (((a) > (b)) ? (a) : (b))
#define swap(type, a, b) do { type temp = (a); (a) = (b); (b) = temp; } while (0)

int main(void)
{
    int a = 3, b = 5;

    if ((3 + big(a, b)) > b)
        swap(int, a, b);

    printf("%d %d\n", a, b);
}

Preprocessor output:

int main(void)
{
    int a = 3, b = 5;

    if ((3 + (((a) > (b)) ? (a) : (b))) > b)
        do { int temp = (a); (a) = (b); (b) = temp; } while (0);

    printf("%d %d\n", a, b);
}

Program output:

5 3

An issue not addressed by this example is that both these macros evaluate their arguments more than once, something that becomes tricky when side effects are involved (e.g., macro(func(x++), y--)).

See also: GCC CPP - 3.10.4 Duplication of Side Effects