need codereview for krb consortium masterkey keytab stash project

[Will Fiveash]

I've committed the code changes based on your review to the mkey_keytab
branch.

On Tue, Jul 15, 2008 at 12:40:26PM -0500, Will Fiveash wrote:
> On Mon, Jul 14, 2008 at 04:39:24PM -0400, Ken Raeburn wrote:
> > Mostly small stuff...
> >
> > The DAL API is changed, but I think that's still an internal-only interface 
> > so not doing version checks is okay.
> 
> That's what I was assuming.
> 
> > KADM5_CONFIG_KVNO will need to be reassigned when merging; the iprop merge 
> > used up that number.  That'll be trivial.
> 
> Okay.
> 
> > It may be my viewer, but some of the indentation in dump.c doesn't seem to 
> > match MIT's code style (indenting function arguments on continuation lines 
> > to line up with the first argument).
> >
> > Function definitions shouldn't have spaces between the parens and the 
> > arguments.
> 
> The problem I have is trying to decide what is correct.  If you look at
> the definition of kdb_def_set_mkey() (which I did not modify) in
> kdb_default.c you'll see a space between the parens and the args.
> I've gone ahead and modified the formatting to remove the space between
> the parens and the args for the functions I modified.
> 
> In general I was told by Tom that he preferred a C-style where
> indentation was 4 columns and spaces are used instead of hard tabs.
> This was what I was using however I understand your point about not
> introducing unnecessary diffs so I've gone back and fixed the obvious
> places where the formatting was different.
> 
> > kdb_default.c: On my Solaris system, mktemp(3C) is documented as wanting a 
> > string with six trailing Xs; your template string has five.  (The Mac 
> > version says it'll take any number, but I've seen six mentioned elsewhere.)
> 
> Fixed this to use six X's.
> 
> > The new keytab code should be exercised by the test suite; do you have a 
> > test to verify that an old-format stash file will still work?
> 
> Not yet but I will.  Can this be postponed?
> 
> -- 
> Will Fiveash
> Sun Microsystems Inc.
> http://opensolaris.org/os/project/kerberos/

-- 
Will Fiveash
Sun Microsystems Inc.
http://opensolaris.org/os/project/kerberos/