Exception handling and resource cleanup

[ghudson@MIT.EDU]

We have at least five different patterns for handling exceptions in
Kerberos right now (example snippets at the end):

  1. goto cleanup
  2. if ladders (e.g. plugins/preauth/encrypted_challenge)
  3. Nested if statements (e.g. rule_an_to_ln)
  4. Free everything and return immediately (e.g. krb5_parse_name)
  5. CLEANUP_PUSH/CLEANUP_DONE (used exclusively in mk_safe and friends)

This is not the highest-priority issue facing us, but for the sake of
consistency in new code I would like to resolve:

* How much consistency do we want here, and
* Which style is best if we do want consistency
* Which styles should be ruled out if we don't want consistency

I'm talking specifically about medium-length or long functions here; I
don't think anyone wants to add boilerplate to short functions which
can't fail or can only fail in one place.

Personally, I would like to see consistency around (1) or consistency
around (2).  I can train myself to recognize either style, but
shifting between them is a bit jarring (sort of like shifting between
GNU-style and BSD-style code).  (3) has a certain LISP-like elegance,
but doesn't interact well with our 79-column limit and our long
function names.  (4) is responsible for a lot of memory leaks in our
current code base, duplicates a lot of code, and puts big blocks of
exception-handling code in the middle of the main logic.  I don't
think (5) should be seriously considered because it overuses the
preprocessor and requires the programmer to count the highest number
of resources which might need to be cleaned up.

As for (1) vs. (2): we currently use (1) a lot, but Sam has recently
added a bunch of code using (2) with his FAST work.  The main caveats
are that (1) provokes many people's visceral dislike of goto, and (2)
pushes the main logic of the function one indentation level to the
right (plus another level inside of loops, if the function has them).

Opinions appreciated.  Using a language with garbage collection and
exception handling is not on the table. :)

----------------
Example snippets
----------------

These are intentionally simplified; real-life functions will be a bit
more complicated due to either (a) loops, or (b) return-value
allocations which should be preserved on success but cleaned up on
failure.

1. goto cleanup

  retval = f(...);
  if (retval)
    goto cleanup;
  .
  .
  retval = f(...);
  if (retval)
    goto cleanup;
  .
  .
cleanup:
  free(stuff);
  return retval;

2. if ladders

  retval = f(...);
  if (retval == 0) {
    .
    .
    retval = f(...);
  }
  if (retval == 0) {
    .
    .
    retval = f(...);
  }
  free(stuff);
  return retval;

3. Nested if statements

  retval = f(...);
  if (retval == 0) {
    .
    .
    retval = f(...);
    if (retval == 0) {
      .
      .
    }
    .
    .
    free(stuff);
  }
  return retval;

4. Free everything and return immediately:

  retval = f(...);
  if (retval) {
    free(a);
    free(b);
    return retval;
  }
  .
  .
  retval = f(...);
  if (retval) {
    free(a);
    free(b);
    free(c);
    return retval;
  }
  .
  .
  return 0;

5. CLEANUP_PUSH/CLEANUP_DONE

  {
    CLEANUP_INIT(2); /* 2 is the largest number of cleanups possible */
    .
    .
    CLEANUP_PUSH(stuff, free);
    retval = f(...);
    if (retval) {
      CLEANUP_DONE();
      return retval;
    }
    CLEANUP_PUSH(otherstuff, krb5_free_stuff);
    .
    .
    if (retval) {
      CLEANUP_DONE();
      return retval;
    }
    CLEANUP_DONE();
    return 0;
  }