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