[SSSD] [PATCH] AD: inicialize root_domain_attrs field

Jakub Hrozek jhrozek at redhat.com
Fri Sep 25 08:09:55 UTC 2015


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 at lists.fedorahosted.org
> >>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >>
> 
> >From df57de04abb0d5871bd5d802251e761c1daa1889 Mon Sep 17 00:00:00 2001
> >From: Pavel Reichl <preichl at 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.


More information about the sssd-devel mailing list