Services4User review

Luke Howard lukeh at padl.com
Fri Sep 11 08:47:38 EDT 2009


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

s4u_identify_user() will fail to chase referrals without this change.

>  * I wasn't able to intuit what "gcvt" was supposed to stand for, even
> after you added the comment to krb5int_send_tgs.

Fixed.

>  * There's some mech logic in s4u_gss_glue.c (conversion between OID
> membership in a list and prerfc_mech/rfc_mech flags) which appears  
> to be
> common with acquire_cred.c and should be factored out.

Presently not fixed.

>  * Going forward, the preferred pattern for handling output parameters
> is to (1) initialize them to NULL (or something appropriate for
> non-pointer types), and (2) not touch them after that until a  
> successful
> return is guaranteed.  This facilitates static analysis tools like
> Coverity in determining the correspondence between return values and
> allocation of return pointers.

Noted.

>  * In future work, please briefly document the purpose of new internal
> functions with a comment above the function definition.

Noted.

-- Luke



More information about the krbdev mailing list