Using a linked list as an error type

Question

Background

I've been looking into a few different way to implement error reporting for an embedded system in C and there a couple things that I'd like to incorporate:

• A generic and extensible error type.
• Backward compatibility with OK and ERROR returns (i.e. can effectively return 0 or 1).
• Well defined error codes on a per layer/module basis (i.e. a typedef'd enum).
• Multiplexed error codes.
• Ideally posix compliant.

I also like using rust's result type as it provides a well defined method for handling returned values. I managed to come up with a header-only implementation in C similar to that found here. But I don't want to focus on that here. I only mentioned as context for how I plan to isolate the complexity.

Error Type

I tend to support "multiplexed error codes" via bit masking intergers. This tends to be implemented like so:

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

typedef enum {
    NO_ERROR       = 0,
    ERROR_OVERTEMP = 0x0001,
    ERROR_NO_READ  = 0x0002,
} error_e;

float readVal(void) {
    return 0.0;
}

uint32_t getSensorVal(float *returnVal) {
    uint32_t errorRet = 0;
    float ret = readVal();
    if (ret == 0) {
        errorRet |= ERROR_NO_READ;
    }
    return errorRet;
}

int main() {
    float val;
    int ret = getSensorVal(&val);
    if (!ret) {
        printf("Sensor Val: %f \n", val);
    }
    else {
        printf("Oh no we have an error \n");
    }
    return 0;
}

This works well enough but is somewhat limiting in that the number of error codes for a given enumeration set can only be as many as 32 (64 on 64 bit systems). This can be problematic when errors are passed throughout a vast system. This led me to come up with the following typedef:

typedef struct errorList_s {
    unsigned const type;
    uint32_t value;
    struct errorList_s *next;
} errorList_t;

This allows developers to build on interfaces with error codes that are extremely extensible. Its use might look something like the following:

typedef enum {
    NO_ERROR       = 0,
    ERROR_OVERTEMP = 0x0001,
    ERROR_NO_READ  = 0x0002,
} error_e;

uint32_t getSensorVal(float *returnVal) {
    uin32_t errorRet = 0;
    float ret = readVal()
    if (ret == 0) {
        errorRet |= ERROR_NO_READ;
    }

}

int main() {
    float val;
    int ret = getSensorVal(&val);
    if (!ret) {
        printf("Sensor Val: %f \n", val);
    }
    else {
        printf("Oh no we have an error \n");
    }
    return 0;
}

This works well enough but is somewhat limiting in that the number of error codes for a given enumeration set can only be as many as 32 (64 on 64 bit systems). This can be problematic when errors are passed throughout a vast system. This led me to come up with the following typedef:

typedef struct errorList_s {
    unsigned const type;
    uint32_t value;
    struct errorList_s *next;
} errorList_t;

This allows developers to build on interfaces with error codes that are extremely extensible. Its use might look something like the following:

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

typedef struct errorList_s {
    unsigned type;
    uint32_t value;
    struct errorList_s *next;
} errorList_t;

typedef enum {
    SENSOR_ERROR = 0,
    DEVICE_ERROR = 1,
} errorTypes_e;

typedef enum {
    OVER_TEMP    = 0x001,
    OVER_CURRENT = 0x002,
} sensorErrors_e;

typedef enum {
    DEVICE_UNRESPONSIVE = 0x001,
    BAD_DEVICE_ID       = 0x002,
} deviceErrors_e;

/* Dummy impl */
float readVal(void) {
    return 0.0f;
}

errorList_t *getDevice_MotorPos(float *pos) {
    errorList_t *devError = NULL;
    errorList_t *sensorError = NULL;
    float ret;
    /* Some logic says that a read of zero from some hw device means
       overtemp*/
    ret = readVal();

    if (ret == 0.0f) {
        sensorError = (errorList_t *) calloc(1, sizeof(errorList_t));
        sensorError->type = SENSOR_ERROR;
        sensorError->value |= OVER_TEMP;
        devError = (errorList_t *) calloc(1, sizeof(errorList_t));
        devError->type = DEVICE_ERROR;
        devError->value |= DEVICE_UNRESPONSIVE;
        devError->next = sensorError;
    }
    else {
        *pos = ret;
    }

    return devError;
}
void sensorErrorHandler(errorList_t *const err);
void deviceErrorHandler(errorList_t *const err);

void selectErrorType(errorList_t *const err){
    switch (err->type) {
        case SENSOR_ERROR:
        sensorErrorHandler(err);
        break;
        case DEVICE_ERROR:
        deviceErrorHandler(err);
        break;
        default:
            printf("Unkown error type %d \n", err->type);
    }
}

void sensorErrorHandler(errorList_t *const err) {
    if (err->value & OVER_TEMP) {
        printf("Sensor read over temp \n");
    }
    if (err->value & OVER_CURRENT) {
        printf("Sensor read over current \n");
    }
    if (err->next) {
        selectErrorType(err->next);
    }
}

void deviceErrorHandler(errorList_t *const err) {
    if (err->value & BAD_DEVICE_ID) {
        printf("Invalid device ID used to contact device \n");
    }
    if (err->value & DEVICE_UNRESPONSIVE) {
        printf("Device unresponsive \n");
    }
    if (err->next) {
        selectErrorType(err->next);
    }
}

int main() {
    float val;
    errorList_t *const ret = getDevice_MotorPos(&val);
    if (!ret->value) {
        printf("Sensor Val: %f \n", val);
    }
    else {
        selectErrorType(ret);
    }
    return 0;
}

Other than the inherent complexity I'm introducing by requiring "unwrapping" of errors (which will be handled by helper macros)

Is there something about this that seems wrong? Is there something here to be improved upon? I'd like to implement this in such a way that is posix compliant although it doesn't seem immediately obvious how to do that.

All feedback is much appreciated.

EDIT:

If we consider the example as being in some file errorTest.c this can be compiled and run with:

gcc errorTest.c -o errorTest
./errorTest

If implemented formally I would likely create an error handler interface typedef and do more complicated things than just printing errors. After writing this out it also strikes me that this could be used for more in depth status reporting than just error reporting.

Answer 1

What is the goal?

This works well enough but is somewhat limiting in that the number of error codes for a given enumeration set can only be as many as 32 (64 on 64 bit systems).

So 4294967296 possible error codes (or 18446744073709551616 on 64-bit systems) isn't extensible enough? You don't need to use bitmasks you know; in the example you gave you would never have both ERROR_OVERTEMP and ERROR_NO_READ set at the same time. Anyway, there are two parts to your errorList_t:

1. You have a structured error that is split into a category (type) and an error code within that category (value).
2. You have a way to return multiple error codes in one go.

The question is though: who is this for, and what can they do with this information? Is it for the end user, or are these codes supposed to be handled by the application? If this list of errors is meant to be read by the end user, then they are not interested in codes. They just want to see error messages. Instead of storing type and value, why not just store a pointer to a string?

typedef struct errorList_s {
    const char *message;
    struct errorList_s *next;
} errorList_t;

And then use it like so:

errorList_t *error = ...;
error->message = "Device unresponsive";
return error;

However, instead of waiting with printing the error message until a whole linked list is returned, why not immediately print the error message, and just return an error code? Then again, if this is for an embedded system, would the user ever get to see an error message?

If it's meant for the application, then consider whether it is useful at all to have a list of errors. And what does it mean to get multiple errors returned? Are they all independent errors, for example if a function tried to read from 10 different sensors, and 3 of the sensors returned an error? Or is it more like a stack, where each error goes into more detail?

Consider that most error handling is rather simplistic, and looks like:

error = someFunction(...);

if (error) {
    cleanUp();
    return error;
}

There is almost never any parsing being done. The few exceptions I know of are when writing networking code, where you might want to distinguish between a real failure and something like EINTR being returned, or when creating a file and you want to distinguish between really not being able to create one, or perhaps the file already existing. In either case, the application almost always only needs a few bits of information about the cause of the error, and certainly not a linked list of error codes.

After writing this out it also strikes me that this could be used for more in depth status reporting than just error reporting.

It might be good to come up with some practical use cases for this before spending too much time on creating an error reporting framework.

Add functions to manipulate error lists

You spend a lot of lines of code creating a linked list:

errorList_t *devError = NULL;
errorList_t *sensorError = NULL;

if (ret == 0.0f) {
    sensorError = (errorList_t *) calloc(1, sizeof(errorList_t));
    sensorError->type = SENSOR_ERROR;
    sensorError->value |= OVER_TEMP;
    devError = (errorList_t *) calloc(1, sizeof(errorList_t));
    devError->type = DEVICE_ERROR;
    devError->value |= DEVICE_UNRESPONSIVE;
    devError->next = sensorError;
}

return devError;

Ideally you want to create some functions that do all this work for you. For example, the above could be rewritten as:

errorList_t *errors;

if (ret == 0.0f) {
    errors = createError(SENSOR_ERROR, OVER_TEMP);
    errors = appendError(errors, DEVICE_ERROR, DEVICE_UNRESPONSIVE);
}

return errors;

Avoid memory leaks

You should also clean up an errorList_t after you've used it. Again, writing a function that frees all the elements in a given list would be best.