[RESEND PATCH] DsBind and DsGetDomainControllerInfo to return 2k8 structs complete

voidswitch voidswitch at gmail.com
Wed Apr 20 19:15:47 UTC 2016


Hi,

Currently I've done some cleanup, and look into writing tests. Is it 
preferable to write them in Python, or should it be added to 
source4/torture/rpc/drsuapi.c?

regards,

Dirk

On 19.04.2016 23:45, Garming Sam wrote:
> Hi,
>
> I see what you mean now, so that all seems fine. If you haven't already
> found it, you might want to look at the definition of drs_DsBind in
> python/samba/drs_utils.py for the tests.
>
> Cheers.
>
> On 19/04/16 17:08, voidswitch wrote:
>> Hi,
>>
>> On 19.04.2016 00:56, Garming Sam wrote:
>>> Hi Dirk,
>>>
>>> Just from looking a the patch, I've noticed that there was a info24 that
>>> used to be handled as well as the info28 (so a case statement with the
>>> three would be preferable). I believe there are some other lengths also
>>> available (probably for 2012), but we probably don't handle them
>>> properly in other places either so you probably don't need to worry
>>> about those (unless you really want to go out of your way).
>> drsuapi_bind_state used to store info28 structs only, so any other
>> size needed to be converted. This was done only with info24 requests.
>> I changed drsuapi_bind_state to use DsBindInfoCtr. Whatever the client
>> sends, we simply can store it in drsuapi_bind_state. Same with our
>> responses.
>>
>> Our response should probably take the current functional level into
>> account. Currently I send a response which clients should understand,
>> as length of the request >= length of the response. Clients sending 24
>> requests have always gotten a 28 response.
>>> More importantly though, a test would be good to assert that the changes
>>> haven't broken anything (and test your new behaviour). A good way to do
>>> this would be to order the patches so that first there is the test which
>>> passes against the existing levels and fails against the new one. And
>>> then would be the patch that fixes the behaviour for the new case so all
>>> the tests now pass.
>>>
>>> An example of how you might start:
>>> source4/torture/drs/python/getnc_exop.py
>>>
>>> You'd need to do the bind at the different levels and basically just
>>> assert that the result values are how you would expect. It should be a
>>> straightforward test, but a valuable one.
>> This should push me in the right direction regarding tests. I'll add
>> them.
>>> Some minor things:
>>> * There's a few C++ '//' comments, which we try to avoid.
>>> * The for loop is a little inconsistent, there's spaces for i <
>>> res->count, but not for i = 0. And I now realize that it was copied from
>>> above... It would be nice to be able to share some part of this code if
>>> possible.
>> The copy was the simplest option available, since it is really the
>> same. I guess a patch to clean this up should follow. Anyway I'll look
>> at it.
>>> If you haven't already read the README.Coding, it's a good place to
>>> start. Otherwise, the changes seem quite reasonable and thanks for
>>> working on it. If you need any other help, feel free to ask.
>> Thanks for the comments,
>> Dirk
>>
>>> Cheers,
>>>
>>> Garming
>>>
>>> On 19/04/16 09:20, Void wrote:
>>>> I'll get this right eventually.
>>>>
>>>> Thanks
>>>> Dirk




More information about the samba-technical mailing list