Coding style and return early refactoring

kded/backends/lan/lanlinkprovider.cpp

@@ -68,10 +68,13 @@ LanLinkProvider::LanLinkProvider()
 
 void LanLinkProvider::onStart()
 {
-    mUdpServer->bind(QHostAddress::Broadcast, port, QUdpSocket::ShareAddress);
+    bool buildSucceed = mUdpServer->bind(QHostAddress::Broadcast, port, QUdpSocket::ShareAddress)
+    Q_ASSERT(buildSucceed);
 
-    tcpPort = port;
+    mTcpPort = port;
-    while (!mTcpServer->listen(QHostAddress::Any, tcpPort)) tcpPort++;
+    while (!mTcpServer->listen(QHostAddress::Any, mTcpPort)) {
+        mTcpPort++;
+    }
 
     onNetworkChange(QNetworkSession::Connected);
 }
@@ -87,12 +90,14 @@ void LanLinkProvider::onNetworkChange(QNetworkSession::State state)
 {
     Q_UNUSED(state);
 
-    if (!mTcpServer->isListening()) return;
+    if (!mTcpServer->isListening()) {
+        return;
+    }
 
     NetworkPackage np("");
     NetworkPackage::createIdentityPackage(&np);
-    np.set("tcpPort",tcpPort);
+    np.set("tcpPort", mTcpPort);
-    QUdpSocket().writeDatagram(np.serialize(),QHostAddress("255.255.255.255"), port);
+    mUdpSocket.writeDatagram(np.serialize(), QHostAddress("255.255.255.255"), port);
 }
 
 //I'm the existing device, a new device is kindly introducing itself (I will create a TcpSocket)
@@ -109,40 +114,39 @@ void LanLinkProvider::newUdpConnection()
         NetworkPackage* np = new NetworkPackage("");
         bool success = NetworkPackage::unserialize(datagram,np);
 
-        if (success && np->type() == PACKAGE_TYPE_IDENTITY) {
+        mUdpServer->readDatagram(datagram.data(), datagram.size(), &sender);
+
+        NetworkPackage* receivedPackage = new NetworkPackage("");
+        bool success = NetworkPackage::unserialize(datagram, receivedPackage);
+
+        if (!success || receivedPackage->type() != PACKAGE_TYPE_IDENTITY) {
+            delete receivedPackage;
+        }
 
         NetworkPackage np2("");
         NetworkPackage::createIdentityPackage(&np2);
 
-            if (np->get<QString>("deviceId") == np2.get<QString>("deviceId")) {
+        if (receivedPackage->get<QString>("deviceId") == np2.get<QString>("deviceId")) {
             //kDebug(kdeconnect_kded()) << "Ignoring my own broadcast";
             return;
         }
 
-            int tcpPort = np->get<int>("tcpPort",port);
+        int tcpPort = receivedPackage->get<int>("tcpPort", port);
 
         //kDebug(kdeconnect_kded()) << "Received Udp presentation from" << sender << "asking for a tcp connection on port " << tcpPort;
 
         QTcpSocket* socket = new QTcpSocket(this);
-            receivedIdentityPackages[socket].np = np;
+        receivedIdentityPackages[socket].np = receivedPackage;
         receivedIdentityPackages[socket].sender = sender;
         connect(socket, SIGNAL(connected()), this, SLOT(connected()));
         connect(socket, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(connectError()));
         socket->connectToHost(sender, tcpPort);
-
-
-        } else {
-
-            delete np;
-
     }
 }
 
