master4-dcerpc-ok
Stefan (metze) Metzmacher
metze at samba.org
Wed Mar 19 04:11:05 MDT 2014
Am 19.03.2014 09:53, schrieb Andreas Schneider:
> On Wednesday 19 March 2014 09:45:26 Stefan Metzmacher wrote:
>> Am 19.03.2014 08:43, schrieb Andrew Bartlett:
>>> On Wed, 2014-03-19 at 07:33 +0100, Stefan (metze) Metzmacher wrote:
>>>> Hi Andrew,
>>>>
>>>>>>>> here's new stuff in my
>>>>>>>> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/
>>>>>>>> master4-dcerpc-ok branch.
>>>>>>>>
>>>>>>>> Please review and push:-)
>>>>>>>
>>>>>>> There're some updates in this branch, which also fix the regressions
>>>>>>> Andrew found.
>>>>>>
>>>>>> Any idea why didn't our tests find that regression?
>>>>>>
>>>>>> Andrew Bartlett
>>>>>
>>>>> 305f97ceb5bc2c00d2e1725a3e45482d4e59bac2
>>>>>
>>>>> s4:libcli/clilsa: make use of dcerpc_pipe_connect_b()
>>>>>
>>>>> Can you explain what calling this over and over with the same binding
>>>>> the "connection" does?
>>>>>
>>>>> status = dcerpc_binding_set_pointer_option(binding,
>>>>>
>>>>> + "connection",
>>>>> + struct smbXcli_conn,
>>>>> + cli->transport->conn);
>>>>> + if (!NT_STATUS_IS_OK(status)) {
>>>>> + talloc_free(lsa);
>>>>> + return status;
>>>>> + }
>>>>> +
>>>>> + status = dcerpc_binding_set_pointer_option(binding,
>>>>> + "connection",
>>>>> + struct smbXcli_session,
>>>>> + cli->session->smbXcli);
>>>>> + if (!NT_STATUS_IS_OK(status)) {
>>>>> + talloc_free(lsa);
>>>>> + return status;
>>>>> + }
>>>>> +
>>>>> + status = dcerpc_binding_set_pointer_option(binding,
>>>>> + "connection",
>>>>> + struct smbXcli_tcon,
>>>>> + lsa->ipc_tree->smbXcli);
>>>>>
>>>>> There is a similar pattern in 04061bb3bf6d63cfa76028a6c878df9d3fe7af2c
>>>>> s4:torture/samba3rpc: split out pipe_bind_smbXcli() and go via
>>>>> dcerpc_pipe_connect_b()
>
>
> I would argue that there are important comments in the code missing to
> understand this ...
>
>
>>> What I don't get is why we have a string "connection" here, that doesn't
>>> change (so why isn't it a enum or at least a #define), and then a magic
>>> type, which you would think would be only a way to ensure the void*
>>> being passed in was correct, but I presume is used as part of a
>>> key-value list somewhere.
>>
>> Yes, see the commit message of
>> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=34e2c0762950210a
>> c6ef5dd40a5b4211ca595b4e
>
> This should be doxygen documentation of the function!! The last patchset I
> reviewed I gave a NAK if a public function had no doxygen documentation.
>
> I could review the patchset too or Andrew you give the NAK for missng
> documentation.
I'll add some doxygen stuff...
metze
More information about the samba-technical
mailing list