On Mon, 2009-09-14 at 19:57 +0200, Jakub Hrozek wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Thank you for the review, new patches attached.
On 09/13/2009 04:20 PM, Simo Sorce wrote:
On Thu, 2009-09-10 at 23:36 +0200, Jakub Hrozek wrote:
Patch 1:
- mostly good but mem hierarchy between ops_ctx and tools_ctx needs
to
be reversed
^^^ this is still not addressed, once you do 0001 can be acked.
Sorry, but I'm confused now. During the first review, you said that: "you should make tctx a child of octx so that talloc_free(octx) will be enough" The code now allocates octx first, then passes it as mem_ctx parameter to setup_db where it is used to create tctx.
this is the code I see for the function init_sss_tools()
+ octx = talloc_zero(tctx, struct ops_ctx); + if (octx == NULL) { + DEBUG(1, ("Could not allocate memory for data context\n")); + ERROR("Out of memory\n"); + ret = ENOMEM; + goto fini; + } + octx->ctx = tctx; + + *_octx = octx;
This means that the 'octx' memory context is the child of 'tctx'. Needs to be the other way around otherwise if you free octx, tctx will not be automatically freed.
Hope it is better now, see i.e:
struct user_mod_state user_mod_send() user_mod_attr_done() user_mod_rm_group_done() user_mod_add_group_done()
[..]
All operations now have a separate struct foo_state.
_send(),_done(),_recv() subrequest.
Done.
[..]
- it uses member_dn_fn() please don't do that.
Undone.
[..]
No more user_mod_cont in the current patchset
Awesome, this latest code looks exactly as I was thinking it should be. I'll test it asap.
Patch 3:
- looks sane but I'd like a second look from one of ours python
resident
experts
I see John replied to that part, I'll let you address the concerns he raised for the 3rd patch.
John's concerns should be addressed in the attached patch.
I'll let John comment on it
Simo.