On (23/06/15 10:51), Jakub Hrozek wrote: >On Tue, Jun 23, 2015 at 10:43:31AM +0200, Lukas Slebodnik wrote: >> On (23/06/15 10:01), Jakub Hrozek wrote: >> >On Thu, Jun 11, 2015 at 06:20:34PM +0200, Lukas Slebodnik wrote: >> >> On (11/06/15 10:44), Stephen Gallagher wrote: >> >> >On Thu, 2015-06-11 at 16:19 +0200, Lukas Slebodnik wrote: >> >> >> On (11/06/15 09:35), Stephen Gallagher wrote: >> >> >> > On Thu, 2015-06-11 at 09:19 -0400, Stephen Gallagher wrote: >> >> >> > > We're attempting to use strerror() to print the result from >> >> >> > > ad_gpo_access_check(), but that function returns an extended SSSD >> >> >> > > errno. >> >> >> > > >> >> >> > > This resulted in "Unknown Error" being printed to the logs. >> >> >> > >> >> >> > And now with the patch attached... >> >> >> >> >> >> > From 108c6e9f47560a53d2d044c0399e66528615d73d Mon Sep 17 00:00:00 >> >> >> > 2001 >> >> >> > From: Stephen Gallagher <sgall...@redhat.com> >> >> >> > Date: Thu, 11 Jun 2015 09:17:02 -0400 >> >> >> > Subject: [PATCH] GPO: Fix incorrect strerror on GPO access denial >> >> >> > >> >> >> > We're attempting to use strerror() to print the result from >> >> >> > ad_gpo_access_check(), but that function returns an extended SSSD >> >> >> > errno >> >> >> > --- >> >> >> > src/providers/ad/ad_gpo.c | 2 +- >> >> >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> > >> >> >> > diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c >> >> >> > index >> >> >> > 4e7afb12a73313ece00c6ffdd24cfad1b73a831d..995bd083b27710814350bef8d1e43b874d5a8a27 >> >> >> > >> >> >> > 100644 >> >> >> > --- a/src/providers/ad/ad_gpo.c >> >> >> > +++ b/src/providers/ad/ad_gpo.c >> >> >> > @@ -1446,11 +1446,11 @@ ad_gpo_perform_hbac_processing(TALLOC_CTX >> >> >> > *mem_ctx, >> >> >> > deny_size); >> >> >> > >> >> >> > if (ret != EOK) { >> >> >> > DEBUG(SSSDBG_OP_FAILURE, >> >> >> > "GPO access check failed: [%d](%s)\n", >> >> >> > - ret, strerror(ret)); >> >> >> > + ret, sss_strerror(ret)); >> >> >> > goto done; >> >> >> > } >> >> >> >> >> >> You can also replace strerror on line 1152 after >> >> >> ad_gpo_extract_policy_setting >> >> >> 1178 after >> >> >> ad_gpo_extract_policy_setting >> >> >> 1673 after >> >> >> sdap_id_op_connect_recv >> >> >> 1995 after >> >> >> ad_gpo_filter_gpos_by_cse_guid >> >> >> 2044 after >> >> >> sysdb_gpo_delete_gpo_result_object >> >> >> 2136 after >> >> >> sysdb_gpo_get_gpo_by_guid >> >> >> 4037 after >> >> >> ad_gpo_parse_gpo_child_response >> >> >> >> >> >> But strerror can be replaced on all places with sss_strerror >> >> >> because in value is not within sssd error code range then strerror is >> >> >> called. >> >> > >> >> > >> >> >To be clear, is this a Nack? I was really only trying to address a >> >> >specific message that was proving to be misleading. If you really want >> >> >me to replace all instances of strerror(), that's a bigger effort. >> >> I didn't want to replace on all places. Michal has patch for this IIRC. >> >> just in the src/providers/ad/ad_gpo.c. I've already provided lines :-) >> >> >> >> Or should we push 6 one-liners :-) (without ACK) >> >> >> >> LS >> > >> >I think we can push this one since it's already written, then continue >> >with Michal's patch (read=ACK) >> > >> Michal has different use case. >> He want t use sss_strerror on all places and fail with >> compile time error incase of using "strerror" >> >> >btw what is blocking the mass-conversion to sss_strerror? >> Nothing. >> Michal just need to finish his job. >> >> If Stephen does not have a time I can update patch. >> I provided exact lines which need to be changed. > >Sure, if you have time, fix the other occurences in GPO atop Stephen's >patch and I'll push both. The commit message would be the same for both patches. I just updated Stephen's patch and added my signoff.
LS
>From 06c622365f83fbc8b3a541a54e26c51606f60e81 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Thu, 11 Jun 2015 09:17:02 -0400 Subject: [PATCH] GPO: Fix incorrect strerror on GPO access denial We're attempting to use strerror() to print the result from ad_gpo_access_check(), but that function returns an extended SSSD errno Signed-off-by: Lukas Slebodnik <lsleb...@redhat.com> --- src/providers/ad/ad_gpo.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c index 4e7afb12a73313ece00c6ffdd24cfad1b73a831d..974fd04b99709055f25ed2a3b77821b3caec09ad 100644 --- a/src/providers/ad/ad_gpo.c +++ b/src/providers/ad/ad_gpo.c @@ -1149,7 +1149,7 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain, if (ret != EOK && ret != ENOENT) { DEBUG(SSSDBG_CRIT_FAILURE, "ad_gpo_extract_policy_setting failed for %s [%d][%s]\n", - allow_key, ret, strerror(ret)); + allow_key, ret, sss_strerror(ret)); goto done; } else if (ret != ENOENT) { ret = sysdb_gpo_store_gpo_result_setting(domain, @@ -1175,7 +1175,7 @@ ad_gpo_store_policy_settings(struct sss_domain_info *domain, if (ret != EOK && ret != ENOENT) { DEBUG(SSSDBG_CRIT_FAILURE, "ad_gpo_extract_policy_setting failed for %s [%d][%s]\n", - deny_key, ret, strerror(ret)); + deny_key, ret, sss_strerror(ret)); goto done; } else if (ret != ENOENT) { ret = sysdb_gpo_store_gpo_result_setting(domain, @@ -1448,7 +1448,7 @@ ad_gpo_perform_hbac_processing(TALLOC_CTX *mem_ctx, if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "GPO access check failed: [%d](%s)\n", - ret, strerror(ret)); + ret, sss_strerror(ret)); goto done; } @@ -1670,7 +1670,7 @@ ad_gpo_connect_done(struct tevent_req *subreq) if (dp_error != DP_ERR_OFFLINE) { DEBUG(SSSDBG_OP_FAILURE, "Failed to connect to AD server: [%d](%s)\n", - ret, strerror(ret)); + ret, sss_strerror(ret)); goto done; } else { DEBUG(SSSDBG_TRACE_FUNC, "Preparing for offline operation.\n"); @@ -1992,7 +1992,7 @@ ad_gpo_process_gpo_done(struct tevent_req *subreq) if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Unable to filter GPO list by CSE_GUID: [%d](%s)\n", - ret, strerror(ret)); + ret, sss_strerror(ret)); goto done; } @@ -2041,7 +2041,7 @@ ad_gpo_process_gpo_done(struct tevent_req *subreq) default: DEBUG(SSSDBG_FATAL_FAILURE, "Could not delete GPO Result from cache: [%s]\n", - strerror(ret)); + sss_strerror(ret)); goto done; } } @@ -2133,7 +2133,7 @@ ad_gpo_cse_step(struct tevent_req *req) cached_gpt_version = -1; } else { DEBUG(SSSDBG_FATAL_FAILURE, "Could not read GPO from cache: [%s]\n", - strerror(ret)); + sss_strerror(ret)); return ret; } @@ -4034,7 +4034,7 @@ static void gpo_cse_done(struct tevent_req *subreq) if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "ad_gpo_parse_gpo_child_response failed: [%d][%s]\n", - ret, strerror(ret)); + ret, sss_strerror(ret)); tevent_req_error(req, ret); return; } else if (child_result != 0){ -- 2.4.3
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel