Avoiding redundancy in checking the return values from C subroutines?-CodePudding

There is so much redundancy in the following code:

int init( )
{
    int err;

    err = init_a();
    if(err)
        return -1;

    err = init_b();
    if(err)
        return -1;

    err = init_c();
    if(err)
        return -1;

   // etc.

    return 0;
}

Is there a better way of checking the err return values?

CodePudding user response：

If you really have large numbers of init functions and are not afraid of function pointers,

#include <stddef.h>

int init_a(void), init_b(void), init_c(void);
int init(void);

static int (*const func[])(void) = {
    &init_a,
    &init_b,
    &init_c,
    // etc.
};

#define ELEMENTS(array) (sizeof(array) / sizeof(array[0]))

int init(void) {
    for (size_t i = 0; i < ELEMENTS(func);   i) {
        if (func[i]() != 0) {
            return -1;
        }
    }
    return 0;
}

For another init_x() all you need to do is insert it at the appropriate index of the func[] array. It's also super easy, barely an inconvenience, to swap around and delete functions. You could even return 1 i; if you want to know which function failed.

Now your init() has become data driven by the func[] array (as opposed to code driven by statements in init()).

This only works if your init_* functions have the same prototype and take the same arguments, if any.

CodePudding user response：

Use Short-circuit Evaluation:

bool init( )
{
    return init_a() || init_b() || init_c();
}

If you can accept using a boolean value as final indication of success or failure (in my example failure = true), then Short-circuiting operators can make things quite compact.

The C language employs short-circuiting, so in a logical expression with several operands, the code will only execute "as far as is needed" to deduce the final value of the expression. So if A is true in the expression A || B || C then B and C will not be evaluated. If none of the init_N() functions return failure, then the return value will be success.

You can of course negate the logic if you prefer success = true with return A && B && B (but your code suggests success is false)

Also, if you want to allow the cause of the error, ie an error code, to be reported back to the caller, just pass in a pointer to an error code, and let all sub-inits set it to something useful:

bool init(int* err)
{
    return init_a(err) || init_b(err) || init_c(err);
}

CodePudding user response：

If these initialization sub-processes must be called in order, which is common in hardware/driver software, status check after each step is necessary. In your example, return a different error code is more indicative.

int init(void) {
  int err = init_a();
  if (0 != err) {
    return -1;
  }

  err = init_b();
  if (0 != err) {
    return -2;
  }

  err = init_c();
  if (0 != err) {
    return -3;
  }

  // etc.

  return 0;
}

CodePudding user response：

The code can be written more compactly, even as one-liners as stated in the comments, but it may be hard to maintain. I would personally go for something like this (maybe adding curly braces after the if's):

int init( )
{
    if(init_a())
        return -1;
    else if(init_b())
        return -1;
    else if(init_c())
        return -1;

   // etc.

    return 0;
}

However, it is important to notice that (almost) no matter how this code is written, a decent level of compiler optimization will turn it into identical binary code. See examples here: https://godbolt.org/z/s8fb1czh3

Thus, the priority should be on code clarity (which again is a matter of context and preference).

CodePudding user response：

There is so much redundancy in the following code:

I don't know about that. There is a repeating motif, which I guess is what you are talking about, but nothing there is redundant. If you are looking for DRYer code, then this sort of thing can be addressed with the help of preprocessor macros:

#define RETURN_IF_NZ(x, r) do { if (x) return (r); } while (0)

int init(void) {
    RETURN_IF_NZ(init_a(), -1);
    RETURN_IF_NZ(init_b(), -1);
    RETURN_IF_NZ(init_c(), -1);

    // ...

    return 0;
}

Among the advantages are:

No (explicit) repetition of the error-handling code
The behavior of the function is more easily followed (once the macro is understood) because the key elements are less obscured by error-handling scaffolding

Among the disadvantages are:

Not as easy to debug as the original version (but easier than some alternatives that have been presented)
Humans have to know, guess, or look up the meaning of the macro

Macro usage is poorly regarded by some, on the basis that functions should be preferred for most purposes. However, this is among the cases that macros can handle, but functions can't.