[SSSD] [PATCH] Do not keep growing event context

Simo Sorce simo at redhat.com
Tue Apr 23 18:02:17 UTC 2013


On Mon, 2013-04-22 at 12:18 +0200, Jakub Hrozek wrote:
> On Fri, Apr 19, 2013 at 02:14:21PM -0400, Simo Sorce wrote:
> > > > > >      /* Stop monitoring for this child */
> > > > > >      talloc_free(child_ctx);
> > > > > > +    talloc_free(imm);
> > > > > >  }
> > > > > 
> > > > > Why not simply make the imm context a child of the child_ctx it is freed
> > > > > just over here ?
> > > > > 
> > > > > Why is it made child of the event context at all ?
> > > > > 
> > > > > Simo.
> > > > 
> > > > I will check the code again but I think it was because we wanted to be
> > > > on the safe side and make sure the context is still valid even in case
> > > > of timeouts etc. so we allocated it on the event context which is always
> > > > available and long lived.
> > > > 
> > > > I'll check and change the code if appropriate.
> > > 
> > > I didn't find any reason the immediate context could outlive the
> > > child_ctx so I allocated the immediate context on the child_ctx.
> > > 
> > > But in the case of the sss_child_invoke_cb() I think it is still better to
> > > free the handler there because the caller owns the child_ctx and I don't
> > > think we should grow it over time. In the case of child_invoke_callback
> > > the child_ctx is released there, so we're good.
> > > 
> > > New patch is attached.
> > 
> > Looks good to me.
> > 
> > Simo.
> 
> Can I treat this as an Ack? :-)

If you are confident of your testing, then yes.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the sssd-devel mailing list