lukeh at padl.com
Fri Sep 11 02:00:29 EDT 2009
> * The kfree.c changes to krb5_free_checksum_contents and
> krb5_free_unparsed_name are unnecessary and undesirable; please revert
> them (or explain them if I am misunderstanding).
* Do not check for nullity before calling free or krb5_free functions.
(I don't think this is in the coding practices document but is what
we're moving towards.) Examples:
However, if the caller does not check for nullity, the krb5_free
functions must. Which is it to be?
> I have some other notes, which I am not going to require changes for:
> * I wasn't sure what the get_in_tkt.c changes were for. They relate
> to changing the server name for referrals in an as-req, which seems
> peripherally related to S4U at most.
Right, however this fixes a bug where the server realm was not being
rewritten when chasing referrals. It affects S4U but you probably want
it for 1.7.1 too.
> * I wasn't able to intuit what "gcvt" was supposed to stand for, even
> after you added the comment to krb5int_send_tgs.
I will try and think of something more sensible.
> * I got slightly lost in the mechglue changes, so I wasn't able to
> review those as carefully as the rest of the code.
More information about the krbdev