6
\$\begingroup\$

I'm thinking about a unified error handling interface.

I've come to conclusion that the errno global variable style isn't so bad, and decided to implement an improved version of it: every function called sets an error code to a global error variable through the get/set functions.

The difference with the errno pattern is that the code is set even if there's no actual error (ERR_OK enum value), so we get a status of an operation after each function call. This allows us to not reserve a special "error" return value. The global error variable is of thread local storage type.

What pitfalls could be met with such approach?

err.h:

#ifndef ERR_H
#define ERR_H

enum err {
    ERR_OK
    ERR_SOME,
};

void err_set(enum err e);
int err_get(void);

#define ERR (err_get() != ERR_OK)

#endif /* ERR_H */

err.c:

#include "err.h"

_Thread_local static int error;

void err_set(enum err e) {
    error = e;
}

int err_get(void) {
    return error;
}

main.c:

#include <stdio.h>
#include "err.h"

static size_t get_size(void *arg) {
    if (!arg) {
        err_set(ERR_SOME);
        return 0;
    }

    err_set(ERR_OK);
    return sizeof(*arg);
}

int main(void) {
    void *bad = NULL;

    size_t size = get_size(bad);
    if (ERR) {
        printf("Bad pointer!\n");
        return 1;
    }

    printf("size == %zu\n", size);

    return 0;
}
\$\endgroup\$
  • 3
    \$\begingroup\$ Obvious pitfall: needing to check the error code of the call executed by another thread (that might have finished by now). \$\endgroup\$ – hoffmale Nov 16 '17 at 8:18
  • 1
    \$\begingroup\$ Another more tricky issue IMO is that before you read the error code you cannot call any other function. That will make your code as verbose as the error return code. Also a single enum for all the errors can be painful to maintain (see Windows API error codes..) \$\endgroup\$ – Adriano Repetti Nov 17 '17 at 18:44
4
\$\begingroup\$

Error handling is indeed an issue in C with a multitude of potential handling mechanisms. Good to see your take on it.

  1. Allow for atomic error handling. In addition to getting the error, it would be useful to have a test and clear to allow simpler error testing. The point is that once upper level code tests positive for an error (the error is caught), the global test status should be immediately cleared (atomic) to allow for subsequent setting without a chance of intervening code messing up things.

    bool err_test_and_clear(int *err) {
      // Simplistic code.  See also "C11 7.17 Atomics <stdatomic.h>" 
      if (err) {
        *err = error;
      }
      bool e = error != ERR_OK;
      error = ERR_OK;
      return e;
    }
    
    // usage
    foo();
    int err;
    if (err_test_and_clear(&err)) { 
      foo_clean_up();  // code may cause an error
      fprintf(stderr, "Error: %d\n", err);  // report original error
    }
    

    vs.

    // usage
    foo();
    if (ERR) {
      foo_clean_up(); // code may cause error - oops this should have been after below 2
      int err = err_get();
      err_set(ERR_OK);
      fprintf(stderr, "Error: %d\n", err);  // report maybe original error
    }
    
  2. Not a fan of ERR, which looks like a constant or object and it is not. Perhaps make it look like a function. Or be emphatic that ERR_OK is 0.

    // #define ERR (err_get() != ERR_OK)
    
    // better
    #define ERR() (err_get() != ERR_OK)
    
    // or best no define, just use `if(err_get())`
    
  3. If code like if (err_get() != ERR_OK) is preferred over if (err_get()), then code should use

    // _Thread_local static int error;
    _Thread_local static int error = ERR_OK;
    
  4. Missing ,

    enum err {
        ERR_OK    , // add ,
        ERR_SOME,
    };
    
  5. I'm leaning that _Thread_local static int error may benefit with being volatile. Have to ponder that more.

  6. For easier to read semantics, consider a clear function. err_clear(); has more clarity than err_set(ERR_OK); ("setting the error" of "OK error"). Further ERR_OK looks contradictory. Perhaps ERR_NONE?

    void err_clear(void) {
      error = ERR_OK;
    }
    
  7. Mixed "types". I'd expect both int or both enum err

                 vvvvvvvv
    void err_set(enum err e);
    int err_get(void);
    ^^^
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.