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