New GSS-preauth plugin available for testing

Alejandro Perez Mendez alex at um.es
Mon Sep 3 10:27:44 EDT 2012


On 31/08/12 08:28, Luke Howard wrote:
> 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,

Hi Luke,

thanks a lot for all the hints and suggestions, they are really helpful, 
and you are right in all of them. I will update the code in the next days.

Thanks God you didn't have time to build or use the code, or analyse the 
logic, otherwise..., I cannot imagine how long this mail could have been :)

Regards,
Alejandro

> -- Luke



More information about the krbdev mailing list