[RESEND PATCH] DsBind and DsGetDomainControllerInfo to return 2k8 structs complete
voidswitch at gmail.com
Tue Apr 19 05:08:47 UTC 2016
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
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:
> 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
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,
> On 19/04/16 09:20, Void wrote:
>> I'll get this right eventually.
More information about the samba-technical