[PATCH] Remove cached event context from irpc and imessaging

Andrew Bartlett abartlet at samba.org
Mon May 12 14:34:09 MDT 2014


On Mon, 2014-05-12 at 12:45 +0200, Stefan (metze) Metzmacher wrote:
> Am 12.05.2014 02:27, schrieb Andrew Bartlett:
> > On Wed, 2014-05-07 at 22:35 +0200, Stefan (metze) Metzmacher wrote:
> >> Am 07.05.2014 05:37, schrieb Andrew Bartlett:
> >>> On Tue, 2014-05-06 at 13:13 +1200, Andrew Bartlett wrote:
> >>>> On Tue, 2014-05-06 at 01:14 +0200, Stefan (metze) Metzmacher wrote:
> >>>>> Am 06.05.2014 01:05, schrieb Andrew Bartlett:
> >>>>>> On Mon, 2014-05-05 at 09:49 +0200, Stefan (metze) Metzmacher wrote:
> >>>>>>> Am 05.05.2014 07:16, schrieb Andrew Bartlett:
> >>>>>>>> The only part of this code with a stored event context is now the
> >>>>>>>> binding_handle created by irpc_binding_handle() in the client, and
> >>>>>>>> only if specified (otherwise a new nested event context is created).
> >>>>>>>
> >>>>>>> As far as I can see the "only if specified" case is not done in the commit
> >>>>>>> and dcerpc_binding_handle_set_sync_ev() is still always called.
> >>>>>>
> >>>>>> It already did that internally (if you pass in NULL). 
> >>>>>
> >>>>> But we never pass NULL, so I think it's confusing in this commit message.
> >>>>>
> >>>>>>> What about moving dcerpc_binding_handle_set_sync_ev() into the callers?
> >>>>>>
> >>>>>> A very reasonable suggestion for a follow-up patch.  I'll see what I can
> >>>>>> do.
> >>>>>
> >>>>> Thanks! I guess squashing this would make the patch simpler
> >>>>> as we don't have to add the 'tevent_context' argument and then remove it
> >>>>> again.
> >>>>
> >>>> I agree.  See attached, please review/push.  (I'm glad I prepared it in
> >>>> two steps however, as it meant the compiler found all the calls I had to
> >>>> add this after).
> >>>
> >>> This set of patches fixes an issue with the change in service_task.c,
> >>> and implements the TODO you requested in your tree, that we remove the
> >>> sync handler from the async users.   
> >>>
> >>> I do this in two steps so we can bisect any future issue, but you can
> >>> squash them if you prefer.
> >>>
> >>> Can you look over this carefully, and review/push if you are OK?
> >>
> >> I've split them a bit and added some comments.
> >>
> >> There's not real diff compared to yours in the end.
> >>
> >> Both doesn't pass autobuild for me, even if I remove the top commit in both
> >> versions...
> > 
> > I ran a local autobuild (of samba only) on my irpc-no-event-context
> > branch, and it passed.   Do you have logs of the failure so I can look
> > into it?
> > 
> > (I'll run an autobuild on sn-devel now)
> 
> Both versions pass for me now (more than once), so it was the GETADDRINFO
> problem or something similar.
> 
> Are you fine with the attached patches?

Yes, thanks!

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140513/15b712f4/attachment.pgp>


More information about the samba-technical mailing list