Merge pull request #8939 from DonLakeFlyer/UDPMutexMaster

Add mutex for _sessionTargets access. Remove range_for usage. General…

src/comm/UDPLink.cc

@@ -7,14 +7,6 @@
  *
  ****************************************************************************/
 
-
-/**
- * @file
- *   @brief Definition of UDP connection (server) for unmanned vehicles
- *   @author Lorenz Meier <mavteam@student.ethz.ch>
- *
- */
-
 #include <QtGlobal>
 #include <QTimer>
 #include <QList>
@@ -31,15 +23,12 @@
 #include "SettingsManager.h"
 #include "AutoConnectSettings.h"
 
-#define REMOVE_GONE_HOSTS 0
-
 static const char* kZeroconfRegistration = "_qgroundcontrol._udp";
 
 static bool is_ip(const QString& address)
 {
     int a,b,c,d;
-    if (sscanf(address.toStdString().c_str(), "%d.%d.%d.%d", &a, &b, &c, &d) != 4
+    if (sscanf(address.toStdString().c_str(), "%d.%d.%d.%d", &a, &b, &c, &d) != 4 && strcmp("::1", address.toStdString().c_str())) {
-            && strcmp("::1", address.toStdString().c_str())) {
         return false;
     } else {
         return true;
@@ -48,29 +37,28 @@ static bool is_ip(const QString& address)
 
 static QString get_ip_address(const QString& address)
 {
-    if(is_ip(address))
+    if (is_ip(address)) {
         return address;
+    }
     // Need to look it up
     QHostInfo info = QHostInfo::fromName(address);
-    if (info.error() == QHostInfo::NoError)
+    if (info.error() == QHostInfo::NoError) {
-    {
         QList<QHostAddress> hostAddresses = info.addresses();
-        for (int i = 0; i < hostAddresses.size(); i++)
+        for (int i=0; i<hostAddresses.size(); i++) {
-        {
             // Exclude all IPv6 addresses
-            if (!hostAddresses.at(i).toString().contains(":"))
+            if (!hostAddresses.at(i).toString().contains(":")) {
-            {
                 return hostAddresses.at(i).toString();
             }
         }
     }
-    return {};
+    return QString();
 }
 
 static bool contains_target(const QList<UDPCLient*> list, const QHostAddress& address, quint16 port)
 {
-    for(UDPCLient* target: list) {
+    for (int i=0; i<list.count(); i++) {
-        if(target->address == address && target->port == port) {
+        UDPCLient* target = list[i];
+        if (target->address == address && target->port == port) {
             return true;
         }
     }
@@ -78,20 +66,22 @@ static bool contains_target(const QList<UDPCLient*> list, const QHostAddress& ad
 }
 
 UDPLink::UDPLink(SharedLinkConfigurationPointer& config)
-    : LinkInterface(config)
+    : LinkInterface     (config)
-    #if defined(QGC_ZEROCONF_ENABLED)
+#if defined(QGC_ZEROCONF_ENABLED)
-    , _dnssServiceRef(nullptr)
+    , _dnssServiceRef   (nullptr)
-    #endif
+#endif
-    , _running(false)
+    , _running          (false)
-    , _socket(nullptr)
+    , _socket           (nullptr)
-    , _udpConfig(qobject_cast<UDPConfiguration*>(config.data()))
+    , _udpConfig        (qobject_cast<UDPConfiguration*>(config.data()))
-    , _connectState(false)
+    , _connectState     (false)
 {
     if (!_udpConfig) {
         qWarning() << "Internal error";
     }
-    for (const QHostAddress &address: QNetworkInterface::allAddresses()) {
+    auto allAddresses = QNetworkInterface::allAddresses();
-        _localAddress.append(QHostAddress(address));
+    for (int i=0; i<allAddresses.count(); i++) {
+        QHostAddress &address = allAddresses[i];
+        _localAddresses.append(QHostAddress(address));
     }
     moveToThread(this);
 }
@@ -116,7 +106,7 @@ UDPLink::~UDPLink()
  **/
 void UDPLink::run()
 {
-    if(_hardwareConnect()) {
+    if (_hardwareConnect()) {
         exec();
     }
     if (_socket) {
@@ -127,8 +117,7 @@ void UDPLink::run()
 
 void UDPLink::_restartConnection()
 {
-    if(this->isConnected())
+    if (this->isConnected()) {
-    {
         _disconnect();
         _connect();
     }
@@ -155,7 +144,8 @@ bool UDPLink::_isIpLocal(const QHostAddress& add)
     // On Windows, this is a very expensive call only Redmond would know
     // why. As such, we make it once and keep the list locally. If a new
     // interface shows up after we start, it won't be on this list.
-    for (const QHostAddress &address: _localAddress) {
+    for (int i=0; i<_localAddresses.count(); i++) {
+        QHostAddress &address = _localAddresses[i];
         if (address == add) {
             // This is a local address of the same host
             return true;
@@ -216,7 +206,7 @@ void UDPLink::readBytes()
         _socket->readDatagram(datagram.data(), datagram.size(), &sender, &senderPort);
         databuffer.append(datagram);
         //-- Wait a bit before sending it over
-        if(databuffer.size() > 10 * 1024) {
+        if (databuffer.size() > 10 * 1024) {
             emit bytesReceived(this, databuffer);
             databuffer.clear();
         }
@@ -236,7 +226,7 @@ void UDPLink::readBytes()
         }
     }
     //-- Send whatever is left
-    if(databuffer.size()) {
+    if (databuffer.size()) {
         emit bytesReceived(this, databuffer);
     }
 }
@@ -267,8 +257,7 @@ void UDPLink::_disconnect(void)
  **/
 bool UDPLink::_connect(void)
 {
-    if(this->isRunning() || _running)
+    if (this->isRunning() || _running) {
-    {
         _running = false;
         quit();
         wait();
@@ -408,7 +397,8 @@ void UDPConfiguration::_copyFrom(LinkConfiguration *source)
     if (usource) {
         _localPort = usource->localPort();
         _clearTargetHosts();
-        for(UDPCLient* target: usource->targetHosts()) {
+        for (int i=0; i<usource->targetHosts().count(); i++) {
+            UDPCLient* target = usource->targetHosts()[i];
             if(!contains_target(_targetHosts, target->address, target->port)) {
                 UDPCLient* newTarget = new UDPCLient(target);
                 _targetHosts.append(newTarget);
@@ -432,13 +422,10 @@ void UDPConfiguration::_clearTargetHosts()
 void UDPConfiguration::addHost(const QString host)
 {
     // Handle x.x.x.x:p
-    if (host.contains(":"))
+    if (host.contains(":")) {
-    {
         addHost(host.split(":").first(), host.split(":").last().toUInt());
-    }
+    } else {
-    // If no port, use default
+        // If no port, use default
-    else
-    {
         addHost(host, _localPort);
     }
 }
@@ -446,7 +433,7 @@ void UDPConfiguration::addHost(const QString host)
 void UDPConfiguration::addHost(const QString& host, quint16 port)
 {
     QString ipAdd = get_ip_address(host);
-    if(ipAdd.isEmpty()) {
+    if (ipAdd.isEmpty()) {
         qWarning() << "UDP:" << "Could not resolve host:" << host << "port:" << port;
     } else {
         QHostAddress address(ipAdd);
@@ -460,11 +447,10 @@ void UDPConfiguration::addHost(const QString& host, quint16 port)
 
 void UDPConfiguration::removeHost(const QString host)
 {
-    if (host.contains(":"))
+    if (host.contains(":")) {
-    {
         QHostAddress address = QHostAddress(get_ip_address(host.split(":").first()));
         quint16 port = host.split(":").last().toUInt();
-        for(int i = 0; i < _targetHosts.size(); i++) {
+        for (int i=0; i<_targetHosts.size(); i++) {
             UDPCLient* target = _targetHosts.at(i);
             if(target->address == address && target->port == port) {
                 _targetHosts.removeAt(i);
@@ -488,7 +474,7 @@ void UDPConfiguration::saveSettings(QSettings& settings, const QString& root)
     settings.beginGroup(root);
     settings.setValue("port", (int)_localPort);
     settings.setValue("hostCount", _targetHosts.size());
-    for(int i = 0; i < _targetHosts.size(); i++) {
+    for (int i=0; i<_targetHosts.size(); i++) {
         UDPCLient* target = _targetHosts.at(i);
         QString hkey = QString("host%1").arg(i);
         settings.setValue(hkey, target->address.toString());
@@ -505,7 +491,7 @@ void UDPConfiguration::loadSettings(QSettings& settings, const QString& root)
     settings.beginGroup(root);
     _localPort = (quint16)settings.value("port", acSettings->udpListenPort()->rawValue().toInt()).toUInt();
     int hostCount = settings.value("hostCount", 0).toInt();
-    for(int i = 0; i < hostCount; i++) {
+    for (int i=0; i<hostCount; i++) {
         QString hkey = QString("host%1").arg(i);
         QString pkey = QString("port%1").arg(i);
         if(settings.contains(hkey) && settings.contains(pkey)) {
@@ -518,7 +504,7 @@ void UDPConfiguration::loadSettings(QSettings& settings, const QString& root)
 
 void UDPConfiguration::updateSettings()
 {
-    if(_link) {
+    if (_link) {
         UDPLink* ulink = dynamic_cast<UDPLink*>(_link);
         if(ulink) {
             ulink->_restartConnection();
@@ -529,7 +515,7 @@ void UDPConfiguration::updateSettings()
 void UDPConfiguration::_updateHostList()
 {
     _hostList.clear();
-    for(int i = 0; i < _targetHosts.size(); i++) {
+    for (int i=0; i<_targetHosts.size(); i++) {
         UDPCLient* target = _targetHosts.at(i);
         QString host = QString("%1").arg(target->address.toString()) + ":" + QString("%1").arg(target->port);
         _hostList << host;

src/comm/UDPLink.h

@@ -209,7 +209,7 @@ private:
     UDPConfiguration*       _udpConfig;
     bool                    _connectState;
     QList<UDPCLient*>       _sessionTargets;
-    QList<QHostAddress>     _localAddress;
+    QMutex                  _sessionTargetsMutex;
-
+    QList<QHostAddress>     _localAddresses;
 };