Macro to log error and return false

Question

Most of the statements in the project I work with is of the form

if(!someObject->someFunction(arg1,arg2))
{
    cerr << "Something bad happened. Args are: " << arg1.print() << " " << arg2  << endl;
    return false;
}

This makes my code not readable as error logging triples the actual code. Hence I wrote the following macro,

#define ReturnFalse(error) \
  ({ \
   cerr << error << "@File: " << __FILE__ << "Line: " << __LINE__ << endl; \
   return false; \
   false; \
   }) \

Now I can use it as,

someObject->someFunction(arg1,arg2) || ReturnFalse("Something bad happened.Args are:" << arg1.print() << " " << arg2 );

This works but I am afraid if I am following bad practice.

Is the code illegal/undefined?
In performance wise, is it any worse than if?
My vim does not recognize the formatting and highlight as if there is some syntax error. Is there anyway to fix this?
Is there a better way to log a message and return with a neat syntax?
It is not straight forward that the macro could return from the function. So I named it ReturnFalse. Can anyone suggest a better name?

Edit:

Thanks for all replies.Is there a standard way to do it using comma operators? I am not able to get it right.

That construct is a GCC extension, not standard C++. `({...})`, I mean. — Cat Plus Plus, Jul 27 '11 at 17:17
how about saving some typing and clutter with this function: bool Failed( string err ){ cerr << err; return false; } and just use it like if( !someFunc() ) return Failed( "err" ); ? — stijn, Jul 27 '11 at 17:23
@Aditya, `operator ||` expects a `bool` on right hand side and `return` statement is `void` @stijn, This will not print me the line numbers. — balki, Jul 27 '11 at 17:40
When you say "using comma operators" do you actually mean the comma operator, or do you just want to pass additional arguments to the macro? The comma separating arguments to functions/macros is not the comma operator, its just a bit of syntax. — Dennis Zickefoose, Jul 27 '11 at 18:12
@Dennis, I mean the comma operator, like I tried, `func() || (cerr< — balki, Jul 27 '11 at 18:19
@balki: That's unfortunate. Perhaps you should start throwing exceptions instead. If the code base isn't too large you could even start doing that right away. — GManNickG, Jul 27 '11 at 18:24
@GMan, Nope it is a huge codebase. :(. Also Exceptions are rarely used and not encouraged in my team. — balki, Jul 27 '11 at 18:43
"Exceptions are rarely used and not encouraged in my team" : This policy will be a pain. all you have to do is wait and watch. — A. K., Jul 27 '11 at 18:48
@balki: Your team's attitude needs to change, then. Exceptions are superior to error codes. — GManNickG, Jul 27 '11 at 19:39

score 2 · Answer 1 · edited May 23 '17 at 12:26

The code is not C++ standard conforming and non portable.

({..})

is a gcc compiler extension called as Statement Expression.

If you compile your code with -pedantic flag & it will tell you that it is non Standard conforming.

Unless you are not worried about the portability of your source code across different compilers, you should refrain from using any code constructs which are not supported by the C++ Standard and are just compiler extensions.

littleadv · Accepted Answer · 2011-07-27T19:05:02.573

0

I think that it would better be a function:

bool fnAssert(bool cond, char * strCond, char *msg, char *file, int line)
{
    if (!cond)
    {
        cerr << "@File: " << file << ":" << line << " " << strCond << " failed."
              << msg << endl;
    }
    return cond;
}

#define Assert(cond, a) fnAssert(cond, #cond, a, __FILE__, __LINE__)

This way it is portable to all compilers, will show syntax highlighting, and the macro allows automatically inserting the file name and the line number. You'll also be able to print the failed condition, and put breakpoints when you debug it.

edit

Although it's very ugly and I advise you strongly not to do that, here's a way to automatically return on assert from within the code (using the above macros):

#define ReturnOnAssert(cond, a, b) if (! Assert(cond,a) ) return b;

Now when you write a statement:

ReturnOnAssert(isItBad, "Something bad happened", retVal);

or

DoWhatever(), ReturnOnAssert(isItBad, "Something bad happenedAgain", retVal);

You'll return from your function with the value retVal, if isItBad is false.

BUT - hiding return in a macro is a big NO-NO IMHO. Don't do that. What are you trying to save, 2 lines of code on an if?

edited Jul 27 '11 at 19:05

answered Jul 27 '11 at 17:30

littleadv

20,100
2
36
50

Looks good but thinking if it is do able using comma operator in a standard way. – balki Jul 27 '11 at 17:41
1

@balki: Why? The trickier you make this macro, the more likely it is to have some edge case where it fails horribly. KISS – Dennis Zickefoose Jul 27 '11 at 17:46
@balki - you can do it with `operator,`, but why? The result will be exactly the same, just harder to read and understand. – littleadv Jul 27 '11 at 17:52
The macro will anyway hide into some header. Wont the statements which use look more cleaner? By using your fn, I wont be able to print the arguments/state. @Dennis. Yes, but if I get it right once, i would save lots of code repetition and probably bugs. – balki Jul 27 '11 at 18:00
@balki: And this does not avoid code repetition? Wait, do you mean you want to pass the additional parameters as arguments to the macro? That's not the comma operator at all. – Dennis Zickefoose Jul 27 '11 at 18:05
@Dennis Yes. It looks like unnecessary `if`s everywhere but can't be avoided. I feel `if` will be more clear for the control flow of the program logic rather than for error checking in most of the lines. – balki Jul 27 '11 at 18:13
@balki - you can't write a generic function for a particular case. if you want to print the function arguments - you'll have to actually write code that does it. But that you do on the entry to the function, not where the error actually occurs. – littleadv Jul 27 '11 at 18:19
@littleadv, Yes. I am looking for a macro so that I can customize and write the log more verbose, For eg., `openFile(fileName,mode) || ReturnFalse("Cannot open File: " << fileName << "with the mode: " << mode);` – balki Jul 27 '11 at 18:34
@littleadv, your macro will not return from my function I want to return. I still have to write, `if(!Assert(cond,"cond Failed")) return false;` – balki Jul 27 '11 at 18:41
@balki Oh now I get what you want... Dude, that's ugly, don't do that. – littleadv Jul 27 '11 at 19:00

A. K. · Answer 3 · 2011-07-27T17:55:15.640

When you want a better method of error handling in a C++ program, you should probably have a look at the Appendix E of the TCPL Special Edition. It is better to avoid macro magic if you can. The appendix discusses techniques to be used while handling errors. This is also available at: http://www2.research.att.com/~bs/3rd_safe.pdf

EDIT: And for the macro magic case using the comma... This might be a possible way:

 #define ReturnFalse(error,a1,a2) \
 ({ \
  cerr << error << "@File: " << __FILE__ << "Line: " << __LINE__ << #a1<<" "<<#a2 <<endl; \
  return false; \
  false; \
 })

use case: ReturnFalse("Something bad happened.", arg1 , arg2)

Macro to log error and return false

3 Answers3