Re: Regarding Async DNS resolver

On Mon, Jul 22, 2019 at 10:27 PM Daniel Stenberg <daniel_at_haxx.se> wrote:

> On Mon, 22 Jul 2019, Amit wrote:
>
> > I have done the changes as per your suggestion - that is, to create
> socket
> > pair during async thread initialization, return the read socket fd to
> client
> > application and write dummy data to signal client that socket is
> readable.
> >
> > I did the testing on version which I am using (7.57.11) and verified the
> > changes.
> >
> > Could you please review the attached patch file (generated after merging
> the
> > changes to latest libcurl) ? .
>
> Thanks! This is a great start.
>
> First, I had to edit it slighly to fix some checksrc warnings and I
> changed
> Curl_resolver_getsock() to not do the short expire things if there is a
> file
> descriptor to wait for as that would defeat its purpose a bit.
>
> I created a PR out of this for you so that it'll run all the tests first:
>
> https://github.com/curl/curl/pull/4140
>
> It needs to go green in all the tests to be subject for merge. Since this
> code
> seems to be using socketpair() unconditionally (ie also on Windows) I
> assume
> this won't build on Windows. It need to make that conditionally on the
> presense of that function adding #ifdef HAVE_SOCKETPAIR perhaps?
>
> Thanks for quick review !!

    I have added the changes under compile time switch (HAVE_SOCKET) to fix
compilation on Windows.

    Also, I forgot to provide one change - that is, to
return multi_runsingle CURLM_CALL_MULTI_PERFORM in case of
    async DNS resolver (so that client can query the sock FD for polling)
and have corrected the same. I have added some
    extra safe checks to take care of error cases. Please find attached new
patch file for your review.

 * P.S: Please note that I have done the changes only for
HAVE_GETADDRINFO. Let me know if you would like me to *
* consider the use-case of gethostbyname_thread as well ? The changes
would not be major but I was only concerned *
* about testing part. Or is it fine to add it later based on the
requirement ?*

      --

>
> / daniel.haxx.se | Get the best commercial curl support there is - from
> me
> | Private help, bug fixes, support, ports, new features
> | https://www.wolfssl.com/contact/
>

-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html