Coding practices proposals

[Nico Williams]

On Fri, Mar 18, 2011 at 10:49 AM, Tom Yu <tlyu at mit.edu> wrote:
> Sam Hartman <hartmans at MIT.EDU> writes:
>> Hmm.  I'd like to propose dropping the recommendation against inner
>> scope variables and the requirement to clean up owner pointers in the
>> top-most scope in this case.
>
> I prefer to retain the recommendation of avoiding inner scope
> variables.  Functions complicated enough to need inner scope variables
> are often complicated enough that they should be split into smaller
> pieces.  There are some borderline examples that I might consider to
> be OK, such as local variables inside a small loop body, but they
> should still be dealt with carefully.

I much prefer having a single lexical scope for all owner pointer
variables in functions, with one goto label for all cleanup.  I'd even
prefer if all such variables were always initialized to NULL where
declared and then have free() called unconditionally on each of those
variables' values in the cleanup section.  Alias and non-pointer
variables should be OK to declare in inner scopes.  Whenever an
pointer ownership is transferred the old owner should be set to NULL.
This style makes it much easier to get a high degree of confidence
that there are no memory leaks and no double frees.

You might even want to have a naming convention (a common affix) for
owner pointer variables.  I'd rather there never were any pointer
aliases as structure fields -- always dup objects instead or else use
reference counting.  And if you'll use any reference counting, then
consider using macros or [inline] functions to handle the reference
get/put operations.

Also, all output arguments MUST be initialized on function entry, even
if the documentation says that output values are undefined in error
cases.  (Also, on when you know you're in the client-side I'd rather
assert() than return EINVAL-like errors when functions are used
incorrectly, but this is too difficult to do in a krb5 library, so, oh
well.)

I don't personally care for having a naming convention for
distinguishing output arguments from others as long as the types used
make it obvious which arguments are inputs and which are outputs.
E.g., if the output arguments are all of the form "type **" (or
"pointer_type *") and all input arguments have fewer '*'s in their
types, then there's no need to add an "_out" suffice to the output
arguments.  Avoid using arguments for input and output.

Nico
--