Merge pull request #17031 from Chocobo1/net

Fix wrong GUI behavior in "Optional IP address to bind to" setting

src/base/net/geoipdatabase.cpp

@@ -516,3 +516,22 @@ QVariant GeoIPDatabase::readArrayValue(quint32 &offset, const quint32 count) con
 
     return array;
 }
+
+template <typename T>
+QVariant GeoIPDatabase::readPlainValue(quint32 &offset, const quint8 len) const
+{
+    T value = 0;
+    const uchar *const data = m_data + offset;
+    const quint32 availSize = m_size - offset;
+
+    if ((len > 0) && (len <= sizeof(T) && (availSize >= len)))
+    {
+        // copy input data to last 'len' bytes of 'value'
+        uchar *dst = reinterpret_cast<uchar *>(&value) + (sizeof(T) - len);
+        memcpy(dst, data, len);
+        fromBigEndian(reinterpret_cast<uchar *>(&value), sizeof(T));
+        offset += len;
+    }
+
+    return QVariant::fromValue(value);
+}

src/base/net/geoipdatabase.h

@@ -70,24 +70,7 @@ private:
     QVariant readMapValue(quint32 &offset, quint32 count) const;
     QVariant readArrayValue(quint32 &offset, quint32 count) const;
 
-    template<typename T>
-    QVariant readPlainValue(quint32 &offset, quint8 len) const
-    {
-        T value = 0;
-        const uchar *const data = m_data + offset;
-        const quint32 availSize = m_size - offset;
-
-        if ((len > 0) && (len <= sizeof(T) && (availSize >= len)))
-        {
-            // copy input data to last 'len' bytes of 'value'
-            uchar *dst = reinterpret_cast<uchar *>(&value) + (sizeof(T) - len);
-            memcpy(dst, data, len);
-            fromBigEndian(reinterpret_cast<uchar *>(&value), sizeof(T));
-            offset += len;
-        }
-
-        return QVariant::fromValue(value);
-    }
+    template <typename T> QVariant readPlainValue(quint32 &offset, quint8 len) const;
 
     // Metadata
     quint16 m_ipVersion;

src/gui/advancedsettings.cpp

@@ -156,7 +156,7 @@ AdvancedSettings::AdvancedSettings(QWidget *parent)
 {
     // column
     setColumnCount(COL_COUNT);
-    QStringList header = {tr("Setting"), tr("Value", "Value set for this setting")};
+    const QStringList header = {tr("Setting"), tr("Value", "Value set for this setting")};
     setHorizontalHeaderLabels(header);
     // row
     setRowCount(ROW_COUNT);
@@ -171,7 +171,7 @@ AdvancedSettings::AdvancedSettings(QWidget *parent)
     horizontalHeader()->setStretchLastSection(true);
 }
 
-void AdvancedSettings::saveAdvancedSettings()
+void AdvancedSettings::saveAdvancedSettings() const
 {
     Preferences *const pref = Preferences::instance();
     BitTorrent::Session *const session = BitTorrent::Session::instance();
@@ -249,23 +249,14 @@ void AdvancedSettings::saveAdvancedSettings()
     pref->resolvePeerCountries(m_checkBoxResolveCountries.isChecked());
     pref->resolvePeerHostNames(m_checkBoxResolveHosts.isChecked());
     // Network interface
-    if (m_comboBoxInterface.currentIndex() == 0)
-    {
-        // All interfaces (default)
-        session->setNetworkInterface(QString());
-        session->setNetworkInterfaceName(QString());
-    }
-    else
-    {
-        session->setNetworkInterface(m_comboBoxInterface.itemData(m_comboBoxInterface.currentIndex()).toString());
-        session->setNetworkInterfaceName(m_comboBoxInterface.currentText());
-    }
-
+    session->setNetworkInterface(m_comboBoxInterface.currentData().toString());
+    session->setNetworkInterfaceName((m_comboBoxInterface.currentIndex() == 0)
+        ? QString()
+        : m_comboBoxInterface.currentText());
     // Interface address
     // Construct a QHostAddress to filter malformed strings
-    const QHostAddress ifaceAddr(m_comboBoxInterfaceAddress.currentData().toString().trimmed());
+    const QHostAddress ifaceAddr {m_comboBoxInterfaceAddress.currentData().toString()};
     session->setNetworkInterfaceAddress(ifaceAddr.toString());
-
     // Announce IP
     // Construct a QHostAddress to filter malformed strings
     const QHostAddress addr(m_lineEditAnnounceIP.text().trimmed());
@@ -314,7 +305,7 @@ void AdvancedSettings::saveAdvancedSettings()
 }
 
 #ifndef QBT_USES_LIBTORRENT2
