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

Reply via email to