2

I have a thread in my operating system that is called in a fixed interval and then executes a list of 10-15 distinct functions sequentially. Each function has a return parameter that either says 0 (OK) or not 0 (error). Looks something like this:

while (1) {
    error &= function_1();
    error &= function_2(some_parameter);
    error &= function_3();
    handleError(error);
}

However it would be preferred that when one of the functions returns an error, the error is handled immediately and the other functions are not being executed anymore (single error failure).

For two functions I could do an if condition before each function but for 10-15 that would lead to a lot of unnecessary ifs.

For that I would use an array of function pointers that I go through sequentially:

int (*p_functions[3])() = { function_1, function_2, function_3 }
while (1) {
    for (int i = 0; i < 3, i++) {
        error = p_functions[i];
        if (error) {
            handle_error(error);
            break;
        }
    }
}

My issue here is that as you can see in the first example my function_2() has a parameter that gets maybe generated by another function beforehand. So I can't deal with functions that have different parameters.

Are there any other ways to solve that? Or maybe with some tricks for pointer casting? I heard dirty casting is a thing?

Jorengarenar
  • 2,705
  • 5
  • 23
  • 60
Julian
  • 915
  • 8
  • 24

6 Answers6

4

Given a function that returns 0 on success and 1 on error, you can change the existing code to use ||, which has short circuit behavior and more closely matches what you want to do anyway, instead of &:

while (1) {
    error = error || function_1();
    error = error || function_2(some_parameter);
    error = error || function_3();
    handleError(error);
}

Now, once error is set to 1, no further functions will be called.

As far as handling a specific error, you can set the variable with a function's return value shifted a certain amount based on which function failed, then check the bitmap in the error function.

uint32_t error_map = 0;
while (1) {
    error_map || (error_map |= (function_1()                << 0));
    error_map || (error_map |= (function_2(some_parameter)  << 1));
    error_map || (error_map |= (function_3()                << 2));
    handleError(error_map);
}

Then in handleError:

if (error_map & (1<<0)) {
    // function 1 error
}
if (error_map & (1<<1)) {
    // function 2 error
}
if (error_map & (1<<2)) {
    // function 3 error
}

If the functions could return any non-zero value on error, you can capture that error code in a separate variable:

uint32_t error = 0, error_map = 0;
while (1) {
    error_map||(error_map |= (((error = function_1()) != 0)               << 0));
    error_map||(error_map |= (((error = function_2(some_parameter)) != 0) << 1));
    error_map||(error_map |= (((error = function_3()) != 0)               << 2));
    handleError(error, error_map);
}

And the above with a macro to make it more readable:

#define RUN_ON_NO_ERROR(error, error_map, index, call) \
  ((error_map)||((error_map) |= ((((error) = (call)) != 0) << (index))))

uint32_t error = 0, error_map = 0;
while (1) {
    RUN_ON_NO_ERROR(error, error_map, 0, function_1());
    RUN_ON_NO_ERROR(error, error_map, 1, function_2(some_parameter));
    RUN_ON_NO_ERROR(error, error_map, 2, function_3());
    handleError(error, error_map);
}
dbush
  • 205,898
  • 23
  • 218
  • 273
  • thanks! whats the benefit of using the error map compared to just enums? – Julian Sep 28 '21 at 12:32
  • My functions would return a value thats NOT 0, but the value indicates the error type. Might be able to solve that with the maps... – Julian Sep 28 '21 at 12:39
  • The converse strategy has also been used, for functions that return false on failure: `bool ok = true; ok = ok && function1(); ok = ok && function2(); ... ` – Tim Randall Sep 28 '21 at 12:39
  • 1
    `error_map || error_map |= ((error = function_x()) != 0) << x` would even retain a specific error code if that's of interest... – Aconcagua Sep 28 '21 at 12:39
  • wouldnt error |= function1() be the same and even shorther? – Julian Sep 28 '21 at 13:12
  • This version has the same flaw as the macro version in another answer: lots of needless branching. If there was an error there's no need to repeatedly check for it over and over. – Lundin Sep 28 '21 at 13:15
  • @Lundin In this case it think that falls under premature optimization. This type of checking is more readable, and even more so with macros. – dbush Sep 28 '21 at 13:34
  • How would the compiler be able to optimize the checks, since the right operand needs to be there in case there were no errors, and that one contains side effects. I don't see how this will get optimized well. There's pre-mature optimization and then there's pre-mature assumptions that the compiler can do magic :) – Lundin Sep 28 '21 at 13:45
