How to "exit current function, if error on current statement" in C++

Question

I start with an example to elaborate my problem. And conclude with the exact statement of the question at the end.

So in C, we can write a macro like this,

#define NO_ERROR 0
#define RETURN_IF_ERROR(function)                                       \
{                                                                       \
  RetCode = function;                                                   \
  if (RetCode != NO_ERROR)                                              \
  {                                                                     \
      LOG(ERROR,"[%d] line [%d] [%s]", RetCode, __LINE__, __FILE__ ) ); \
      return RetCode;                                                   \
  }                                                                     \
  else {                                                                \
      LOG(VERBOSE,"[%d] line [%d] [%s]", RetCode, __LINE__, __FILE__ ) );\
  }                                                                     \
}

Now this macro could be used in a function like this,

int ConstructAxes() {
    RETURN_IF_ERROR(GetAxis("alpha"));
    RETURN_IF_ERROR(GetAxis("beta"));
    RETURN_IF_ERROR(GetAxis("gamma"));
    RETURN_IF_ERROR(GetAxis("delta"));
    .
    .
    .
}

So, we exit the current function (e.g. ConstructAxes) immediately with an error code, if one of the functions called within, returns with an error. Without that macro, for each of those 4 statements, I would have to write an if...else block. I'd end up with 4 lines of code that shows the actual functionality or task and then another 16 lines of error-checking (and/or optional logging). (I have seen functions which are 700+ lines long with only 20~30 lines of functionality and 600+ lines of if...else error checking and logging. Not very easy to follow the main logic then.)

(p.s. before anyone points out, I cannot use exceptions. This is a legacy project and exceptions are not used and not desired to be used, nor am I an expert at writing exception-safe code. Also before anyone points out, the returned error code is reinterpreted into some meaningful text, at a higher level. Where and how is not relevant to this question at hand.)

The question is, macros can be problematic and I'd prefer a function. So is there some clean&elegant way to do this in C++ without using a macro? I have a feeling it is NOT possible.

It is theoretically possible with `setjmp`/`longjmp`. Not so much different from using exceptions. I'd recommend against it, **and** I wouldn't use this macro either. Better keep your functions small, and, if necessary, use a `goto` to one central error-handling per function. — , Jul 31 '18 at 08:08
"Is there a clean and elegant way to do this in C++ without using a macro?" Yes, that way is called Exception Handling. If you don't want to use that for whatever reason, then there isn't really a better way. — Zinki, Jul 31 '18 at 08:09
It's possible with `co_await` and a user-defined result type if the coroutines TS is merged closely enough to how it is now, but it's not exactly descriptive for that purpose or particularly cheap in its current state. — chris, Jul 31 '18 at 08:11
You *can* replace the macro with a variadic template, so you at least get the logging stuff moved out of the top function... but I would argue this wouldn't make things any less ugly. And as you mentioned "this is legacy and hence no exceptions", I doubt you'd get a pass on using variadic templates either. — DevSolar, Jul 31 '18 at 08:11
using a macro to throw exception is good because it allows to gather information (`__*__` macros) from the throw site without writing much code. — Tyker, Jul 31 '18 at 08:12
You have to use the macro if you want a control flow decision to be made by the "call". You could abstract the rest of the logic to a function although it doesn't really matter — M.M, Jul 31 '18 at 08:20
To the people writing "use exceptions" - it is not my decision. I follow the rules, not make them. And even if I had the freedom, I am not sure how exception handling would keep the code clean and readable in this scenario. <... thinking ...> Would I put all the statements in the try block and upon anyone of the called function statements e.g. GetAxis(blabla) failing, an exception would be emitted terminating any further execution of the current function e.g. ConstructAxes()? If yes, ok, that would solve the first problem of exiting upon an error.But what about logging with file&line macros? — Duck Dodgers, Jul 31 '18 at 08:36
Using that kind of macro in C would be impossible because it will skip function cleanup prior to returning. I'm not even talking about crippling debugging experience by putting return into macro and relying on verbose logging. — user7860670, Jul 31 '18 at 08:37
@JoeyMallone to solve the second problem you can throw in a macro that does what you want, log, gather information ... and throws — Tyker, Jul 31 '18 at 08:39
@JoeyMallone: IMHO, the macros does 2 different things: it logs and optionnaly exits the block. First part should go in a function, second part should use exception handling with a try block containing all of your calls and an empty catch. — Serge Ballesta, Jul 31 '18 at 08:43
@FelixPalmen, yes, thank you for that idea. I have seen that `goto` way, used in the code for error-handling like you said it. But I was never sure, if this was a good idea. I mean, it's `goto` ... :D. Another variant of this idea, using a `do {} while(0)` block, where a break statement is used to cause the same effect as a `goto` would. — Duck Dodgers, Jul 31 '18 at 08:45
@JoeyMallone "hiding" a `goto` behind something that does the same in less obvious code is IMHO a bad idea. Error handling with `goto` is perfectly fine **in C** when done correctly. This might be different in C++. Double-tagging a question is always a bit problematic. — , Jul 31 '18 at 08:47
@alk, Then would an error-handling tag do better? I can change it or remove it if it causes confusion. This question is mainly about how best to handle the part of the code that deals with the runtime-errors? Or do I misunderstand what runtime-errors are? — Duck Dodgers, Jul 31 '18 at 11:36

