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

Garming Sam garming at catalyst.net.nz
Mon Apr 18 22:56:00 UTC 2016


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).

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.

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.

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.


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