-void AdvancedSettings::updateCacheSpinSuffix(int value)
+void AdvancedSettings::updateCacheSpinSuffix(const int value)
 {
     if (value == 0)
         m_spinBoxCache.setSuffix(tr(" (disabled)"));
@@ -335,45 +326,57 @@ void AdvancedSettings::updateSaveResumeDataIntervalSuffix(const int value)
 
 void AdvancedSettings::updateInterfaceAddressCombo()
 {
-    // Try to get the currently selected interface name
-    const QString ifaceName = m_comboBoxInterface.itemData(m_comboBoxInterface.currentIndex()).toString(); // Empty string for the first element
-    const QString currentAddress = BitTorrent::Session::instance()->networkInterfaceAddress();
+    const auto toString = [](const QHostAddress &address) -> QString
+    {
+        switch (address.protocol()) {
+        case QAbstractSocket::IPv4Protocol:
+            return address.toString();
+        case QAbstractSocket::IPv6Protocol:
+            return Utils::Net::canonicalIPv6Addr(address).toString();
+        default:
+            Q_ASSERT(false);
+            break;
+        }
+        return {};
+    };
 
-    // Clear all items and reinsert them, default to all
+    // Clear all items and reinsert them
     m_comboBoxInterfaceAddress.clear();
-    m_comboBoxInterfaceAddress.addItem(tr("All addresses"), {});
-    m_comboBoxInterfaceAddress.addItem(tr("All IPv4 addresses"), u"0.0.0.0"_qs);
-    m_comboBoxInterfaceAddress.addItem(tr("All IPv6 addresses"), u"::"_qs);
+    m_comboBoxInterfaceAddress.addItem(tr("All addresses"), QString());
+    m_comboBoxInterfaceAddress.addItem(tr("All IPv4 addresses"), QHostAddress(QHostAddress::AnyIPv4).toString());
+    m_comboBoxInterfaceAddress.addItem(tr("All IPv6 addresses"), QHostAddress(QHostAddress::AnyIPv6).toString());
 
-    const auto populateCombo = [this](const QHostAddress &addr)
+    const QString currentIface = m_comboBoxInterface.currentData().toString();
+    if (currentIface.isEmpty())  // `any` interface
     {
-        if (addr.protocol() == QAbstractSocket::IPv4Protocol)
+        for (const QHostAddress &address : asConst(QNetworkInterface::allAddresses()))
         {
-            const QString str = addr.toString();
-            m_comboBoxInterfaceAddress.addItem(str, str);
+            const QString addressString = toString(address);
+            m_comboBoxInterfaceAddress.addItem(addressString, addressString);
         }
-        else if (addr.protocol() == QAbstractSocket::IPv6Protocol)
+    }
+    else
+    {
+        const QList<QNetworkAddressEntry> addresses = QNetworkInterface::interfaceFromName(currentIface).addressEntries();
+        for (const QNetworkAddressEntry &entry : addresses)
         {
-            const QString str = Utils::Net::canonicalIPv6Addr(addr).toString();
-            m_comboBoxInterfaceAddress.addItem(str, str);
+            const QString addressString = toString(entry.ip());
+            m_comboBoxInterfaceAddress.addItem(addressString, addressString);
         }
-    };
+    }
 
-    if (ifaceName.isEmpty())
+    const QString currentAddress = BitTorrent::Session::instance()->networkInterfaceAddress();
+    const int index = m_comboBoxInterfaceAddress.findData(currentAddress);
+    if (index > -1)
     {
-        for (const QHostAddress &addr : asConst(QNetworkInterface::allAddresses()))
-            populateCombo(addr);
+        m_comboBoxInterfaceAddress.setCurrentIndex(index);
     }
     else
     {
-        const QNetworkInterface iface = QNetworkInterface::interfaceFromName(ifaceName);
-        const QList<QNetworkAddressEntry> addresses = iface.addressEntries();
-        for (const QNetworkAddressEntry &entry : addresses)
-            populateCombo(entry.ip());
+        // not found, for the sake of UI consistency, add such entry
+        m_comboBoxInterfaceAddress.addItem(currentAddress, currentAddress);
+        m_comboBoxInterfaceAddress.setCurrentIndex(m_comboBoxInterfaceAddress.count() - 1);
     }
-
-    const int index = m_comboBoxInterfaceAddress.findData(currentAddress);
-    m_comboBoxInterfaceAddress.setCurrentIndex(std::max(index, 0));
 }
 
 void AdvancedSettings::loadAdvancedSettings()
