0

We know that multiline macros have to be inclosed in a do while(0) loop for them to be safely included anywhere in the code.

For example this is 100% safe:

#define swap( a , b )       \
        do                  \
        {                   \
            int t = a ;     \
            a = b ;         \
            b = t ;         \
        }                   \
        while(0)

I would like to know if it is also safe to have a macro with a part of the code outside of the loop, and if it isn't, in what circumstances could problems occur.

#define swap( a , b )       \
        int t ;             \   //should be in the same scope where the macro was called and before the loop
        do                  \
        {                   \
            t = a ;         \
            a = b ;         \
            b = t ;         \
        }                   \
        while(0)

Is there a safe way to achieve this, what would have to be changed or what rules should I follow if it isn't safe.

this
  • 5,229
  • 1
  • 22
  • 51

5 Answers5

3

It is not safe in many ways. First, if any variable called t already existed, it wouldn't compile.

Apart from that, you have to be careful with things like:

if (...)
    swap(a,b);

As it would be expanded to:

if (...)
    int t;
    do { ... } while(0);

Note that the if body would be only a declaration because there are no braces for the if block.

Why don't you declare t inside the do block?

Filipe Gonçalves
  • 20,783
  • 6
  • 53
  • 70
2

Two cases I can think of. First, if there's already a variable t in current scope you will get either a multiple definitions or conflicting definitions in same scope error. Second, consider:

if (some_condition) swap(a,b);

will expand to:

if (some_condition) int t;
do
{
    t = a;
    a = b;
    b = t;
} while (0);
Dan
  • 10,531
  • 2
  • 36
  • 55
  • Is the if statement the only place where this might occur? If i use if{}else{} everywhere then i don't get that? – this Oct 01 '13 at 12:33
  • 1
    No, you'd have the same issue if the `if` were replaced with a `for` or a `while`. You might have problems with switch statements. You would have problems if you tried to use it with any of the short-circuiting logical operators `||` or `&&`. – Dan Oct 01 '13 at 12:37
1

It would not be compile safe if a variable named t was anywhere else in that scope.

Daniel A. White
  • 187,200
  • 47
  • 362
  • 445
1

There's a second problem in your macro besides the scope story. You forgot to put parenthesis to your macro variables, this can have some surprizing effects.

#define swap( a , b )       \
        do                  \
        {                   \
            int t = a ;     \
            a = b ;         \
            b = t ;         \
        }                   \
        while(0)

In C++ if you invoke the macro for instance with a ternary: swap(x?x:y, z) it would have a surprizing effect.

do
  {
  int t = x?x:y;
  x?x:y = z ;
  z = t;
  }
while(0)

as the ternary has lower priority than affectation, the second line will be interpreted as: x?x:(y = z);

There could be other surprizes, therefore as a rule:

Always put parenthesis around macro parameter, ALWAYS!

EDIT: This is the correct #define

#define swap( a , b )       \
        do                  \
        {                   \
            int t = (a) ;     \
            (a) = (b) ;         \
            (b) = t ;         \
        }                   \
        while(0)
Patrick Schlüter
  • 11,394
  • 1
  • 43
  • 48
  • My compiler doesn't allow me to put a ternary operator in the `swap` macro either way (Lvalue required). I must say i have read all about putting parenthesis around macro variables, but never around macro parameters. Can you provide some info(link)? Thank you. – this Oct 01 '13 at 16:11
  • There might be a misunderstanding, it's in the `#define` that every occurence of a macro parameter should be put in parenthesis, not at the invokation point. – Patrick Schlüter Oct 02 '13 at 11:42
  • I didn't find an example to show the problem in C that's why I said in `C++` as the ternary has a different priority as in C and where they are allowed as `lvalue`. That doesn't mean that there couldn't be other problems. – Patrick Schlüter Oct 02 '13 at 11:44
  • http://stackoverflow.com/questions/7186504/c-macros-and-use-of-arguments-in-parentheses – Patrick Schlüter Oct 02 '13 at 20:51
  • http://stackoverflow.com/questions/10820340/the-need-for-parentheses-in-macros-in-c – Patrick Schlüter Oct 02 '13 at 20:52
  • and an external link with several rules and problems known for macros http://linux.web.cern.ch/linux/scientific4/docs/rhel-cpp-en-4/macro-pitfalls.html – Patrick Schlüter Oct 02 '13 at 20:53
0

As your actual goal is to define t for access from outside the makro, you should declare it outside your makro. If your makro is something that can be used multiple times in one code block (like the swap you use as an example - you want to be able to swap multiple values), then you will get issues with multiple declarations of t.

Instead, you could do it like MFC with USES_CONVERSION and define another makro USES_SWAP that prepares all "global" data for your makro, and may be called only once in each code block, before any and all appearances of SWAP (so ideally at the beginning of the code block). Now you can swap things around as often as you like and still handle declaration of required variables in one makro.

Of course, the issue remains that the t declared in USES_SWAP will conflict with other ts, but that might just be a necessity if you can't find another way to do it. Which you should try to do ;) To reduce the possibility of this becoming an issue, you should consider a more specific name, such as swap_t or even SwapInternalVariable_t. The danger of someone declaring a variable with that name in normal code is pretty low.

SvenS
  • 795
  • 7
  • 15