Not able to understand if in #define

Question

Can anyone help me to understand the meaning of this line?

I know it's kind of macro structure, but what does , suggest in the code??

#define ReturnErr(fCall) if (iErr = (fCall), (iErr != NO_ERRORS)) {return iErr;}

Answer 1

A qualified guess is that the macro is meant to be used like this:

err_t func (void)
{
  err_t iErr;

  ReturnErr(some_function());
  ...

  return NO_ERRORS;
}

In that case the macro expands to:

err_t func (void)
{
  err_t iErr;

  if(iErr = some_function(), iErr != NO_ERRORS) { return iErr; }
  ...

  return NO_ERRORS;
}

which in turn is just a needlessly obfuscated way of writing

err_t func (void)
{
  err_t iErr;

  iErr = some_function();
  if(iErr != NO_ERRORS)
  { 
    return iErr; 
  }
  ...

  return NO_ERRORS;
}

In other words, the macro is likely an attempt from repeating the same error handling code over and over.

Answer 2

Macros are a text substitution. It means that if someone writes, for example,

ReturnErr(x)

then their code will be processed as:

if ( iErr = (x), (iErr != NO_ERRORS) )
{
    return iErr;
}

This is bad style, but they probably want to have their function return when a failure occurs and save some typing over copying out that code at each point they need to check an error code.

Answer 3

The macro takes a single argument named fCAll. The macro expands to the following code:

if (iErr = (fCall), (iErr != NO_ERRORS)) {
   return iErr;
}

I guess you are confused by the usage of the , operator in the if statement.

In the C and C++ programming languages, the comma operator (represented by the token ,) is a binary operator that evaluates its first operand and discards the result, and then evaluates the second operand and returns this value (and type).

This is quote from the wikipedia article btw.

Thus the statement in the body will be executed if and only if iErr != NO_ERRORS i.e. there are errors.

Answer 4

The macro wants to use the value iErr twice, once in the if and once in the return, but it wants to execut fCall only once. It uses the comma which evaluates both its operands but is equal only to the right-most.

Thus if we expand by hand and do a little refactoring, we get:

iErr = (... macro argument here ...);
if((iErr != NO_ERRORS)) {
    return iErr;
}

Answer 5

Lundin and others have explained what the author of the macro intended. However, the way it is written it can have unintended consequences too, because it doesn't follow the canonical way of writing multi-statement macros, which is: wrap it in a do..while(0) loop.

Why? Consider:

if( someCondition)  ReturnErr(fCall); else doSomethingElse();

This fails syntactically because the programmer's semicolon behind the expanded macro's curly brace is an empty statement, making the else dangling.

The programmer may choose to remove the superfluous semicolon in order to mollify the compiler, writing

if( someCondition)  ReturnErr(fCall) else doSomethingElse();

This is probably not what s/he intended, because the else part is now silently attached to the macro's if. That's pretty bad because it's totally invisible.

The canonical way is:

#define ReturnErr(fCall) do { if (iErr = (fCall), (iErr != NO_ERRORS)) \
                              return iErr; \
                            } while(0)

This allows, even requires the syntactically natural semicolon at the end and counts as a single statement whenever one is needed.

@Jens: :-)