Error handling for multi-layer implementation in C-CodePudding

I am writing firmware for a device in C. The Software allows the PC to communicate with this device over the serial interface (UART). The firmware contains multiple layers as following:

Communication layer to send and receive data over UART.
Block layer: This layer enables/disables certain blocks on the device by writing data to the device over UART.
API layer: This contains a sequence of calls to the routines in the block layer. It is used to enable or disable a set of blocks on the device.

My problem is the error handling since in C there are no exceptions. Here is how I've implemented my firmware and I am trying to see if there is a more efficient and compact way to build it while still handling errors efficiently. I want to avoid checking at each layer the status of the call of the lower layer. The code below is very compact and in reality, I have a long sequence of send_uart_commands in the block layer.


// Communication layer

operation_status_t send_uart_command(command_id_t id, command_value_t value)
{
    // Send data over UART
    // Return success if the operation is successful; otherwise failure
}

// Block layer

operation_status_t enable_block1(void)
{
    if (send_uart_command(BLOCK1_COMMAND_1, 10) != operation_success)
           return operation_failure;

    if (send_uart_command(BLOCK1_COMMAND_2, 20) != operation_success)
           return operation_failure;

    // A list of sequences

    if (send_uart_command(BLOCK1_COMMAND_N, 15) != operation_success)
           return operation_failure;

    return operation_success;
}

operation_status_t enable_block2(void)
{
    if (send_uart_command(BLOCK2_COMMAND_1, 1) != operation_success)
           return operation_failure;

    if (send_uart_command(BLOCK2_COMMAND_2, 8) != operation_success)
           return operation_failure;

    return operation_success;
}

// API layer

operation_status_t initialize(void)
{
    if (enable_block1() != operation_success)
           return operation_failure;

    if (enable_block2() != operation_success)
           return operation_failure;

   // A list of calls to the functions in the block layer

    return operation_success;
}

CodePudding user response：

One of the many big problems with exception handling like in C is that exceptions can crash through all layers like a cannonball. So when you are writing some completely unrelated code, you suddenly get that cannonball in your face: "UART framing error!" When you haven't even touched the UART code...

Therefore "I want to avoid checking at each layer the status of the call of the lower layer" is the wrong primise. Rather you should do like this:

Check errors at each layer.
Deal with errors as close to the error source as possible.
Only forward errors to the caller in case they are actually meaningful to them.
You can rename/change type of errors to suit the caller along the way.

For example: "UART framing error" might useful to the code calling the UART driver, but useless to the higher layer application. "Possible incorrect baudrate settings" might be a more relevant error description which you should pass along. Though in some cases you want detailed errors available even at the higher layers.

One reason why you might want that, is that it's common and often good design to have a centralized error handler on the top layer, which can make decisions of state changes, print/log errors etc from one single place in the code. Instead of doing that from all over the place. You'll often find the top layer of a microcontroller application looking something like this:

void main (void)
{
  /* init & setup code called */

  for(;;)
  {
    kick_watchdog(); // the only place in the program where you do this
      
    result = state_machine[state]();
    
    if(result != OK)
    {
      state = error_handler(result);
    }
  }
}

As for your specific code, it looks just fine and mostly doesn't contradict anything of what I've written above. It is always good to return an error code upon error - less confusing than goto, or even worse: massively nested statements and/or loops with error condition flags.

CodePudding user response：

Your code is fine. Actually, it's a bad practice to avoid explicit error checking in C. But if you really want it you could use longjmp. But you should use it with a great care.

This feature allows to jump through the stack skipping arbitrary number of nested calls.

Below you can find an exemplary with mocked up send_uart_command().

#include <setjmp.h>
#include <stdio.h>

jmp_buf env;

void send_uart_command_impl(const char *cmd, int val) {
    static int left = 3;
    if (left-- == 0) {
        printf("%s(%d): failed\n", cmd, val);
        longjmp(env, 1);
    }
    printf("%s(%d): success\n", cmd, val);
}

#define send_uart_command(name, val) send_uart_command_impl(#name, val)

void enable_block1(void) {
    send_uart_command(BLOCK1_COMMAND_1, 10);
    send_uart_command(BLOCK1_COMMAND_2, 20);
    send_uart_command(BLOCK1_COMMAND_N, 15);
}

void enable_block2(void) {
    send_uart_command(BLOCK2_COMMAND_1, 1);
    send_uart_command(BLOCK2_COMMAND_2, 8);
}

int initialize(void) {
    if (setjmp(env)) return -1;
    enable_block1();
    enable_block2();
    return 0;
}

int main() {
    if (initialize() != 0)
        puts("initialize failed");
    else
        puts("initialize success");
}

The program is constructed to fail on the 4th invocation of send_uart_command(). Adjust variable left to select other invocation.

The program logic is very streamlined and it prints the expected output:

BLOCK1_COMMAND_1(10): success
BLOCK1_COMMAND_2(20): success
BLOCK1_COMMAND_N(15): success
BLOCK2_COMMAND_1(1): failed
initialize failed