Date: Fri, 11 Jan 2013 11:43:12 +0100 From: pbrezina(a)redhat.com To: sssd-devel(a)lists.fedorahosted.org Subject: Re: [SSSD] #570-fall back to defaults from krb5.conf if the realm is not specified explicitly in sssd.conf On 11/26/2012 06:30 PM, Ariel Barria wrote: > Hi. > Apologies, I have been working in ticket #570 and I send for your > comments before the send a patch final. > I was reviewing where else needs to change and I find this places > 1 - krb5_auth.c ---> tevent_req *krb5_auth_send( > 2 - krb5_utils.c ---> char *expand_ccname_template( > 3 - ipa_dyndns.c ---> int create_nsupdate_message( > This correct? > > https://fedorahosted.org/sssd/ticket/570 > > Thanks Hi, thank you for contribution. I'm sorry this took so long. This is a good start, but I have few comments. 1. Can we put this code into a common function instead of copy and paste? Say: errno_t krb5_get_realm(TALLOC_CTX *mem_ctx, struct dp_option *opts, char **_realm); It will store chosen krb5 realm in *_realm attached to talloc context mem_ctx. 2. When creating tmp_ctx, don't forget to free it before returning from function. When using done/fail label, it should say: done: ... talloc_free(tmp_ctx); return ret; 3. Initialize krb5_ctx only when you know you're gonna need it. I.e. after if (krb5_realm == NULL) 4. Additional code that needs to be altered: ipa_common.c ipa_get_auth_options() krb5_common.c krb5_install_sigterm_handler() krb5_common.c krb5_get_simple_upn() krb5_common.c expand_ccname_template() I went through references to KRB5_REALM option. I didn't include all references, some of them seemed that it should not be changed. Can somebody check me please? 5. But in the end, we don't have to change those functions at all. We can chose the realm during provider initialization and use dp_opt_set_option() to set the option to chosen value. Cheers, Pavel. _______________________________________________ sssd-devel mailing list sssd-devel(a)lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel