On Fri, Sep 25, 2015 at 08:24:35AM +0200, Lukas Slebodnik wrote:
On (24/09/15 17:30), Pavel Reichl wrote:
On 09/24/2015 05:11 PM, Jakub Hrozek wrote:
On Thu, Sep 24, 2015 at 05:05:20PM +0200, Pavel Reichl wrote:
Hello,
please see simple patch attached.
Thanks!
Thanks, this would solve the bug, but can you also change the allocation of the structure from: req_ctx = talloc(be_req, struct ad_subdomains_req_ctx); to: req_ctx = talloc_zero(be_req, struct ad_subdomains_req_ctx);
So that we get a crash next time?
Sure, done. I just don't think we would get the crash sooner. I think this actually makes explicit initialization to NULL somewhat redundant. Would you like me to remove the explicit initialization?
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
From df57de04abb0d5871bd5d802251e761c1daa1889 Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Thu, 24 Sep 2015 11:03:12 -0400 Subject: [PATCH] AD: inicialize root_domain_attrs field
Resolves: https://fedorahosted.org/sssd/ticket/2805
src/providers/ad/ad_subdomains.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c index 8ed3dab0995f78a16f4a7df2e729ea88a39a782c..c2a6544fb7f146058acee9baca9b0cc6ee50aa3f 100644 --- a/src/providers/ad/ad_subdomains.c +++ b/src/providers/ad/ad_subdomains.c @@ -505,7 +505,7 @@ static void ad_subdomains_retrieve(struct ad_subdomains_ctx *ctx, int dp_error = DP_ERR_FATAL; int ret;
- req_ctx = talloc(be_req, struct ad_subdomains_req_ctx);
- req_ctx = talloc_zero(be_req, struct ad_subdomains_req_ctx); if (req_ctx == NULL) { ret = ENOMEM; goto done;
@@ -519,6 +519,7 @@ static void ad_subdomains_retrieve(struct ad_subdomains_ctx *ctx, req_ctx->root_id_ctx = NULL; req_ctx->root_op = NULL; req_ctx->root_domain = NULL;
- req_ctx->root_domain_attrs = NULL; req_ctx->reply_count = 0; req_ctx->reply = NULL;
There is a common agreement (at least from management side) that we want to improve code coverage. This patch is great opportunity to add regression test. We have a possibility to execute unit tests with valgrind in our CI script. So in this case cmocka unit test should work better. You can inspire in another similar test "test_ipa_subdom_server"
BTW I know there is a planned release for 1.13.1 but we are not in a hurry and it can be postponed few days until test will be ready.
We should release when we know downstream tests are all OK (except this one). The test can be added later (as long as Pavel agrees to write the patch).
The test_ipa_subdom_server test took me some time, that's why I don't think we need to block the release over a test.