On Sun, 23 Jan 2011, Vsevolod Novikov wrote:

Thanks a lot for your patch!

> Not just replace, but be able to add a new one

Right, that's what I meant: a new added one replaces c-ares as the one in use
during requests as there's only ever one active at a time.

> patching only those files which should be. Ideal solution is to make it
> possible using configure script (or even runtime option) only, and proposed
> patch makes such solution more available.

I think selecting resolver backend with configure is good enough. It's already
there and working, and that's how we select other backends like SSL library.

Your patch is a good step along the way and I really appreciate it. But I
can't just accept a step along the way unless I know that someone (like
yourself) will continue the walk until the goals are met, as that would mean
that I will have to do it and I'm not prepared to do that just yet. Hence my
questions and remarks.

I want to see this work get done, but we need to discuss on how it should work
to become as good as we want it.

> The resolver template used in a patch, has been got from the threaded
> resolver, not from the cares binding. The threaded resolver starts a
> resolving thread at a moment when it is asked for a name, and reserves
> resources at the same time. These resources are released when the
> ..._destroy_... function is called.

Yes, but then the threaded resolver has nothing that survives after that, but
the c-ares code has.

> I've implemented the same behaviour for the cares resolver, so a separate
> control for ares handler is not required.

Right, but you've also sacrifised performance and the existing way of
operating, so I wanted to know your reasoning for this. Your response here
seems to imply that you had no reasoning at all. Possibly because you didn't
consider and were aware of the consequences.

I think we should allow the resolver backend to have a pointer/handle in the
SessionHandle struct for stuff that can be kept around between requests - as
the c-ares code works today. And when that is used, the resolver backend also
needs to be told when the dup_handle() function is called so that the resolver
handle can be duped.

> It is not good that tests don't cover possible resolver issues, but i've
> noticed that some of ones use requests to named resources, so the resolver
> requests should be issued ... probably ...

There is very limited name resolve testing primarily because it isn't that
easy to do in a controlled manner (and we have no DNS test server), and
secondary simply because we haven't had anyone work on that...

> To see what the patch really does, you can use git with option like "sticky
> tag" in svn ... i am using svn, and it allows to get any previous version of
> sourses ... i think the git allows to do the same easy ...

Sure, I could use the older code and apply your patch to, but what would the
purpose of that be? We've _fixed_ things in the new code so we want the new
way of doing things, not the old way...

-- 
  / daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html