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.
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...
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@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.
LS
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@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.
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@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
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@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)
btw what is blocking the mass-conversion to sss_strerror?
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@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.
LS
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@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.
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@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
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@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
On Tue, Jun 23, 2015 at 02:41:53PM +0200, Jakub Hrozek wrote:
ACK thanks for taking care of it. CI: http://sssd-ci.duckdns.org/logs/job/18/13/summary.html
* master: b08b6a994dcca108bb571458da092e0e320ce9c2
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Lukas Slebodnik -
Stephen Gallagher