25

For example, never define a macro like this:

#define DANGER 60 + 2

This can potentially be dangerous when we do an operation like this:

int wrong_value = DANGER * 2; // Expecting 124

Instead, define like this because you don't know how the user of the macro may use it:

#define HARMLESS (60 + 2)

The example is trivial, but that pretty much explains my question. Are there any set of guidelines or best practices that you would recommend when writing a macro?

Thanks for your time!

1
  • 2
    That's not a question. It's a statement.
    – BIBD
    Nov 26, 2008 at 15:42

11 Answers 11

31

Not only should you put parens around the arguments, you should put parens around the expression returned.

#define MIN(a,b)  a < b ? a : b     // WRONG  

int i = MIN(1,2); // works
int i = MIN(1,1+1); // breaks

#define MIN(a,b)  (a) < (b) ? (a) : (b)   // STILL WRONG

int i = MIN(1,2); // works
int i = MIN(1,1+1); // now works
int i = MIN(1,2) + 1; // breaks

#define MIN(a,b)  ((a) < (b) ? (a) : (b))   // GOOD

int i = MIN(1,2); // works
int i = MIN(1,1+1); // now works
int i = MIN(1,2) + 1; // works

However, MIN(3,i++) is still broken...

The best rule is only to use #defines only when NO OTHER APPROACH WILL WORK! I know you're asking about C instead of C++, but still bear his in mind.

3
  • 1
    Maybe you should add a note about using enum instead of defining numeric constants with #define. Nov 26, 2008 at 17:53
  • Ack! MIN(3, i++)! I never considered that case... That's gotta suck to debug that!
    – Adam Davis
    Nov 26, 2008 at 18:42
  • Of course, the fix is trivial - make it a function, or take the increment out of that line of code. I don't agree that macros shouldn't be used except when no other approach will work - Like any other tool macros can be dangerous if improperly used, but done well they can decrease development time
    – Adam Davis
    Nov 26, 2008 at 18:45
31

When doing a macro that is to run its argument and behave like an expression, this is idiomatic:

 #define DOIT(x) do { x } while(0)

This form has the following advantages:

  1. It needs a terminating semicolon
  2. It works with nesting and braces, e.g. with if/else
3
  • 1
    And what if x is only a single statement? For instance foobar = 42, do we still need to enclose it into do{}while(0), or just putting it inside parentheses is enough? Sep 3, 2011 at 4:10
  • 1
    You would still need it if you want 1) the macro to demand a terminating semicolon, and 2) to use it as a stand-alone instruction in a one-line if-statement. Nov 5, 2011 at 20:46
  • 1
    Check this link. It explains it very well : kernelnewbies.org/FAQ/DoWhile0 Nov 5, 2011 at 20:47
10

Use parenthesis around the entire macro and around each argument referred to in the expansion list:

#define MAX(x, y) ((x) > (y) ? (x) : (y))

Avoid writing macros that evaluate their arguments multiple times. Such macros will not behave as expected when arguments have side effects:

MAX(a++, b);

Will evaluate a++ twice if a is greater than b.


Use UPPERCASE names for macros to make it clear it is a macro and not a function so that the differences can be considered accordingly (another general good practice is not passing arguments that have side effects to functions either).


Don't use macros to rename types like this:

#define pint int *

because it won't behave as expected when someone types

pint a, b;

Use typedefs instead.

7

use static const values instead of macros for constant values, integral or other. The compiler can often optimize them away, and they remain a 1-st class citizen in the language's type system.

static const int DANGER = 60 + 2;
3
  • Will that really work in standard C? In a header file?
    – Roddy
    Nov 26, 2008 at 15:51
  • @Roddy: no, it won't work in C in a header - use an enum instead. Nov 26, 2008 at 17:48
  • 1
    @John: it depends on the context as to whether this is appropriate. Mar 19, 2009 at 23:37
5

In the expansion, put parenthesis around the arguments, so that if they pass in a expression you will get the intended behavior.

#define LESS_THAN(X,Y) (((X) < (Y) ? (X) : (Y))
1
  • Ironically, this is a bad macro because it evaluates its arguments twice.
    – user4945014
    Nov 19, 2021 at 17:44
5

Response to the MAX/MIN macros, taken from GCC hacks in the Linux kernel:

