1. 7

Note: I am the author of this post, and I do not endorse the use of C programming languages in this day and age, but if you absolutely must for some reason, this technique will result in massive improvements to code clarity and will reduce LOC significantly. Am submitting in reaction to Go error handling post.

  1.  

  2. 6

    Ah.

    My Pet Hobby Horse.

    Let me Ride It.

    Firstly. Programmers are vastly confused about errors. The term is hideously overloaded.

    The primary distinction should be between errors that are intrinsic to the program and those external.

    If the program is just plain wrong, and attempting to proceed would inevitably result in a crash or infinite loop, and the only remedy is another version of the program….

    Then for pity sake abort() and die as messily and abruptly as you can…. and find the problem and fix it.

    Attempting to handle such errors, or “return error codes” in the vain hope that some higher layer may be able to rewrite itself to be correct…

    Madness lies that way.

    Ok, so assuming we are not dealing with that class of errors.

    I’m quite happy with the “early return” pattern.

    ie. A sequence of checks that validate whether this operation can proceed, or returns error.

    A sanity check on that though is…..

    Does each check return a different error status?

    If not, the client isn’t going to be able to figure out what is wrong or do anything about it.

    If you aren’t returning a different error status because the client CAN’T do anything (apart from installing a new version of the program), don’t return a status, curl up and die.

    eg.

    bool_t foo()
    {
         if( checkThis() || checkThat())
             return true;
          }
    
          // Do stuff
         return false;
    }
    
    
    main()
    {
    
    
       if( foo()) {
             // Hey! He told me to fix something. Either fix this or that, I don't know which!
       }
    }
    

    So you’re telling me this thing can fail for two entirely different reasons…. and the higher level is going to fix it? Nope.

    Not going fix it, doesn’t know what to fix.

    I’ll accept this…

    void foo()
    {
         assert( !checkThis()); // Fix ya damn code
         assert( !checkThat());
    
          // Do stuff
    }
    

    Or I will accept this…

    enum {
       SUCCESS,
       THIS_IS_WRONG,
       THAT_IS WRONG
    } errorCode_t
    
    errorCode_t foo()
    {
          if( checkThis()) {
               return THIS_IS_WRONG;
           }
    
           if( checkThat()) {
               return THAT_IS_WRONG;
           }
    
          // Do stuff
    
          return SUCCESS;
    }
    

    But I will kick your butt if I see this….

    (void)foo();

    Check every damn error code. Be professional!

    or this everywhere…If you are going to die everywhere, die inside foo!

    errorCode_t err = foo();
    if( SUCCESS != err) {
        die( err);
    }
    

    Usually I see a mix of the two antipatterns above… which I truly hate.

    The only reason for a “goto fail” is to clean up resources.

    However, a resource that needs cleaning up tells you where to break it into another function….

    bool_t doStuff(...)
    {
        resource_t resource = allocate(); // Allocated before I'm sure I will succeed!
    
        if( foo()) {
           // BUG! Forgot to free resource
           return true;
        }
    
        // Do Stuff With resource
    
        freeResource( resource);
    
        return false;
    }
    

    Replace that with…..

    bool_t doStuff(...)
    {
    
        if( foo()) {
           return true;
        }
    
        { // Candidate block for extracting a "takes preallocated resource, returns void" function.
            resource_t resource = allocate(); // Allocated only when I'm sure I will succeed!
    
           // Do Stuff With resource. 
    
            freeResource( resource);
        } // Note scope boundary.
    
       return false;
    }
    

    Where it gets tricky is with nested resources….

    bool_t doStuffWithResource( resource_t resource)
    {
        if( bah( resource)) {
           return true;
        }
    
        { // Candidate block for extracting "takes preallocated resource2, returns void" function.
            resource2_t resource2 = allocate2( resource); // Allocated only when I'm sure I will succeed!
    
           // Do Stuff With resource, resource2 
    
            freeResource( resource2);
        } // Note scope boundary.
    
       return false;
    }
    
    
    
    
    bool_t doStuff(...)
    {
    
        if( foo()) {
           return true;
        }
    
        { // Candidate block for extracting "takes preallocated resource, returns bool_t" function.
            resource_t resource = allocate(); // Allocated only when I'm sure I will succeed!
    
            if( doStuffWithResource( resource) ) { // Remember rule? Returns true if requires special handling...
                freeResource( resource); // Yup, you still have to remember to free, but at least it's only one thing.
                return true;                     // The rule is keep it to one resource per level... Wouldn't it be nice to have RAII?
            }
    
            // Do more stuff with resource 
    
            freeResource( resource);
            return false;
        } // Note scope boundary.
    
       return false;
    }
    

    The next lesson is to stop overloading results and errors.

    ie. Stop writing API’s like

    // Returns < 0 on error, otherwise length.
    int foo();
    

    Usage…

    int result = foo();
    if( result < 0) {
       // Handle error.
    } else {
       // Do something with result
    }
    

    Write them like….

     // Returns true for the case that likely will require special handling.
     // length will always be set to something sane.
     bool_t foo( size_t *length);
    

    And use then

    if( foo( &length)) {
        // Do special handling
        return;
     }
    
     // Do happy case.
    
    1. 1

      I can’t tell from your post whether you are actually responding to the link or are creating your own post on error handling here.

      To be clear, the techniques described in the link enable the efficient and clean checking of all possible errors.

      We use them to great success in our application Espionage, where it is critical that all errors be checked and handled accordingly.

      1. 1

        Both.

        It’s the “All possible errors” that bother me.

        It’s the “let’s add macros until making our functions too complex stops hurting quite so much” that bothers me. The correct answer is “make the functions simpler.”

        Error codes are a plague where programmers think, “Oh, I don’t know how to handle this… so I will return an error code and someone or something greater and wiser than me can handle it.”

        And then build compendiums and taxonomies of errors…. only a very very few of which the higher layers, even assuming very very wise and diligent programmers, can actually recover from!

        So the very first thing any blog on error codes should mention is never return an error code which you can’t reasonably expect a higher layer to handle.

        If truly handling that error means “install new version of software”, die rather than return. To return an error code is to invite flaky software that “sometimes works”.

        The next core to my post is wrapping error handling “goto fail” macro’s is complexity only required because your code is too complex.

        Simplify, by the early return pattern and then you only need the “goto fail” to recover allocated resources.

        Simplify even further by decomposing into a subfunction every time you allocate an additional resource with a different recovery pattern…. and you don’t even need the “goto fail”.

        1. 2

          The next core to my post is wrapping error handling “goto fail” macro’s is complexity only required because your code is too complex.

          Simplify, by the early return pattern and then you only need the “goto fail” to recover allocated resources.

          Simplify even further by decomposing into a subfunction every time you allocate an additional resource with a different recovery pattern…. and you don’t even need the “goto fail”.

          From this, I can tell that you don’t seem to understand the post, as your comments are mostly off topic to it.

          The post is not about whether to return a bool or an error code from a function, although that is certainly a question worth asking.

          The post is about how to deal with error handling in C, regardless of how the functions are written. Quite the opposite of complexity, these techniques simplify all C error handling.

          You can’t just tell me that “your code is too complex” or that I should “decompose functions into subfunctions”. I didn’t write these functions, and their behavior cannot be changed by me, you, or anyone else (even the original authors). Like many functions, they are part of an API that is frozen and will never change.

          I need to use these functions, and handle and log errors appropriately. The post describes the best way of doing that in C that I’m aware of.

          1. 2

            You’ve basically implemented a poor man’s exception handler, which is fine.

            The point that JohnCarter seems to be making is that, if you’re writing code that requires lots of error handling it probably actually doesn’t.

            In a lot of code bases (including the one I’m plagued with at work), a developer decides that errors are the most important thing ever to return, and gleefully return them always. Usually, an error given to a calling function isn’t super useful–it probably complains, and bubbles up an error in turn. Eventually, you people up either (a) ignoring the return codes of functions or (b) handling every single obnoxious error and ballooning the codebase.

            The really obnoxious thing is when a developer merrily returns error codes without any sort of mechanism for making them human-readable. Instead, you end up poring through logs and header files until you finally find a system network error that was wrapped in two different error code systems before finally being belched up. Ugh.

            The “let it crash” philosophy of not handling errors, in languages that support it, is kinda neat.

            1. 1

              Eventually, you people up either (a) ignoring the return codes of functions or (b) handling every single obnoxious error and ballooning the codebase.

              OK, maybe that applies to code bases that use exceptions.

              The techniques described in the link do not do that. They result in the exact opposite: handled errors, and smaller codebase.

              Give them a try. If you find an issue, let me know in a comment to the post itself (I’ll receive an email notification).