[PATCH] DsReplGetInfo() - infoType == DS_REPL_INFO_NEIGHBORS

[tridge at samba.org]

Hi Erick,

Thanks for the patch!

 > This is the patch for the implementation of the DsGetReplInfo()
 > MS-DRSR 4.1.13) for the case infoType == DS_REPL_INFO_NEIGHBORS
 > (request information about the repsFrom neighbors of the target DC).
 > There is also a torture test for this call.

Thanks. For this sort of patch, it is best to split it up into logical
pieces. In this case I think your smbtorture work should be a separate
patch from your dsdb/kcc work, and your two dsdb/kcc changes could
also be better split. I think one patch adding the framework for the
kccdrs_replica_get_info call, and one for each of the info levels
would make sense. If you are not sure how to split up patches, then
perhaps the easiest is to take the code as a whole and use "git add --patch"
then follow the prompts for what you want to include in the current
commit. After you've included what you want in a commit then use "git
commit" to add it.

 > On the tests I made using smbtorture (with RPC-DSGETINFO test) it
 > looks like the values on the fields of the reply message are consistent
 > with what Windows 2008 fill in those fields.

good!

 > I'm planning to send one patch for each of the infoTypes. I can't
 > implement some of them now, because the information needed is
 > not available yet on drs. The next patch will be for infoType
 > DS_REPL_INFO_REPSTO.

that sounds good, but please also do a bit of cleanup of your current
patches so they can be included. Apart from the patch reorganisation I
mentioned above, I think the following things could be improved:

 - the patch puts in a lot of debug code at a low logging level. It is
   useful to have this while developing the code, but its a bit too
   verbose for production code.

 - the big repsFromTo1 comment at the top of kcc_drs_replica_info.c is
   not really appropriate in a patch 

 - you need to check for more failures, for example memory allocation
   failures, and return an appropriate error code

 - some of your line lengths are far too long, and the indentation is
   very bad (eg. in the function header of
   kccdrs_replica_get_info_neighbours()) (see the samba coding guide
   for details)

 - rather than including pieces of the MS-DRSR doc, please just use a
   reference to the section number and doc name

 - please make functions static when they are used only by one file
   (eg. get_master_ncs)

 - in torture/rpc/dsgetinfo.c, please put your own copyright at the
   top, but say which other file the code is based on

These may all seem like minor things, but it helps us to keep the code
base a bit neater and easier for others to follow. Thanks!

Cheers, Tridge