rohithpr
rohithpr

Reputation: 6330

What is the correct way to define this C macro?

#include <stdio.h>
#define ABS(a)  (a) < 0 ? -(a) : (a)
int main(void)
{
  printf("%d\n", ABS(-3) + 1);
  return 0;
}

This code snippet, from Herbert Schildt's book, looks like it will produce the output 4 but it actually prints 3. Why?

How do I fix it?

Upvotes: 2

Views: 1613

Answers (4)

Subhiksh
Subhiksh

Reputation: 301

Unlike functions macros are expanded in the place where they are encountered. Therefore in this code

printf("%d\n", ABS(-3) + 1);

when ABS(-3) is encountered it is expanded, i.e

 printf("%d\n", (-3) < 0 ? -(-3) : (-3) + 1);

So the expressoin is true and -(-3) is evaluated (in your case). If the expression was evaluated to false (say) then the result would have been (-3)+1 which is -2.

To fix this, instead of

#define ABS(a)  (a) < 0 ? -(a) : (a)

write

#define ABS(a)  ((a) < 0 ? -(a) : (a))

Upvotes: 2

Cacho Santa
Cacho Santa

Reputation: 6914

Expand your macro:

#define ABS(a)  (a) < 0 ? -(a) : (a)

printf("%d\n", ABS(-3) + 1);

printf("%d\n", (-3) < 0 ? -(-3) : (-3) + 1); // you can get this with **gcc -E source.c

printf("%d\n", (-3) < 0 ? 3 : -2); //read about precedence to understand this step.

printf("%d\n", 3);

This is the step by step explanation of why is printing 3. You need to fix it using the appropriate parenthesis.

Upvotes: 4

Sohil Omer
Sohil Omer

Reputation: 1181

Fix Parenthesis #define ABS(a) (((a) < 0 )? -(a) : (a))

Upvotes: 1

M.M
M.M

Reputation: 141618

The correct way is to use an inline function, with a _Generic case for each type you want to support. Otherwise you evaluate a twice.

In lieu of that you can fix it by parenthesizing the expression in the macro. That is always a good idea to prevent exactly this sort of problem.

#define ABS(a) ((a) < 0 ? -(a) : (a))

The problem comes about because X ? Y : Z + 1 means X ? Y : (Z + 1) .

Upvotes: 5

Related Questions