On 02/23/2015 02:54 PM, Jakub Hrozek wrote:
On Mon, Feb 23, 2015 at 01:51:36PM +0100, Pavel Reichl wrote:
> Your patch adds at 3 places another empty line. Is there a reason for that?
> I suppose it's just a typo...
Yes, it's a copy-n-paste error, thanks for catching that. Attached is a
new patch.
btw please note that every KRB5_DEBUG call also means there's a
sss_log() call that emits to syslog, I hope that's acceptable as well
since those errors are fatal..
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel I am Sorry I didn't
notice it before but adding KRB5_CHILD_DEBUG() might
be duplicit as It's called every time the create_ccache() call fails -
as you can see at attached snippet.
static krb5_error_code get_and_save_tgt_with_keytab(krb5_context
ctx,
--
}
/* Use the updated principal in the creds in case canonicalized */
kerr = create_ccache(ccname, &creds);
if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);
Here.
goto done;
}
kerr = 0;
done:
krb5_free_cred_contents(ctx, &creds);
return kerr;
--
static krb5_error_code get_and_save_tgt(struct krb5_req *kr,
--
}
/* Use the updated principal in the creds in case canonicalized */
kerr = create_ccache(cc_name, kr->creds);
if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);
Here.
goto done;
}
/* Successfull authentication! Check if ccache contains the
* right principal...
*/
kerr = sss_krb5_check_ccache_princ(kr->ctx, kr->ccname,
kr->creds->client);
if (kerr) {
--
static errno_t create_empty_ccache(struct krb5_req *kr)
--
DEBUG(SSSDBG_TRACE_LIBS, "Creating empty ccache\n");
kerr = create_empty_cred(kr->ctx, kr->princ, &creds);
if (kerr == 0) {
kerr = create_ccache(kr->ccname, creds);
}
} else {
DEBUG(SSSDBG_TRACE_LIBS, "Existing ccache still valid,
reusing\n");
kerr = 0;
}
if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);
Here.
} else {
kerr = k5c_attach_ccname_msg(kr);
(END)
If I may propose solution I would remove KRB5_CHILD_DEBUG() from the
code pasted above and I would move KRB5_CHILD_DEBUG() in create_ccache()
after 'done:' to save pasting it explicitly into 6 if statement.
kerr = handle_randomized(ccname);
if (kerr != 0) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Cannot create randomized ccache name: %d\n", kerr);
goto done;
}
The only if statement that doesn't contain in KRB5_CHILD_DEBUG() can
return directly, which would be IMO good thing as it returns errno value
but function create_ccache() is supposed to return krb5_error_code, so I
think that explicit conversion would be nice.
Thanks for consideration.