[PATCH] winbindd: disconnect child process if request is cancelled at main process

Stefan (metze) Metzmacher metze at samba.org
Mon Jun 29 01:14:23 MDT 2015


Hi Uri,

> Here's V3  - changing comments and adding BUG: to commit message.
> 
> https://bugzilla.samba.org/show_bug.cgi?id=11358

Looks good thanks!

Can I get a 2nd team reviewer?

So that I can push this?

Thanks!
metze

> On Wed, Jun 24, 2015 at 1:43 PM, Stefan (metze) Metzmacher
> <metze at samba.org> wrote:
>> Hi Uri,
>>
>>> Thanks very much for reviewing.
>>> I've tried following your suggestions and the example and indeed the
>>> result is much more pleasant. Review appreciated.
>>>
>>> One change I did make was to use the subrequest and not the child
>>> pointer as a test for whether we have anything to do, and this is
>>> because unlike the smbcli example, the request may be queued initially
>>> with no real "io" request. In that state we want the cleanup to be a
>>> no-op. More generally it's the presence of a subrequest that calls for
>>> cleanup considerations and if there isn't one, then the cleanup should
>>> be a no-op. Given that, I did not include the setting of child to NULL
>>> once we're done with it, although one might want that out of general
>>> hygiene considerations.
>>
>> I guess the /* we're done and don't need a reference to the child anymore */
>> comment is confusing without setting state->child = NULL;
>>
>> Please create a bug report and add a reference to the commit message
>> and add a comment before ret = wb_simple_trans_recv(subreq, state,
>> &state->response, &err);
>> explaining that we don't call TALLOC_FREE(subreq); and defer that to
>> wb_child_request_cleanup().
>>
>> Otherwise Reviewed-by: me
>>
>> Thanks!
>> metze
>>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150629/6fa2da34/attachment.pgp>


More information about the samba-technical mailing list