nick@??? wrote:

>   if (... incoming message at "frame" does not make sense ...) {
>     status = SYSTEM_STATUS_COMMAND_ERROR;
>     goto error;
>   }

...

>     if (status != SYSTEM_STATUS_SUCCESS)
>       goto error1;

...
...normal code stuff...
> status = SYSTEM_STATUS_SUCCESS;
> error1:
> ... put the chip back into normal mode ...
> error:
> msg_buf_reset(frame, NULL, 0);
> return status;
> }

> What do you think of the approach generally?

A problem that often occurs is that the error handling code is never
exercised. Error handling is only ever tested in the presence of an
error. Unless actively unit testing and forcing every possible error
that code is not ever executed. Errors in the error handling code
very often occur and only when the error condition is triggered
creating this cascade effect of not just having one actual error
condition but then a second due to a bug in the error handling code.

The above is a clever way to ensure that the error handling code is
always executed. It's executed as part of the normal processing as
part of the cleanup. It's only a very small amount of code, in this
case setting the status variable, that is part of the error handling
specifically. I find this a very interesting paradigm. I have always
considered goto to be a keyword of the damned but this causes me to
stop and think and consider that maybe I have been too harsh when it
is used appropriately.

Bob