1

My code has many checks to detect errors in various cases (many conditions would result in the same error), inside a function returning an error struct. Instead of looking like this:

err_struct myfunc(...) {
    err_struct error = { .error = false };

    ...

    if(something) {
        error.error = true;
        error.description = "invalid input";
        return error;
    }

    ...

        case 1024:
            error.error = true;
            error.description = "invalid input";  // same error, but different detection scenario
            return error;
            break;  // don't comment on this break please (EDIT: pun unintended)
            ...

Is use of goto in the following context considered better than the previous example?

err_struct myfunc(...) {
    err_struct error = { .error = false };

    ...

    if(something) goto invalid_input;

    ...

        case 1024:
            goto invalid_input;
            break;

    return error;

    invalid_input:
        error.error = true;
        error.description = "invalid input";
        return error;
  • 3
    it's used like an exception here which has been one of the main reasons to use goto for proper cleanup/rollback – ratchet freak Aug 17 '14 at 19:43
  • see also: [Approaches to checking multiple conditions?](http://programmers.stackexchange.com/q/191208/31260) – gnat Mar 25 '15 at 06:43

2 Answers2

3

if using goto your code reads better, use it. It is a language feature like every other. We are anymore at the age of Dijkstra.

http://david.tribble.com/text/goto.html

int parse()
{
    Token   tok;

reading:
    tok = gettoken();
    if (tok == END)
        return ACCEPT;
shifting:
    if (shift(tok))
        goto reading;
reducing:
    if (reduce(tok))
        goto shifting;
    return ERROR;
}

This is a case where goto-s are even more elegant than whatever other structure.

Emilio Garavaglia
  • 4,289
  • 1
  • 22
  • 23
1

When you see a good move, look for a better one.
—Emanuel Lasker, 27-year world chess champion

Cleanup/error handling of that form is one of the acceptable uses of goto, especially in C. Using a goto is better than repeating those three lines, but you can do better still. Always look for the better move.

For one thing, it's unlikely those errors are truly identical. At some point, someone is going to wonder which condition you've actually hit, then you're back where you started. You need a succinct way to distinguish between different errors at the point they are detected.

Here's one solution:

const err_struct myfunc(...) {
    const err_struct success       = { .error = false };
    const err_struct invalid_input = { .error = true, .description = "invalid input" };

    ...

    if(something) 
        return invalid_input;

    ...

        case 1000:
            return custom_error("Expected %d, found 1000", expected);

        case 1024:
            return invalid_input;

    ...

    return success;
}

For commonly-used error descriptions, I would define them together with err_struct, which would make it easier to be consistent with wording throughout your system, and helps keep an accidental typo in a description from breaking your code.

Karl Bielefeldt
  • 146,727
  • 38
  • 279
  • 479
  • By defining descriptions together with the struct, do you mean a separate array of strings containing descriptions for each error? Like errno/strerror? – Marco Scannadinari Aug 18 '14 at 16:59
  • Just like I have at the top of this function, just move the definition to the header file that defines `err_struct`. – Karl Bielefeldt Aug 18 '14 at 17:51