On Thu, Jun 20, 2013 at 11:11:17AM +0200, Lukas Slebodnik wrote: > Rewritten patches are attached. > > LS
Two nitpicks: > +static char * get_ccache_name_by_principal(TALLOC_CTX *mem_ctx, > + krb5_context ctx, > + krb5_principal principal, > + const char *ccname) > +{ > + krb5_error_code kerr; > + krb5_ccache tmp_cc = NULL; > + char *tmp_ccname = NULL; > + char *ret_ccname = NULL; > + > + kerr = krb5_cc_set_default_name(ctx, ccname); > + if (kerr != 0) { > + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); > + return NULL; > + } > + > + kerr = krb5_cc_cache_match(ctx, principal, &tmp_cc); > + if (kerr != 0) { > + KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr); > + return NULL; > + } > + > + kerr = krb5_cc_get_full_name(ctx, tmp_cc, &tmp_ccname); > + if (kerr !=0) { > + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); > + } > + > + ret_ccname = talloc_strdup(mem_ctx, tmp_ccname); > + if (ret_ccname == NULL) { > + DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed (ENOMEM).\n")); > + } > + > +done: This done label is unused and GCC emits a warning over that. It should be used if krb5_cc_get_full_name() fails. > + if (tmp_cc != NULL) { > + kerr = krb5_cc_close(ctx, tmp_cc); > + if (kerr != 0) { > + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); > + } > + } > + krb5_free_string(ctx, tmp_ccname); > + > + return ret_ccname; > +} > + > > + kerr = krb5_unparse_name(ctx, princ, &full_name); > + if (kerr !=0) { > + KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr); > + goto done; > + } > + > + if (0 == strcmp(default_full_name, full_name)) { > + ret = true; > + } > + If you're changing the other patch, can you also add DEBUG message here saying what has been compared? > +done: > + if (default_cc != NULL) { > + kerr = krb5_cc_close(ctx, default_cc); > + if (kerr != 0) { > + KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr); > + goto done; > + } > + } > + > + /* all functions can be safely called with NULL. */ > + krb5_free_principal(ctx, default_princ); > + krb5_free_unparsed_name(ctx, default_full_name); > + krb5_free_unparsed_name(ctx, full_name); > + > + return ret; > +} So far I ran a number of tests against IPA. I should still check AD provider especially with enterprise principals to see if they still work. If they do I'll ack. But I will also file a ticket for 1.10.1 to move the checks to the backend so they are all done on one place as Sumit suggested earlier. I don't think it's necessary to delay 1.10.0 any longer for this issue, but it's the right thing to do eventually. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel