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 _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