3
#define E(e,x)  e = (e ? e : x)
while (1) {
    error = 0;
    E(error, function_1());
    E(error, function_2(some_parameter));
    E(error, function_3());
    handleError(error);
}

Isn't too bad; this is the style things like the SV test suite are written in; and the actual error value is preserved.

mevets
  • 10,070
  • 1
  • 21
  • 33
  • Cool thanks! I like both approaches and probably will combine them, so my macro will say: E(e,x) e = e || x – Julian Sep 28 '21 at 12:35
  • Coming up with "secret macro languages" is generally not a good idea, so this isn't something I would advise, other than as last resort when maintaining some old code base. – Lundin Sep 28 '21 at 12:44
  • 2
    Btw this macro sneakily hides away a lot of pointless branching. Suppose the first function fails. You then have to check the error result for every following function, each such check including a pointless branch. So not only is this some mysterious macro language, it's also needlessly slow. It's essentially an obscure way of writing an `if-else if` chain. – Lundin Sep 28 '21 at 13:12
  • It sneakily hides almost nothing; it merely makes the intent obvious, keeps the code readable, and employs a common idiom. This idiom is prevalent in *safety* software, as it minimizes the nesting, thus cognitive load of understanding; and leaves it to the compiler to draw the efficiency. A quick *cc -S* shows modern compilers have no problem with this idiom. – mevets Sep 29 '21 at 01:05
  • I disagree that this code is the correct way in safety-related software too. See https://stackoverflow.com/a/42577015/584518. Multiple return statements is clearer, faster and safer. Bit of a pet peeve of mine that I've raged plenty about, to the MISRA-C committee among others (who at least listened enough to relax the rule banning multiple returns from required to advisory). – Lundin Oct 07 '21 at 13:23
  • Also in safety-related software it is generally frowned upon to write code with lots of "cyclomatic complexity" - that is several possible execution paths in the same function. This is 100% in line with reducing branches, so it isn't just for performance but for reducing complexity and making the code easier to test (code coverage tests etc). – Lundin Oct 07 '21 at 13:25
  • Well, no. `err = b(); if (!err) { err = c(); } if (!err) { err = d(); }` generates a very low complexity score, compared to ` err = b(); if (!err) { err = c(); { if (!err) {err =c (); } } }` which is just one of the errors of following poorly examined metrics like McCabe's. MISeRAble is yet another standard, not particularly good, but not much worse than the C, C++, or POSIX standard committees output. But committees don't matter, you want certification, you abide by the rules. Btw; compilers reduce the so-called performance argument to silliness. – mevets Oct 08 '21 at 04:06
  • This is speaking from someone who has spent 31 years implementing commercially successful safety certified kernels; and was shamefully part of the P1003.4 working group. – mevets Oct 08 '21 at 04:08
1

The function pointer alternative is how you often implement schedulers, state machines and similar. The problem with function pointers is however that there exists no generic function pointer like void* for object pointers.

The answer to the question is to not have functions with different parameters. You'll have to design an API that fits all use-cases. In the most simple form, that's none by letting the function take a void pointer parameter, which is optional.

Full example:

#include <stdio.h>

typedef enum
{
  ERR_NONE,
  ERR_THIS,
  ERR_THAT,
} err_t;

typedef err_t func_t (void* opt);

static err_t function1 (void* opt);
static err_t function2 (void* opt);
static err_t function3 (void* opt);

static void handle_error(err_t err);

