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.
LS