For review: GSS memory allocation patches

Greg Hudson ghudson at MIT.EDU
Thu Oct 13 12:33:13 EDT 2011


I found some minor stuff, but nothing serious.

On 10/12/2011 07:43 PM, Kevin Wasserman wrote:
> Simplify gss_indicate_mechs() by using generic_gss_copy_oid_set
> https://github.com/hartmans/kfw-updates/commit/5b8ce7277fbe87278155a69e30ffe96f14dd155e

This looks fine.

> Add new public header gssapi_alloc.h (added gssalloc_calloc; fixed to use 
> strlcpy instead of strcpy)
> https://github.com/hartmans/kfw-updates/commit/091b140c2005e178de4d3d26afab829a39cad405

This appears to modify util/gss-kernel-lib/deps by hand to include
dependencies on gssapi_alloc.h, even though it doesn't create any actual
dependencies on that header from any other source files or headers.

gssapi_alloc.h has an uncuddled left brace on line 18.

> Utility functions to move allocations from k5buf/krb5_data to gss_buffer_t
> https://github.com/hartmans/kfw-updates/commit/f5372211c124317ee7824702d002f6b5f68149c5

gssapiP_generic.h has an uncuddled left brace on line 283.

What is the purpose of the change to the generic_gss_copy_oid_set
prototype at line 320?  As far as I know, adding a "const" there is
meaningless since it just makes the parameter variable read-only.

> Use gssalloc memory management where appropriate
> https://github.com/hartmans/kfw-updates/commit/b17c2e27aec70e2edeeea3d28c89f070b681d2ae

Un-cuddled else at gss-server.c line 528.  The memory management there
looks more reasonable, though.

> Add "-dce" commandline option to gss-client.c to set GSS_C_DCE_STYLE flag
> https://github.com/hartmans/kfw-updates/commit/42a6344bb0074cf939b71f3fcac4dc49403af515 

In README, it is inconsistent to have such a mechanistic description of
what -dce does.  Say it in English, e.g. "Tells the client to request
DCE-style authentication".



More information about the krbdev mailing list