Serge Ballesta · Answer 1 · 2018-07-31T09:24:07.303

Ok, for an embedded device you may need to avoid any complex C++ goodies that could require memory allocation. But I would split the logging part which must always occur and the block short exit.

  int test_and_log(int RetCode) {
    if (RetCode != NO_ERROR)
      {
          LOG(ERROR,"[%d] line [%d] [%s]", RetCode, __LINE__, __FILE__ ) );
      }
      else {
          LOG(VERBOSE,"[%d] line [%d] [%s]", RetCode, __LINE__, __FILE__ ) );
      }
    return RetCode;
  }

Then a minimalist macro (the C way):

#define RETURN_IF_ERROR(x) { int cr; if ((cr = test_and_log(x)) != NO_ERROR) return cr; }

int ConstructAxes() {
    RETURN_IF_ERROR(GetAxis("beta"));
    RETURN_IF_ERROR(GetAxis("gamma"));
    RETURN_IF_ERROR(GetAxis("delta"));
    .
    .
    .
    return 0;                            // ensure a return value if every line passes
}

But for C++ I would still use minimalist exception handling by throwing a int value:

  void test_and_log(int RetCode) {
    if (RetCode != NO_ERROR)
      {
          LOG(ERROR,"[%d] line [%d] [%s]", RetCode, __LINE__, __FILE__ ) );
          throw RetCode;
      }
      else {
          LOG(VERBOSE,"[%d] line [%d] [%s]", RetCode, __LINE__, __FILE__ ) );
      }
  }

and then:

int ConstructAxes() {
  try {
    test_and_log(GetAxis("beta"));
    test_and_log(GetAxis("gamma"));
    test_and_log(GetAxis("delta"));
    .
    .
    .
  }
  catch (int RetCode) {
    return RetCode;
  }
  return 0;                            // ensure a return value if every line passes
}

This is rather hacky, because best practices recommend to only throw subclasses of std::exception to have consistent exception handling. But as you say that you do not want to use exceptions in your application it could be acceptable. The nice point on an embedded system is that no plain exception object is ever constructed. But please never use it in normal code.

And if you want to exchange memory for processing time, you can always declare test_and_log with an inline specifier...

"But as you say that you do not want to use exceptions in your application it could be acceptable." ... :) I am personally open to all solutions that lead to "clean code", that is easy to read, understand and modify. But exceptions are not allowed/desired as per our team-lead. Thank you for your answer. I find the first part suitable for my case already, where you talk about the 'C-approach'. I will think over all the suggested solutions today and see which one fits best. Thank you for your time. — Duck Dodgers, Jul 31 '18 at 09:24
@JoeyMallone The exception is thrown and catched in the same compilation unit, so it is not *far* exception handling, and has no impact at all on the other code. That is the reason why I have proposed it because I find it clean and neat. That being said, you (and you team leader) are on your own. I know that in a programming team *rules must be obeyed* ;-) — Serge Ballesta, Jul 31 '18 at 09:28
I think you should log the file and line of the problematic call, not of the `test_and_log()` function. — Walter, Jul 31 '18 at 11:15
@Walter: You are right of course. A macro should gather and pass them to the test_and_log function. But I'm too lazy to change it now... — Serge Ballesta, Jul 31 '18 at 11:27

