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

[voidswitch]

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