@@ -627,26 +630,23 @@ void AdvancedSettings::loadAdvancedSettings()
     m_checkBoxResolveHosts.setChecked(pref->resolvePeerHostNames());
     addRow(RESOLVE_HOSTS, tr("Resolve peer host names"), &m_checkBoxResolveHosts);
     // Network interface
-    m_comboBoxInterface.addItem(tr("Any interface", "i.e. Any network interface"));
-    const QString currentInterface = session->networkInterface();
-    bool interfaceExists = currentInterface.isEmpty();
-    int i = 1;
+    m_comboBoxInterface.addItem(tr("Any interface", "i.e. Any network interface"), QString());
     for (const QNetworkInterface &iface : asConst(QNetworkInterface::allInterfaces()))
-    {
         m_comboBoxInterface.addItem(iface.humanReadableName(), iface.name());
-        if (!currentInterface.isEmpty() && (iface.name() == currentInterface))
-        {
-            m_comboBoxInterface.setCurrentIndex(i);
-            interfaceExists = true;
-        }
-        ++i;
+
+    const QString currentInterface = session->networkInterface();
+    const int ifaceIndex = m_comboBoxInterface.findData(currentInterface);
+    if (ifaceIndex > -1)
+    {
+        m_comboBoxInterface.setCurrentIndex(ifaceIndex);
     }
-    // Saved interface does not exist, show it anyway
-    if (!interfaceExists)
+    else
     {
+        // Saved interface does not exist, show it
         m_comboBoxInterface.addItem(session->networkInterfaceName(), currentInterface);
-        m_comboBoxInterface.setCurrentIndex(i);
+        m_comboBoxInterface.setCurrentIndex(m_comboBoxInterface.count() - 1);
     }
+
     connect(&m_comboBoxInterface, qOverload<int>(&QComboBox::currentIndexChanged)
         , this, &AdvancedSettings::updateInterfaceAddressCombo);
     addRow(NETWORK_IFACE, tr("Network interface"), &m_comboBoxInterface);

src/gui/advancedsettings.h

@@ -34,15 +34,16 @@
 #include <QSpinBox>
 #include <QTableWidget>
 
-class AdvancedSettings : public QTableWidget
+class AdvancedSettings final : public QTableWidget
 {
     Q_OBJECT
+    Q_DISABLE_COPY_MOVE(AdvancedSettings)
 
 public:
     AdvancedSettings(QWidget *parent);
 
 public slots:
-    void saveAdvancedSettings();
+    void saveAdvancedSettings() const;
 
 signals:
     void settingsChanged();