Sander De Dycker · Answer 2 · 2018-07-31T08:53:49.433

1

While you cannot directly do what you want, you can get close :

int ConstructAxes() {
    int retCode = NO_ERROR;

    auto fail_if_error = [&retCode](int result) -> bool {
        retCode = result;
        if (retCode != NO_ERROR) {
            LOG(ERROR, "[%d] line [%d] [%s]", retCode, __LINE__, __FILE__);
            return false;
        }
        LOG(VERBOSE, "[%d] line [%d] [%s]", retCode, __LINE__, __FILE__);
        return true;
    };

    fail_if_error(GetAxis("alpha"))
      && fail_if_error(GetAxis("beta"))
      && fail_if_error(GetAxis("gamma"))
      && fail_if_error(GetAxis("delta"));

    return retCode;
}

edited Jul 31 '18 at 08:53

answered Jul 31 '18 at 08:39

Sander De Dycker

16,053
1
35
40

2

While this will kind of work you should mention the drastic overhead of spawning function objects for every function call. – user7860670 Jul 31 '18 at 08:43
1

It would be enough to pass the result of the function call instead of a function object... – Serge Ballesta Jul 31 '18 at 08:49
@VTT, ouch. Did I mention this goes into an embedded device. A soft-real time embedded device. Need to avoid overheads. Thank you for bringing that up. – Duck Dodgers Jul 31 '18 at 08:49
what resource is limited in the embedded device? Memory or processing time? If memory then your macro will cause the macro code to be inserted every time you use it, which is wasteful, but its fastest. – bcperth Jul 31 '18 at 08:55
@bcperth, processing time! We really can't afford to lose any cycles. 1ms delay is devastating. Memory, there is plenty (for the moment at least). – Duck Dodgers Jul 31 '18 at 08:58
3

Then be practical and stick with your original solution. No matter what other solution is found that may be more "elegant" or not, it will be slower. You could do things with C++, or like the solutions suggested, which may have less code, but will be slower I assure you! – bcperth Jul 31 '18 at 09:03
1

@JoeyMallone : the overhead of the fixed code (after VTT and Serge Ballesta's suggestions) should be minimal compared to the original macro code (the only reall difference is that it now uses a lambda rather than a macro, and some additional `&&` operations). But it might be a good idea to measure it nevertheless. – Sander De Dycker Jul 31 '18 at 09:03
@SanderDeDycker, ok. Thank you very much. I will test it and get back here in a little while. – Duck Dodgers Jul 31 '18 at 09:09

score 1 · Answer 3 · answered Jul 31 '18 at 09:16

Since you seem to be looking for more of a C solution, the only improvement I would suggest over what you have right now would be to have one point of error handling in the function (suggested by Felix in the comments) that would then also perform cleanup if needed.

#define RETURN_IF_ERROR(function)                                       \
{                                                                       \
  RetCode = function;                                                   \
  if (RetCode != NO_ERROR)                                              \
  {                                                                     \
      goto return_with_error;                                                   \
  }                                                                     \
  else {                                                                \
      LOG(VERBOSE,"[%d] line [%d] [%s]", RetCode, __LINE__, __FILE__ ) );\
  }                                                                     \
}

int ConstructAxes() {
    int RetCode;
    RETURN_IF_ERROR(GetAxis("alpha"));
    RETURN_IF_ERROR(GetAxis("beta"));
    RETURN_IF_ERROR(GetAxis("gamma"));
    RETURN_IF_ERROR(GetAxis("delta"));
    .
    .
    .
    return RetCode;
return_with_error:
    cleanup();
    LOG(ERROR,"[%d] line [%d] [%s]", RetCode, __LINE__, __FILE__ ) );
    return RetCode;
}

