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