[16704] in Kerberos_V5_Development
Re: Coding practices proposals
daemon@ATHENA.MIT.EDU (Nico Williams)
Fri Mar 18 14:59:44 2011
MIME-Version: 1.0
In-Reply-To: <ldvlj0c77j0.fsf@cathode-dark-space.mit.edu>
Date: Fri, 18 Mar 2011 13:59:40 -0500
Message-ID: <AANLkTikG0nnwDfqwoxQYYUAWQeOBEi3Pb8s752tFS_+T@mail.gmail.com>
From: Nico Williams <nico@cryptonector.com>
To: Tom Yu <tlyu@mit.edu>
Cc: Sam Hartman <hartmans@mit.edu>, "krbdev@mit.edu" <krbdev@mit.edu>
Content-Type: text/plain; charset="utf-8"
Errors-To: krbdev-bounces@mit.edu
Content-Transfer-Encoding: 8bit
On Fri, Mar 18, 2011 at 10:49 AM, Tom Yu <tlyu@mit.edu> wrote:> Sam Hartman <hartmans@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 pointervariables in functions, with one goto label for all cleanup. I'd evenprefer if all such variables were always initialized to NULL wheredeclared and then have free() called unconditionally on each of thosevariables' values in the cleanup section. Alias and non-pointervariables should be OK to declare in inner scopes. Whenever anpointer ownership is transferred the old owner should be set to NULL.This style makes it much easier to get a high degree of confidencethat there are no memory leaks and no double frees.
You might even want to have a naming convention (a common affix) forowner pointer variables. I'd rather there never were any pointeraliases as structure fields -- always dup objects instead or else usereference counting. And if you'll use any reference counting, thenconsider using macros or [inline] functions to handle the referenceget/put operations.
Also, all output arguments MUST be initialized on function entry, evenif the documentation says that output values are undefined in errorcases. (Also, on when you know you're in the client-side I'd ratherassert() than return EINVAL-like errors when functions are usedincorrectly, but this is too difficult to do in a krb5 library, so, ohwell.)
I don't personally care for having a naming convention fordistinguishing output arguments from others as long as the types usedmake 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 theirtypes, then there's no need to add an "_out" suffice to the outputarguments. Avoid using arguments for input and output.
Nico--
_______________________________________________krbdev mailing list krbdev@mit.eduhttps://mailman.mit.edu/mailman/listinfo/krbdev