-}
-
 void LanLinkProvider::connectError()
 {
-    QTcpSocket* socket = (QTcpSocket*)sender();
+    QTcpSocket* socket = qobject_cast<QTcpSocket*>(sender());
 
     disconnect(socket, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(connectError()));
     disconnect(socket, SIGNAL(connected()), this, SLOT(connected()));
@@ -150,63 +154,64 @@ void LanLinkProvider::connectError()
     kDebug(kdeconnect_kded()) << "Fallback (1), try reverse connection";
     NetworkPackage np("");
     NetworkPackage::createIdentityPackage(&np);
-    np.set("tcpPort",tcpPort);
+    np.set("tcpPort", mTcpPort);
-    QUdpSocket().writeDatagram(np.serialize(), receivedIdentityPackages[socket].sender, port);
+    mUdpSocket.writeDatagram(np.serialize(), receivedIdentityPackages[socket].sender, port);
 
 }
 
 void LanLinkProvider::connected()
 {
 
-    QTcpSocket* socket = (QTcpSocket*)sender();
+    QTcpSocket* socket = qobject_cast<QTcpSocket*>(sender());
 
     disconnect(socket, SIGNAL(connected()), this, SLOT(connected()));
     disconnect(socket, SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(connectError()));
 
     configureSocket(socket);
 
-    NetworkPackage* np = receivedIdentityPackages[socket].np;
+    NetworkPackage* receivedPackage = receivedIdentityPackages[socket].np;
-    const QString& id = np->get<QString>("deviceId");
+    const QString& deviceId = receivedPackage->get<QString>("deviceId");
     //kDebug(kdeconnect_kded()) << "Connected" << socket->isWritable();
 
-    LanDeviceLink* dl = new LanDeviceLink(id, this, socket);
+    LanDeviceLink* deviceLink = new LanDeviceLink(deviceId, this, socket);
 
     NetworkPackage np2("");
     NetworkPackage::createIdentityPackage(&np2);
 
-    bool success = dl->sendPackage(np2);
+    bool success = deviceLink->sendPackage(np2);
 
     //TODO: Use reverse connection too to preffer connecting a unstable device (a phone) to a stable device (a computer)
     if (success) {
 
         //kDebug(kdeconnect_kded()) << "Handshaking done (i'm the existing device)";
 
-        connect(dl, SIGNAL(destroyed(QObject*)),
+        connect(deviceLink, SIGNAL(destroyed(QObject*)),
                 this, SLOT(deviceLinkDestroyed(QObject*)));
 
-        Q_EMIT onConnectionReceived(*np, dl);
+        Q_EMIT onConnectionReceived(*receivedPackage, deviceLink);
 
-        QMap< QString, DeviceLink* >::iterator oldLinkIterator = links.find(id);
+        //We kill any possible link from this same device
-        if (oldLinkIterator != links.end()) {
+        QMap< QString, DeviceLink* >::iterator oldLinkIterator = mLinks.find(deviceId);
+        if (oldLinkIterator != mLinks.end()) {
             DeviceLink* oldLink = oldLinkIterator.value();
             disconnect(oldLink, SIGNAL(destroyed(QObject*)),
                         this, SLOT(deviceLinkDestroyed(QObject*)));
             oldLink->deleteLater();
-            links.erase(oldLinkIterator);
+            mLinks.erase(oldLinkIterator);
         }
 
-        links[id] = dl;
+        mLinks[deviceId] = deviceLink;
 
     } else {
         //I think this will never happen
         kDebug(kdeconnect_kded()) << "Fallback (2), try reverse connection";
-        QUdpSocket().writeDatagram(np2.serialize(), receivedIdentityPackages[socket].sender, port);
+        mUdpSocket.writeDatagram(np2.serialize(), receivedIdentityPackages[socket].sender, port);
-        delete dl;
+        delete deviceLink;
     }
 
     receivedIdentityPackages.remove(socket);
 
-    delete np;
+    delete receivedPackage;
 
 }
 
@@ -232,57 +237,56 @@ void LanLinkProvider::newConnection()
 //I'm the new device and this is the answer to my UDP introduction (data received)
 void LanLinkProvider::dataReceived()
 {
-    QTcpSocket* socket = (QTcpSocket*) QObject::sender();
+    QTcpSocket* socket = qobject_cast<QTcpSocket*>(sender());
     configureSocket(socket);
 
-    QByteArray data = socket->readLine();
+    const QByteArray data = socket->readLine();
 
     //kDebug(kdeconnect_kded()) << "LanLinkProvider received reply:" << data;
 
     NetworkPackage np("");
     bool success = NetworkPackage::unserialize(data, &np);
 
-    if (success && np.type() == PACKAGE_TYPE_IDENTITY) {
+    if (!success || np.type() != PACKAGE_TYPE_IDENTITY) {
+        kDebug(kdeconnect_kded()) << "LanLinkProvider/newConnection: Not an identification package (wuh?)";
+        return;
+    }
 
-        const QString& id = np.get<QString>("deviceId");
+    const QString& deviceId = np.get<QString>("deviceId");
-        LanDeviceLink* dl = new LanDeviceLink(id, this, socket);
+    LanDeviceLink* deviceLink = new LanDeviceLink(deviceId, this, socket);
 
     //kDebug(kdeconnect_kded()) << "Handshaking done (i'm the new device)";
 
-        connect(dl, SIGNAL(destroyed(QObject*)),
+    connect(deviceLink, SIGNAL(destroyed(QObject*)),
             this, SLOT(deviceLinkDestroyed(QObject*)));
 
-        Q_EMIT onConnectionReceived(np, dl);
+    Q_EMIT onConnectionReceived(np, deviceLink);
 
-        QMap< QString, DeviceLink* >::iterator oldLinkIterator = links.find(id);
+    QMap< QString, DeviceLink* >::iterator oldLinkIterator = mLinks.find(deviceId);
-        if (oldLinkIterator != links.end()) {
+    if (oldLinkIterator != mLinks.end()) {
         DeviceLink* oldLink = oldLinkIterator.value();
         disconnect(oldLink, SIGNAL(destroyed(QObject*)),
                     this, SLOT(deviceLinkDestroyed(QObject*)));
         oldLink->deleteLater();
-            links.erase(oldLinkIterator);
+        mLinks.erase(oldLinkIterator);
     }
 
-        links[id] = dl;
+    mLinks[deviceId] = deviceLink;
 
     disconnect(socket,SIGNAL(readyRead()),this,SLOT(dataReceived()));
-
-    } else {
-        kDebug(kdeconnect_kded()) << "LanLinkProvider/newConnection: Not an identification package (wuh?)";
-    }
-
 }
 
 void LanLinkProvider::deviceLinkDestroyed(QObject* uncastedDeviceLink)
 {
     //kDebug(kdeconnect_kded()) << "deviceLinkDestroyed";
 
-    DeviceLink* deviceLink = (DeviceLink*)uncastedDeviceLink;
+    DeviceLink* deviceLink = qobject_cast<DeviceLink*>(uncastedDeviceLink);
-    const QString& id = deviceLink->deviceId();
+    Q_ASSERT(deviceLink);
 
-    QMap< QString, DeviceLink* >::iterator oldLinkIterator = links.find(id);
+    const QString& id = deviceLink->deviceId();
-    if (oldLinkIterator != links.end() && oldLinkIterator.value() == deviceLink) {
+    QMap< QString, DeviceLink* >::iterator oldLinkIterator = mLinks.find(id);
-        links.erase(oldLinkIterator);
+    if (oldLinkIterator != mLinks.end() && oldLinkIterator.value() == deviceLink) {
+        mLinks.erase(oldLinkIterator);
     }
 
 }

kded/backends/lan/lanlinkprovider.h

@@ -58,10 +58,11 @@ private:
 
     QTcpServer* mTcpServer;
     QUdpSocket* mUdpServer;
+    QUdpSocket mUdpSocket;
     const static quint16 port = 1714;
-    quint16 tcpPort;
+    quint16 mTcpPort;
 
-    QMap<QString, DeviceLink*> links;
+    QMap<QString, DeviceLink*> mLinks;
 
     struct PendingConnect {
         NetworkPackage* np;