-
Notifications
You must be signed in to change notification settings - Fork 365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add DirSync Control / Search #110
Conversation
this implements the dirsync control as described in https://msdn.microsoft.com/en-us/library/aa366978(v=vs.85).aspx check ExampleDirSync() in search_test.go how to use
control.go
Outdated
value.Description += " (DirSync)" | ||
c := new(ControlDirSync) | ||
if value.Value != nil { | ||
valueChildren := ber.DecodePacket(value.Data.Bytes()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason DecodePacketErr
is not used and checked here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
laziness? ;-) this started as a copy of the paging control above...
control.go
Outdated
value.Description = "DirSync Control Value" | ||
value.Children[0].Description = "Flags" | ||
value.Children[1].Description = "MaxAttrCnt" | ||
value.Children[2].Description = "Cookie" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this safe; assuming Children
has at least 3 elements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the spec says there must be always 3, the cookie will be an empty string if it's unset (which only happens when the client sends the first time
@@ -29,3 +31,48 @@ func TestNewEntry(t *testing.T) { | |||
iteration = iteration + 1 | |||
} | |||
} | |||
|
|||
func ExampleDirSync() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there tests that could be added too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, encoding / decoding in control_test.go
the race fix would also fix the build of PR #104 |
conn.go
Outdated
@@ -96,6 +96,7 @@ type Conn struct { | |||
once sync.Once | |||
outstandingRequests uint | |||
messageMutex sync.Mutex | |||
timeoutMutex sync.Mutex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
waiting for #168 ... |
Can we clean this up and get it ready to merge? |
@vetinari any chance you'd want to update this PR too? |
this implements the dirsync control as described in
https://msdn.microsoft.com/en-us/library/aa366978(v=vs.85).aspx
check ExampleDirSync() in search_test.go how to use