[PATCH] DNS cache for Konqueror/KIO

[Roland Harnau]

2008/7/2, Thiago Macieira <thiago at kde.org>:

> Ok, I have finally got my act together and reviewed the patch.

Thanks for the quite in-depth review. I have tried to address the
points you've raised in the attached patch.

[..]
> Why do we need a static function to return the instance to call
> lookupHost()? Can't lookupHost() be static instead?

There is a rather stylistic reason: if state is involved this should
be exposed to the user and not hidden by static methods (as in
KIO::Scheduler). But as the cache  and the list of processed queries
(the state maintained by the HostInfoAgentPrivate instance) is
transparent to the user, this argument is not really conclusive and I
have  therefore reduced the HostInfoAgent class to a mere function.

[..]
> The same in:
> +    if (openQueries.contains(hostName)) {
> +        Query* query = openQueries.value(hostName);
>
> Please use an iterator and compare to openQueries.end() to see if the
> object is there.

openQueries has type QHash<QString, Query*> and  so I've replaced the
double lookup (which is not really hurting because of  O(1) complexity
in the average case)  by a  test for 0. It is somewhat surprising that
your approach is apparently only available for the STL iterator
interface but not for Java-style iterators.

> In all, your patch is very good and very clever. The only drawback (=
> possible future improvement) is the fact that the IP addresses will be
> tried always in the same order. Right now, the DNS server will change the
> order in a round-robin fashion at every request.

According to [1]  the addreses returned by  getaddrinfo() (glibc
version)  are ordered:
"The important thing to remember here is that the addresses are
returned in an order where the first returned address has the highest
probability to succeed."

[1]  http://people.redhat.com/drepper/userapi-ipv6.html

Roland
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kdelibs-DNS-cache.patch
Type: text/x-patch
Size: 14675 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080707/36d1295c/attachment.bin>