Skip to content
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

Support large result sets #14

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Support large result sets #14

wants to merge 3 commits into from

Conversation

djs55
Copy link
Member

@djs55 djs55 commented Apr 3, 2019

If the result set is large (> 1500 bytes?) then the callback is called with the kDNSServiceFlagsMoreComing flag set meaning that we should call DNSServiceProcessResult again to accumulate all the results.

Signed-off-by: David Scott <dave@recoil.org>
Every time the callback is called there is a "more results expected"
flag. If the final one of these is true then we have to call
`DNSServiceProcessResult` a.k.a. `query_result` again to accumulate the
full result list.

Signed-off-by: David Scott <dave@recoil.org>
djs55 added a commit to djs55/vpnkit that referenced this pull request Apr 3, 2019
This incorporates

mirage/ocaml-osx-dnssd#14

backported to the opam-1.2 branch.

Signed-off-by: David Scott <dave.scott@docker.com>
djs55 added a commit to djs55/vpnkit that referenced this pull request Apr 3, 2019
This incorporates

mirage/ocaml-osx-dnssd#14

backported to the opam-1.2 branch.

Signed-off-by: David Scott <dave.scott@docker.com>
The complication here is that when there are more result, we need to
return a flag to the caller so they can `select` on the file descriptor
again to wait for more results. This means the LowLevel API has to be
changed.

Signed-off-by: David Scott <dave@recoil.org>
djs55 added a commit to djs55/vpnkit that referenced this pull request Apr 5, 2019
This incorporates

mirage/ocaml-osx-dnssd#14

backported to the opam-1.2 branch.

Signed-off-by: David Scott <dave.scott@docker.com>
Copy link
Member

@avsm avsm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read through this and it lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants