On Tue, Jun 23, 2015 at 01:16:30PM +0200, Lukas Slebodnik wrote:
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 <sgallagh(a)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
ACK thanks for taking care of it.
CI:
http://sssd-ci.duckdns.org/logs/job/18/13/summary.html