#define min(x, y) ({                       \
        typeof(x) _min1 = (x);             \
        typeof(y) _min2 = (y);             \
        (void) (&_min1 == &_min2);         \
        _min1 < _min2 ? _min1 : _min2; })
6
  • 2
    typeof is a gcc extension and should not be used in portable code. Nov 26, 2008 at 17:46
  • Agreed with Robert. Does anyone want to explain what the void cast and expression is there for? Nov 26, 2008 at 17:50
  • Also, the use of ({ ...; foo;}) as evaluating to foo. That's a new one on me. Nov 26, 2008 at 18:35
  • I wonder if the void cast and expression is trying to tell the compiler that _min1 and _min2 have to be compatible scalars and not be in registers? Nov 26, 2008 at 18:51
  • 3
    "the use of ({ ...; foo;})" is another GCC extension. That's why it's a GCC hack, not a C hack. Not sure what else, but the void thing stops you using one signed and one unsigned argument. The types of x and y would be compatible, but their pointer types aren't. Nov 26, 2008 at 20:59
2

Use fairly unique names for your macros, since they have global scope and can clash with anything, so:

#define MAX 10

could easily clash with other code, so:

#define MYPROJECT_MAX 10

or something even more unique, would be better.

I've seen cases where a clash of this kind didn't produce a compile error, but generated slightly wrong code, so it can be quite insidious.

2

For multiple line macros, use a do { } while (0):

#define foo(x) do {  \
    (x)++;           \
    printf("%d", x); \
} while(0)

Had you done

#define foo(x) {     \
    (x)++;           \
    printf("%d", x); \
}

instead,

if (xyz)
    foo(y);
else
    foo(z);

would've failed.

Also, be careful when introducing temporary variables in macros:

#define foo(t) do {    \
    int x = (t);       \
    printf("%d\n", x); \
} while(0)

int x = 42;
foo(x);

will print 0 and not 42.

If you have a complicated expression which needs to return a value, you can use the comma operator:

#define allocate(foo, len) (foo->tmp = foo->head, foo->head += len, foo->tmp)
2

Undefine your macros.

Your #defines should matched with an #undef. This prevents the preprocessor from getting clogged up and affecting unintended pieces of code.

1

If you're careful and expert, you may be able to accomplish DRY (Don't-Repeat-Yourself) code, by using macros as simple code generators. You do have to explain to other programmers what you're doing, but it can save a lot of code. For example, the list-macro technique:

// define a list of variables, error messages, opcodes
// or anything that you have to write multiple things about
#define VARLIST \
    DEFVAR(int, A, 1) \
    DEFVAR(double, B, 2) \
    DEFVAR(int, C, 3) \

// declare the variables
#define DEFVAR(typ, name, val) typ name = (val);
    VARLIST
#undef  DEFVAR

// write a routine to set a variable by name
void SetVar(string varname, double value){
    if (0);
    #define DEFVAR(typ, name, val) else if (varname == #name) name = value;
        VARLIST
    #undef  DEFVAR
    else printf("unrecognized variable %s\n", varname);
}

// write a routine to get a variable's value, given its name
// .. you do it ..

Now, if you want to add a new variable, delete one, or rename one, it's a 1-line edit.

5
  • 10
    I hope I never have to maintain any code that you write using this idiom. Nov 26, 2008 at 17:51
  • Like I said, it takes explanation, which is true of anything but lowest-common-denominator code. If you want to do code generation, it is a way. Nov 26, 2008 at 17:58
  • there is a places where it's acceptable. But one should be very careful using this technique.
    – Ilya
    Nov 26, 2008 at 18:12
  • 1
    Right. Not every useful idea is popular or well-known. Nov 26, 2008 at 18:13
  • 1
    For reference, this is known as the X macro pattern.
    – 0x5453
    2 days ago
0

See how I hate this:

void bar(void) {
    if(some_cond) {
        #define BAZ ...
        /* some code */
        #undef BAZ
    }
}

Always put them like this:

void bar(void) {
    if(some_cond) {
#define BAZ ...
        /* some code */
#undef BAZ
    }
}
3
  • While I agree with you, this has nothing to do with macros definitions. Nov 26, 2008 at 17:48
  • I wish my gripes were as big as that :-) Nov 28, 2008 at 13:48
  • 2
    well i don't like it :) same with goto labels Nov 28, 2008 at 17:02

Not the answer you're looking for? Browse other questions tagged or ask your own question.