Services4User review

Luke Howard 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).

You said:

* 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  
> only
> 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.

Nico?

cheers,

-- Luke



More information about the krbdev mailing list