Hi,
another trivial patch which should speed up user support in the future. Currently we have no idea which part of create_ccache() failed..
On Mon, Feb 23, 2015 at 01:33:43PM +0100, Jakub Hrozek wrote:
Hi,
another trivial patch which should speed up user support in the future. Currently we have no idea which part of create_ccache() failed..
CI link: http://sssd-ci.duckdns.org/logs/job/7/81/summary.html
On 02/23/2015 01:33 PM, Jakub Hrozek wrote:
kerr = handle_randomized(ccname);
- if (kerr) goto done;
- if (kerr != 0) {
DEBUG(SSSDBG_CRIT_FAILURE,"Cannot create randomized ccache name: %d\n", kerr);goto done;- }
Hello, why didn't you call here KRB5_CHILD_DEBUG() as you do at all other places? Thanks!
On Mon, Feb 23, 2015 at 01:45:51PM +0100, Pavel Reichl wrote:
On 02/23/2015 01:33 PM, Jakub Hrozek wrote:
kerr = handle_randomized(ccname);
- if (kerr) goto done;
- if (kerr != 0) {
DEBUG(SSSDBG_CRIT_FAILURE,"Cannot create randomized ccache name: %d\n", kerr);goto done;- }
Hello, why didn't you call here KRB5_CHILD_DEBUG() as you do at all other places? Thanks!
Because handle_randomized() returns errno codes, not krb5 codes, so I assume KRB5_CHILD_DEBUG() would print something like "Unknown error" anyway.
On 02/23/2015 01:51 PM, Jakub Hrozek wrote:
On Mon, Feb 23, 2015 at 01:45:51PM +0100, Pavel Reichl wrote:
On 02/23/2015 01:33 PM, Jakub Hrozek wrote:
kerr = handle_randomized(ccname);
- if (kerr) goto done;
- if (kerr != 0) {
DEBUG(SSSDBG_CRIT_FAILURE,"Cannot create randomized ccache name: %d\n", kerr);goto done;- }
Hello, why didn't you call here KRB5_CHILD_DEBUG() as you do at all other places? Thanks!
Because handle_randomized() returns errno codes, not krb5 codes, so I assume KRB5_CHILD_DEBUG() would print something like "Unknown error" anyway. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Oh, I see now. Thanks. Maybe kerr is not the right variable to check return code then. But I don't think its a big problem.
On 02/23/2015 01:33 PM, Jakub Hrozek wrote:
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c index 8b3f10d8244f483e6f99a79b01964b0018fa3ee4..174597ad9f2b63f1c9981c0ecd0c203933900a6c 100644 --- a/src/providers/krb5/krb5_child.c +++ b/src/providers/krb5/krb5_child.c @@ -586,16 +586,25 @@ static krb5_error_code create_ccache(char *ccname, krb5_creds *creds) * opened as root and contain possibly references (even open handles ?) * to resources we do not have or do not want to have access to */ kerr = krb5_init_context(&kctx);
- if (kerr) {
if (kerr != 0) {
DEBUG(SSSDBG_CRIT_FAILURE, "Cannot initialize krb5 context\n"); KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr); return ERR_INTERNAL; } kerr = handle_randomized(ccname);
- if (kerr) goto done;
if (kerr != 0) {
DEBUG(SSSDBG_CRIT_FAILURE,"Cannot create randomized ccache name: %d\n", kerr);goto done;}
kerr = krb5_cc_resolve(kctx, ccname, &kcc);
- if (kerr) goto done;
if (kerr != 0) {
DEBUG(SSSDBG_CRIT_FAILURE, "Cannot resolve ccname\n");KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);goto done;}
type = krb5_cc_get_type(kctx, kcc); DEBUG(SSSDBG_TRACE_ALL, "Initializing ccache of type [%s]\n", type);
@@ -603,29 +612,54 @@ static krb5_error_code create_ccache(char *ccname, krb5_creds *creds) #ifdef HAVE_KRB5_CC_COLLECTION if (krb5_cc_support_switch(kctx, type)) { kerr = krb5_cc_set_default_name(kctx, ccname);
if (kerr) goto done;
if (kerr != 0) {DEBUG(SSSDBG_CRIT_FAILURE, "krb5_cc_set_default_name failed\n");KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);goto done;}
2 empty lines
kerr = krb5_cc_cache_match(kctx, creds->client, &cckcc); if (kerr == KRB5_CC_NOTFOUND) { kerr = krb5_cc_new_unique(kctx, type, NULL, &cckcc); switch_to_cc = true; }
if (kerr) goto done;
if (kerr != 0) {DEBUG(SSSDBG_CRIT_FAILURE, "krb5_cc_cache_match failed\n");KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);goto done;}krb5_cc_close(kctx, kcc); kcc = cckcc; }#endif
kerr = krb5_cc_initialize(kctx, kcc, creds->client);
- if (kerr) goto done;
- if (kerr != 0) {
DEBUG(SSSDBG_CRIT_FAILURE, "Cannot initialize ccname\n");KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);goto done;- }
2 empty lines
kerr = krb5_cc_store_cred(kctx, kcc, creds);
- if (kerr) goto done;
- if (kerr != 0) {
DEBUG(SSSDBG_CRIT_FAILURE, "Cannot store creds in ccname\n");KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);goto done;- }
2 empty lines
#ifdef HAVE_KRB5_CC_COLLECTION if (switch_to_cc) { kerr = krb5_cc_switch(kctx, kcc);
if (kerr) goto done;
if (kerr != 0) {DEBUG(SSSDBG_CRIT_FAILURE, "Cannot switch to ccache\n");KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);goto done;}}-- 2.1.0
Your patch adds at 3 places another empty line. Is there a reason for that? I suppose it's just a typo...
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..
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@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.
On 02/23/2015 05:54 PM, Pavel Reichl wrote:
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@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.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Might be it's hard to understand what I proposed to be changed, should I prepare the patch myself?
On Mon, Feb 23, 2015 at 05:54:59PM +0100, Pavel Reichl wrote:
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@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(
I did this.
) in create_ccache() after 'done:' to save pasting it explicitly into 6 if statement.
I didn't do this, because KRB5_CHILD_DEBUG output contains the line number and moving the statement to DEBUG would make it useless.
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.
I explicitly print the error value in handle_randomized now, but I don't think we need to do any converstion, it's perfectly safe to mix krb5 error codes and errno code, they have a different base.
On 03/09/2015 08:32 PM, Jakub Hrozek wrote:
From 57d71ec270286c1eb5e284d866b1aa04bbd8b772 Mon Sep 17 00:00:00 2001 From: Jakub Hrozekjhrozek@redhat.com Date: Tue, 11 Nov 2014 14:04:30 +0100 Subject: [PATCH] KRB5: More debugging for create_ccache()
It was difficult to find where the problem was without advanced techniques like strace.
src/providers/krb5/krb5_child.c | 53 +++++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 13 deletions(-)
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c index 8b3f10d8244f483e6f99a79b01964b0018fa3ee4..10b4e8c948ff9a394910a0f7b7006950963e7ec3 100644 --- a/src/providers/krb5/krb5_child.c +++ b/src/providers/krb5/krb5_child.c @@ -556,7 +556,8 @@ static errno_t handle_randomized(char *in) umask(old_umask); if (fd == -1) { ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE, "mkstemp(\"%s\") failed!\n", ccname);
DEBUG(SSSDBG_CRIT_FAILURE,"mkstemp(\"%s\") failed: %d!\n", ccname, ret); return ret; } }@@ -592,43 +593,73 @@ static krb5_error_code create_ccache(char *ccname, krb5_creds *creds) }
kerr = handle_randomized(ccname);
- if (kerr) goto done;
if (kerr) {
DEBUG(SSSDBG_CRIT_FAILURE, "handle_randomized failed: %d\n", kerr);goto done;}
kerr = krb5_cc_resolve(kctx, ccname, &kcc);
- if (kerr) goto done;
if (kerr) {
KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);goto done;}
type = krb5_cc_get_type(kctx, kcc); DEBUG(SSSDBG_TRACE_ALL, "Initializing ccache of type [%s]\n", type);
#ifdef HAVE_KRB5_CC_COLLECTION if (krb5_cc_support_switch(kctx, type)) {
DEBUG(SSSDBG_TRACE_ALL, "CC supports switch\n"); kerr = krb5_cc_set_default_name(kctx, ccname);
if (kerr) goto done;
if (kerr) {DEBUG(SSSDBG_TRACE_ALL, "Cannot set default name!\n");KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);goto done;} kerr = krb5_cc_cache_match(kctx, creds->client, &cckcc); if (kerr == KRB5_CC_NOTFOUND) {DEBUG(SSSDBG_TRACE_ALL, "Match not found\n"); kerr = krb5_cc_new_unique(kctx, type, NULL, &cckcc); switch_to_cc = true; }
if (kerr) goto done;
if (kerr) {DEBUG(SSSDBG_TRACE_ALL, "krb5_cc_cache_match failed\n");KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);goto done;} krb5_cc_close(kctx, kcc); kcc = cckcc; }#endif
kerr = krb5_cc_initialize(kctx, kcc, creds->client);
- if (kerr) goto done;
if (kerr) {
DEBUG(SSSDBG_TRACE_ALL, "krb5_cc_initialize failed\n");KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);goto done;}
kerr = krb5_cc_store_cred(kctx, kcc, creds);
- if (kerr) goto done;
- if (kerr) {
DEBUG(SSSDBG_TRACE_ALL, "krb5_cc_store_cred failed");
missing '\n'
KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);goto done;}
#ifdef HAVE_KRB5_CC_COLLECTION if (switch_to_cc) {
DEBUG(SSSDBG_TRACE_ALL, "switch_to_cc");
missing '\n'
kerr = krb5_cc_switch(kctx, kcc);
if (kerr) goto done;
if (kerr) {DEBUG(SSSDBG_TRACE_ALL, "krb5_cc_switch\n");KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);goto done;} }#endif
DEBUG(SSSDBG_TRACE_ALL, "returning: %d\n", kerr); done: if (kcc) { /* FIXME: should we krb5_cc_destroy in case of error ? */
@@ -955,7 +986,6 @@ 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); goto done; } kerr = 0;@@ -1026,7 +1056,6 @@ 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); goto done; }@@ -1490,9 +1519,7 @@ static errno_t create_empty_ccache(struct krb5_req *kr) kerr = 0; }
- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);- } else {
- if (kerr == 0) { kerr = k5c_attach_ccname_msg(kr); }
--
Thanks, ci passed:
http://sssd-ci.duckdns.org/logs/job/9/23/summary.html
Would you add 2 missing '\n' before pushing the patch?
ACK
On (09/03/15 20:32), Jakub Hrozek wrote:
On Mon, Feb 23, 2015 at 05:54:59PM +0100, Pavel Reichl wrote:
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..
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(
I did this.
) in create_ccache() after 'done:' to save pasting it explicitly into 6 if statement.
I didn't do this, because KRB5_CHILD_DEBUG output contains the line number and moving the statement to DEBUG would make it useless.
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.
I explicitly print the error value in handle_randomized now, but I don't think we need to do any converstion, it's perfectly safe to mix krb5 error codes and errno code, they have a different base.
From 57d71ec270286c1eb5e284d866b1aa04bbd8b772 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek jhrozek@redhat.com Date: Tue, 11 Nov 2014 14:04:30 +0100 Subject: [PATCH] KRB5: More debugging for create_ccache()
It was difficult to find where the problem was without advanced techniques like strace.
src/providers/krb5/krb5_child.c | 53 +++++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 13 deletions(-)
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c index 8b3f10d8244f483e6f99a79b01964b0018fa3ee4..10b4e8c948ff9a394910a0f7b7006950963e7ec3 100644 --- a/src/providers/krb5/krb5_child.c +++ b/src/providers/krb5/krb5_child.c @@ -556,7 +556,8 @@ static errno_t handle_randomized(char *in) umask(old_umask); if (fd == -1) { ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE, "mkstemp(\"%s\") failed!\n", ccname);
DEBUG(SSSDBG_CRIT_FAILURE,"mkstemp(\"%s\") failed: %d!\n", ccname, ret);
Please add strerror as well. It's easier to see string representation rather than grep header files and transfor number to error code.
LS
On 03/10/2015 11:28 AM, Jakub Hrozek wrote:
On Tue, Mar 10, 2015 at 10:34:23AM +0100, Lukas Slebodnik wrote:
Please add strerror as well. It's easier to see string representation rather than grep header files and transfor number to error code.
LS
strerror and newlines added.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Thanks, ACK
On Tue, Mar 10, 2015 at 01:14:01PM +0100, Pavel Reichl wrote:
On 03/10/2015 11:28 AM, Jakub Hrozek wrote:
On Tue, Mar 10, 2015 at 10:34:23AM +0100, Lukas Slebodnik wrote:
Please add strerror as well. It's easier to see string representation rather than grep header files and transfor number to error code.
LS
strerror and newlines added.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Thanks, ACK
* master: 429d51ec39193dd1681cef2c8eafd74595142a48
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Lukas Slebodnik -
Pavel Reichl