Using goto for error handling and cleanup in C is fine.

alk · Answer 4 · 2018-07-31T11:41:26.813

Talking C:

Your approach is OK, and marcos are only an issue if not used correctly.

So, I'd go for the well structured do {... break on error ...} while(0) approach. This also helps you sticking to the pattern, that a function shall have only one entry- and one exit-point.

Also for the sake of debugging and ease of reading, I'd move the "jump"-statement (break here) out of the macro.

#define NO_ERROR (0)

// log level IDs
#define LL_ERROR (0)
// more log levels here
#define LL_VERBOSE (10)
// more log levels here
#define LL_DEBUG (13)
// more log levels here
#define LL_MAXIMUM (16)

// log level long names
static const char * log_level_names[LL_MAXIMUM] = {
  "error",
  // more here
  "verbose"
  // more here
  "debug"
  // more here
}

int loglevel = LL_ERROR; // default logging-level to error; to be set to any LL_xxxx

// return date-time-stamp string (can be any, and most likely should be ;-)
#define DATETIMESTAMP_STR asctime(localtime(time(NULL)))

// Generic logging
#define LOG(ll, fmt, rc, ...) \
  while (ll <= loglevel) { \
    fprintf(stderr, "%s [%s]: " fmt, DATETIMESTAMP_STR, log_level_name[loglevel], __VA_ARGS__); \
    break; \
  };

// Specific logging
#define LOG_ERROR_OR_VERBOSE(rc, line, fname) \
  do { \
    LOG(NO_ERROR != (rc) ?LL_ERROR :LL_VERBOSE, "[%d] line [%d] [%s]", rc, line, fname); \
  while (0)


int foo(void)
{
  int result = NO_ERROR;

  LOG(LL_DEBUG, "entering '%s'", __func__);

  do {
    result = bar1(...);
    LOG_ERROR_OR_VERBOSE(result, __LINE__, __FILE__);
    if (NO_ERROR <> result) 
      { break; }

    result = bar2(...);
    LOG_ERROR_OR_VERBOSE(result, __LINE__, __FILE__);
    if (NO_ERROR <> result) 
      { break; }

    ...
  } while (0);

  LOG(LL_DEBUG, "leaving '%s' (rc = %d)", __func__, result);

  return result;
}

This roughly gives you a 1:3 signal/noise ratio, which you could approve significantly by changing

if (NO_ERROR <> result) 
  { break; }

to

if (NO_ERROR <> result) { break; }

Another possible improvement would be to change

result = bar1(...);
LOG_ERROR_OR_VERBOSE(result, __LINE__, __FILE__);

to

LOG_ERROR_OR_VERBOSE(result = bar1(...), __LINE__, __FILE__);

This leaves you with a SNR of 1, which is the optimum, I feel ... :-)

Jarod42 · Accepted Answer · 2018-07-31T12:03:00.293

You might write a loop, something like:

int RunFunctions(int line, // __LINE__
                 const char* file, // __FILE__
                 std::initializer_list<std::function<int()>> functions)
{
    int counter = 0;
    for (auto f : functions) {
        auto RetCode = f();

        if (RetCode != NO_ERROR) {
            LOG(ERROR,"[%d] line [%d] [%s] [%d]", RetCode, line, file, counter ));
            return RetCode;
        } else {
            LOG(VERBOSE,"[%d] line [%d] [%s] [%d]", RetCode, line, file, counter ) );
        }
        ++counter;
    }
    return NO_ERROR;
}

int ConstructAxes() {
    return RunFunctions(__LINE__, __FILE__, {
        []() { return GetAxis("alpha"); },
        []() { return GetAxis("beta"); },
        []() { return GetAxis("gamma"); },
        []() { return GetAxis("delta"); }
    });
}

Until we have std::source_location or similar, __LINE__, __FILE__ would be required.

If you don't need capture, you may change std::function by function pointers.

How to "exit current function, if error on current statement" in C++

5 Answers5

Linked