int main (void)
{
  func_t* const functions[] = 
  {
    function1,
    function2,
    function3,
  };
  
  int something_for_function_3;
  void* params[] = 
  {
    &something_for_function_3,
    "hello",
    &something_for_function_3,
  };

  for(size_t i=0; i< sizeof functions/sizeof *functions; i++)
  {
    err_t err = functions[i] (params[i]);
    if(err != ERR_NONE)
    {
      handle_error(err);
      // break; // stop here if necessary
    }
  }
}

static err_t function1 (void* opt)
{
  *(int*)opt = 123;
  puts(__func__);
  return ERR_NONE;
}

static err_t function2 (void* opt)
{
  printf("%s: %s\n", __func__, (const char*)opt);
  return ERR_THIS;
}

static err_t function3 (void* opt)
{
  printf("%s: %d\n", __func__, *(int*)opt);
  return ERR_THAT;
}

static void handle_error(err_t err)
{
  printf("Bad things happened, code: %d\n", err);
}

Output:

function1
function2: hello
Bad things happened, code: 1
function3: 123
Bad things happened, code: 2
Lundin
  • 195,001
  • 40
  • 254
  • 396
  • params are dynamic, i.e. the func1 might generate a param for func2 – Julian Sep 28 '21 at 12:59
  • @Julian Yeah so just pass a pointer to a variable which can get updated. I wrote `void*` and not `const void*` on purpose. – Lundin Sep 28 '21 at 13:05
  • @Julian Although: consider if communicating through some scheduler in the caller application is the correct way for those functions to communicate with each other? Maybe placing them in the same translation unit and using a `static` file scope variable will suffice? Much easier and cleaner API, but not thread-safe. – Lundin Sep 28 '21 at 13:08
  • @Julian I updated the example to show how you could pass parameters between functions if needed. Still somewhat questionable design, but if those two functions reside in wildly unrelated parts of the code, it might be motivated. – Lundin Sep 28 '21 at 13:24
  • Some examples of generic programming https://stackoverflow.com/a/69379395/584518 – Lundin Oct 07 '21 at 13:29
1

You can use generic type pointer void* as function parameters and create another array of pointers that have same indices as array of func pointers. Then for each call to a function you retrieve the corresponding parameter reference, if no parameter required simply a NULL for empty parameters. OR alteratively you can use a task structure which includes a pointer to a function and a generic type parameter. This will be more flexible and let you deal with as many function as you need. Here I modified your code and tested it:

#include <stdio.h>
#include <stdlib.h>

int function_1(void* param){
    if(param == NULL) printf("function_1 parameter is NULL\n");
    return 1;
}

int function_2(void* param){
    if(param == NULL) printf("function_2 parameter is NULL\n");
    printf("function_2 parameter value is: %d\n", *((int*)param));
    return 0;
}

int function_3(void* param){
    if(param == NULL) printf("function_3 parameter is NULL\n");
    return 1;
}

int main()
{
    int p_f2 = 100;
    void* param_f2 = &p_f2;
    int (*p_functions[3])(void*) = { function_1, function_2, function_3 };
    void* params[] = { NULL, param_f2, NULL };

    while (1) {
        for (int i = 0; i < 3; i++) {
            int error = p_functions[i](params[i]);
            if (error == 1) {
                printf("function %d returned with error\n", i+1);
            }
        }
    }

}

Since the output is large because of the while loop I just share only for one loop. The output is:

function_1 parameter is NULL
function 1 returned with error
function_2 parameter value is: 100
function_3 parameter is NULL
function 3 returned with error

