need codereview for krb consortium masterkey keytab stash project

Will Fiveash William.Fiveash at Sun.COM
Tue Jul 15 13:40:26 EDT 2008


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/



More information about the krbdev mailing list