Improve peer host name resolution (closes #360)

- Take a QString in argument to avoid converting it twice. The caller already
  has the IP as a QString.
- Fix bug where hostnames coming from the cache would not the displayed.
- Fix bug where the resolved hostnames would not be displayed if the IP we know
  does not match the first one in the QHostInfo::addresses() list. It may come
  later in the list or may be formatted differently (possible with IPv6).
- Avoid requesting again host names for IPs that were already requested recently
  but whose resolution did not return a useful hostname.

src/properties/peerlistwidget.cpp

@@ -325,12 +325,15 @@ void PeerListWidget::loadPeers(const QTorrentHandle &h, bool force_hostname_reso
       updatePeer(peer_ip, peer);
       old_peers_set.remove(peer_ip);
       if (force_hostname_resolution && m_resolver) {
-        m_resolver->resolve(peer.ip);
+        m_resolver->resolve(peer_ip);
       }
     } else {
       // Add new peer
       m_peerItems[peer_ip] = addPeer(peer_ip, peer);
       m_peerEndpoints[peer_ip] = peer.ip;
+      // Resolve peer host name is asked
+      if (m_resolver)
+        m_resolver->resolve(peer_ip);
     }
   }
   // Delete peers that are gone
@@ -350,9 +353,6 @@ QStandardItem* PeerListWidget::addPeer(const QString& ip, const peer_info& peer)
   m_listModel->insertRow(row);
   m_listModel->setData(m_listModel->index(row, PeerListDelegate::IP), ip);
   m_listModel->setData(m_listModel->index(row, PeerListDelegate::IP_HIDDEN), ip);
-  // Resolve peer host name is asked
-  if (m_resolver)
-    m_resolver->resolve(peer.ip);
   if (m_displayFlags) {
     const QIcon ico = GeoIPManager::CountryISOCodeToIcon(peer.country);
     if (!ico.isNull()) {

src/reverseresolution.h

@@ -59,17 +59,16 @@ public:
     qDebug("Deleting host name resolver...");
   }
 
-  void resolve(const libtorrent::asio::ip::tcp::endpoint &ip) {
-    boost::system::error_code ec;
-    const QString ip_str = misc::toQString(ip.address().to_string(ec));
-    if (ec) return;
-    if (m_cache.contains(ip_str)) {
-      qDebug("Resolved host name using cache");
-      emit ip_resolved(ip_str, *m_cache.object(ip_str));
+  void resolve(const QString &ip) {
+    if (m_cache.contains(ip)) {
+      const QString& hostname = *m_cache.object(ip);
+      qDebug() << "Resolved host name using cache: " << ip << " -> " << hostname;
+      if (isUsefulHostName(hostname, ip))
+        emit ip_resolved(ip, hostname);
       return;
     }
     // Actually resolve the ip
-    QHostInfo::lookupHost(ip_str, this, SLOT(hostResolved(QHostInfo)));
+    m_lookups.insert(QHostInfo::lookupHost(ip, this, SLOT(hostResolved(QHostInfo))), ip);
   }
 
 signals:
@@ -77,20 +76,29 @@ signals:
 
 private slots:
   void hostResolved(const QHostInfo& host) {
-    if (host.error() == QHostInfo::NoError) {
-      const QString hostname = host.hostName();
-      if (host.addresses().isEmpty() || hostname.isEmpty()) return;
-      const QString ip = host.addresses().first().toString();
-      if (hostname != ip) {
-        //qDebug() << Q_FUNC_INFO << ip << QString("->") << hostname;
-        m_cache.insert(ip, new QString(hostname));
-        emit ip_resolved(ip, hostname);
-      }
+    const QString& ip = m_lookups.take(host.lookupId());
+    Q_ASSERT(!ip.isNull());
+
+    if (host.error() != QHostInfo::NoError) {
+      qDebug() << "DNS Reverse resolution error: " << host.errorString();
+      return;
     }
+
+    const QString& hostname = host.hostName();
+
+    qDebug() << Q_FUNC_INFO << ip << QString("->") << hostname;
+    m_cache.insert(ip, new QString(hostname));
+    if (isUsefulHostName(hostname, ip))
+      emit ip_resolved(ip, hostname);
   }
 
 private:
-  QCache<QString, QString> m_cache;
+  static inline bool isUsefulHostName(const QString& hostname, const QString& ip) {
+    return (!hostname.isEmpty() && hostname != ip);
+  }
+
+  QHash<int /* LookupID */, QString /* IP */> m_lookups;
+  QCache<QString /* IP */, QString /* HostName */> m_cache;
 };