• Using unsafe macros
• Use parentheses around macro replacement lists
• Multistatement macros should be wrapped in a do-while loop

Using unsafe macros

What you should not do

#define ABS(x) (((x) < 0) ? -(x) : (x))
#include<stdio.h>

void f(int n) {
    int m;
    m = ABS(++n);
    /* ... */
}

The above macro when called expands to, m = (((++n) < 0) ? -(++n) : (++n));. This leads to n to be incremented twice rather than once.

What you should do

One solution to avoid the above vulnerability is to use an inline function.

#include<stdio.h>

inline int ABS(int x) {
      return x < 0 ? -x : x;
}

void f(int n) {
    int m;
    m = ABS(++n);
    /* ... */
}

Concept Map

This maps to ii in the Concept Map.

The above example is taken from the CERT Secure coding website, https://www.securecoding.cert.org/confluence/display/c/PRE12-C.+Do+not+define+unsafe+macros .

Use parentheses around macro replacement lists

What you should not do

In the code below, the macro CUBE does not contain proper parenthesize for the replacement list.

#define CUBE(X) (X) * (X) * (X)
int i = 3;
int a = 81 / CUBE(i);

The statement, int a = 81 / CUBE(i); expands to int a = 81 / i * i * i;. The evaluation starts with 81/i getting calculated and then 3 is gets multiplied to the result of 81/i. This results to 243 rather than the expected value, 3.

What you should do

Code with correct parenthesis is given below.

Concept Map

This maps to iii in the Concept Map.

The above example is taken from the CERT Secure coding website, https://www.securecoding.cert.org/confluence/display/c/PRE02-C.+Macro+replacement+lists+should+be+parenthesized .

Multistatement macros should be wrapped do-while loop

What you should not do

A macro is defined as given below.

#define SWAP(x, y) { tmp = (x); (x) = (y); (y) = tmp; }

The macro above swaps two values using a third variable, tmp.

This macro is used in the code below.

if (x > y)
   SWAP(x, y); /* Branch 1 */
else
   do_something(); /* Branch 2 */

This macro expands in a manner as shown in the code below.

if (x > y) { /* Single-branch if-statement!!! */

  tmp = x; /* The one and only branch consists */
  x = y; /* of the block. */
  y = tmp;

}
; /* Empty statement */
else /* ERROR!!! "parse error before else" */
do_something();

What you should do

 Wrapping the macro is a do-while loop mitigates the problem.

/*
* Swaps two values and requires
* tmp variable to be defined.
*/
#define SWAP(x, y) \
do { \
   tmp = (x); \
   (x) = (y); \
   (y) = tmp; } \
while (0)

Concept Map

This maps to Bad Code in the Concept Map.

The above example is taken from the CERT Secure coding website, https://www.securecoding.cert.org/confluence/display/c/PRE10-C.+Wrap+multistatement+macros+in+a+do-while+loop .