Kozmotronik
  • 2,080
  • 3
  • 10
  • 25
  • why don't you have to specifcy the two void pointers in the function pointer array in line 3 of the main function? int (*p_functions[3])(void param1, void param2) ? – Julian Sep 30 '21 at 09:37
  • Do you mean the definiton of function pointer? It would be like this: `int (*p_functions[3])(void* param1);` The function pattern is totally specific for use cases. Since you are implementing an OS with threads, it is more versatile and common to implement thread function pattern with a parameter of void pointer. Have look at the [FreeRTOS](https://www.freertos.org/implementing-a-FreeRTOS-task.html)'s task example. – Kozmotronik Sep 30 '21 at 10:04
0

One way I have seen is the use of a macro such as FUNC_RET_IF_ERR below:

 1 #include <stdio.h>
 2 #include <stdlib.h>
 3   
 4 #define FUNC_RET_IF_ERR(func, ...) \
 5     do {                           \
 6         if (func(__VA_ARGS__) < 0) \
 7             return EXIT_FAILURE;   \   
 8     } while(0)
 9   
10 void print_integer(char *designator, int value)
11 {
12     printf("%s = %d\n", designator, value);
13 }
14  
15 int func_arg_must_be_positive(int i)
16 {
17     if (i < 0)
18         return -1; 
19  
20     return 0;
21 }
22  
23 int main()
24 {
25     print_integer("line", __LINE__);
26  
27     FUNC_RET_IF_ERR(func_arg_must_be_positive, 1); 
28  
29     print_integer("line", __LINE__);
30  
31     FUNC_RET_IF_ERR(func_arg_must_be_positive, -1);
32  
33     print_integer("line", __LINE__);
34 }

This will output:

$ gcc main.c && ./a.out
line = 25
line = 29
Jardel Lucca
  • 1,115
  • 3
  • 10
  • 19
0

Why not obfuscate this even more by using short-circuit evaluation from || operator.

error = function_1() ||
        function_2(some_parameter) ||
        function_3();

However, the typical pattern for handling this type of code is by using using a cascade of goto to cleanup/error handlers:

error = function_1();
if (error) goto handle_error;

error = function_2(some_parameter)
if (error) goto cleanup_funtion_1;

error = function_3();
if (error) goto cleanup_function_2;

// ... enjoy SUCCESS !
return;

cleanup_function_2:
  // release resource from function_2
cleanup_function_1:
  // release resource from function_1
handle_error:
  handleError(error);
return;

This pattern allows safe release of any resources (i.e. memory, threads, opened files) that were acquired along execution steps. Moreover, it allows to reverse results of some operations if it's possible.

When no resources are taken by the functions then the pattern could squashed to:

error = function_1();
if (error) goto handle_error;
error = function_2(some_parameter)
if (error) goto handle_error;
error = function_3();
if (error) goto handle_error;
// success
return;

handle_error:
  handleError(error);
  return;
tstanisl
  • 13,520
  • 2
  • 25
  • 40
  • This feels like very old style from the BASIC era though. And lots of code repetition, which isn't ideal. – Lundin Sep 28 '21 at 12:53
  • @Lundin, may I ask what is repeated? – tstanisl Sep 28 '21 at 12:54
  • I considered that, but I can avoid using goto here by just saying if(!error) error = function, then I don't have to use goto at all. – Julian Sep 28 '21 at 12:56
  • @Julian, don't get confused by the "anti-goto" propaganda, even the latest MISRA C allows **forward** "goto" jumps as an acceptable practice for error handling – tstanisl Sep 28 '21 at 13:00
  • yeah true. I found this: https://embeddedgurus.com/barr-code/2018/06/cs-goto-keyword-should-we-use-it-or-lose-it/ interesting read. Even linux kernel uses goto error handling. – Julian Sep 28 '21 at 13:03
  • You are writing essentially the same code 3 times? And it will only grow as more functions get added. Not ideal for maintenance. – Lundin Sep 28 '21 at 13:03
  • @Lundin, the OP query was about avoiding moving through multiple "if" conditions by handling errors as fast as possible. – tstanisl Sep 28 '21 at 13:04
  • @Julian, I mean that function just do something that has no complementary pair. Like `fprintf` vs `fopen` + `fclose` pair – tstanisl Sep 28 '21 at 13:09
  • Yup understood, thanks. – Julian Sep 28 '21 at 13:10
  • @Julian if(!error) error = function involves repeated checking over and over, so that makes the program needlessly slow. The goto stops execution upon error. – Lundin Sep 28 '21 at 13:18