Is it bad practice to have a macro definition return a value for a function?

Using a macro defined to conditionally return a va开发者_如何转开发lue has a disadvantage where it is not apparent from only looking at the client code might exit at the point of the macro.

The use case I am considering is writing a value and error checking, like so:

#define WRITE_CHK(file, param)\
if (!write_that_returns_zero_on_fail(file, param)) {\
   handle_error();\
   return false;\
}

client code:

bool myfunc()
{
   ...
   WRITE_CHK(file, param) // function might return here
   ...
   return true;
}

I am curious if the benefits of the macro (which would be used in many places in my code) would outweigh the disadvantage mentioned above. Are there preferred alternatives besides simply expanding (not using the macro)?

The standard answer is "don't use macros"; but that's often slightly simplistic. There are sometimes cases where they can greatly cut down on the boilerplate verbosity that you'd otherwise have.

So, why not encode the fact into the macro name? e.g. WRITE_OR_RETURN_ON_FAILURE. It may be slightly verbose, but it's much less likely to trip up readers of your code.

Hiding control flow inside a macro is not used very commonly, so it can be confusing to developers who will have to understand or debug your code.

I would recommend against using macros totally, but if you have to use them ensure that the control flow part is explicit.

// assume handle_error returns true
#define WRITE_FAILED(file, param)\
(!write_that_returns_zero_on_fail(file, param) && handle_error())

// macro usage
if(WRITE_FAILED(file, param))
{
    return;
}

Another question to ask yourself would be -- why do you want to use macros here anyway? If it was to hide the control flow then that's not a good idea. Is there another reason?

Make a 2-step macro:

#define WRITE_CHK_(file, param, HANDLER)\
if (!write_that_returns_zero_on_fail(file, param)) {\
    handle_error();\
    HANDLER\
}

#define RFALSE return false;

#define WRITE_CHK(file, param) WRITE_CHK_(file, param, RFALSE)

Then you can make other wrappers if you need a similar check elsewhere (or directly use WRITE_CHK(file, param, return false))

Yes, it's bad. Use a goto.

 #define fail_unless(x) do {if(!(x)) goto fail;} while (0)

 int foo()
 {
     fail_unless( my_write(...) );
     fail_unless( bar() );
     return 0;
 fail:
     cleanup();
     return -1;
 }

Edit: if you're downvoting because you don't understand what that while loop is doing there, you should just have asked. It is a standard technique for making sure a C macro acts as a single statement in any context. If your macro expands to an if statement (like the macros in the other, upvoted answers), then you get unexpected side effects, eg:

 #define DONT_DO_THIS(x) if (!x) { foo; }

 if (a) DONT_DO_THIS(x)
 else something_else();

You'd expect something_else to be executed when !a, but what the macro actually expands to is:

 if (a)
     if (!x) { foo;}
     else something_else();

And something_else is executed when a && x, not when !a as the programmer intended.