New GSS-preauth plugin available for testing

Luke Howard lukeh at padl.com
Fri Aug 31 03:28:16 EDT 2012


Hi Alejandro,

It's fantastic to see this plugin developed.

I haven't attempted to build or use the code, or analyse the logic, but here are a few comments on style that might help with eventual upstream integration.

* LGPL may not be a compatible license if your intention is to have it distributed alongside MIT
* s/cledentials/credentials/
* requested_flags might more conventionally be a macro rather than a static variable (renamed to suit)
* might be cleaner to use an unnamed struct or make gsspreauth_req_context_t more similar to gssapi_req_context_t
* for use_default_gss_cred/federated, Greg would probably prefer an int (I've been picked up for using bitfields before)
* use k5alloc() instead of malloc
* I don't know if this is strictly required by the coding guidelines, but (say in client_plugin_fini), I believe the convention is to have a space on either side of the assignment operator, and a space before braces (unless they're on a line by themselves)
* there are a bunch of lines with trailing spaces that "git diff" flags
* in client_gic_opt(), may be nicer to use braces or eliminate newlines between branches
* check the return value of malloc() (and use k5alloc()). eg. get_name_from_gss_credential(), create_first_request()
* don't use magic numbers (e.g. 200 bytes for prf_in), define a constant macro somewhere
* I would prefer initialising maj_stat to GSS_C_COMPLETE rather than 0
* perhaps use krb5_principal_compare() rather than strcmp()

Once again, great work.

regards,

-- Luke


More information about the krbdev mailing list