Refactoring:

* Some const correctness fixes. * Changed some destructors to virtual which are used in inheritance. * Fixed memory leak from OrientationChain. * Removed SocketHandler::killSocket() hack. QObject::deleteLater() is used to clean up sockets. * Replaced some magic #defines with static consts. * Some simplifications to the code.
sailfishos · Nov 29, 2010 · 798e408 · 798e408
1 parent 489efd6
commit 798e408
Show file tree

Hide file tree

Showing 25 changed files with 69 additions and 112 deletions.
diff --git a/chains/orientationchain/orientationchain.cpp b/chains/orientationchain/orientationchain.cpp
@@ -31,8 +31,6 @@
 OrientationChain::OrientationChain(const QString& id) :
     AbstractChain(id)
 {
-    qRegisterMetaType<TimedUnsigned>("TimedUnsigned");
-    qRegisterMetaType<PoseData>("PoseData");
     SensorManager& sm = SensorManager::instance();
 
     accelerometerChain_ = sm.requestChain("accelerometerchain");
@@ -87,6 +85,7 @@ OrientationChain::~OrientationChain()
     delete accelerometerReader_;
     delete topEdgeOutput_;
     delete faceOutput_;
+    delete orientationOutput_;
     delete filterBin_;
 }
 

diff --git a/datatypes/utils.cpp b/datatypes/utils.cpp
@@ -31,10 +31,12 @@
 #include "xyz.h"
 #include "magneticfield.h"
 #include "unsigned.h"
+#include "timedunsigned.h"
 #include "compass.h"
 #include "orientation.h"
 #include "datarange.h"
 #include "tap.h"
+#include "posedata.h"
 
 void __attribute__ ((constructor)) datatypes_init(void)
 {
@@ -48,6 +50,8 @@ void __attribute__ ((constructor)) datatypes_init(void)
     qDBusRegisterMetaType<DataRangeList>();
     qDBusRegisterMetaType<IntegerRange>();
     qDBusRegisterMetaType<IntegerRangeList>();
+    qRegisterMetaType<TimedUnsigned>();
+    qRegisterMetaType<PoseData>();
 }
 
 void __attribute__ ((destructor)) datatypes_fini(void)

diff --git a/debian/changelog b/debian/changelog
@@ -20,6 +20,11 @@ sensord (0.6.32) unstable; urgency=low
   * Implemented buffering with few unit test cases.
   * Replaced old index based accessors for available dataranges and intervals
     with methods which query list of data with single dbus call.
+  * Some const correctness fixes.
+  * Changed some destructors to virtual which are used in inheritance.
+  * Fixed memory leak from OrientationChain.
+  * Removed SocketHandler::killSocket() hack. QObject::deleteLater() is used
+    to clean up sockets.
 
  -- Timo Rongas <ext-timo.2.rongas@nokia.com>  Wed, 10 Nov 2010 11:54:31 +0200
 

diff --git a/filters/orientationinterpreter/orientationinterpreter.cpp b/filters/orientationinterpreter/orientationinterpreter.cpp
@@ -9,7 +9,6 @@
    @author Timo Rongas <ext-timo.2.rongas@nokia.com>
    @author Lihan Guo <lihan.guo@digia.com>
 
-
    This file is part of Sensord.
 
    Sensord is free software; you can redistribute it and/or modify
@@ -29,22 +28,8 @@
 #include "orientationinterpreter.h"
 #include <sensord/logging.h>
 #include <math.h>
+#include <stdlib.h>
 #include "config.h"
-#include <QDebug>
-
-#define DEFAULT_THRESHOLD 250
-#define RADIANS_TO_DEGREES 180.0/M_PI
-#define ANGLE_LIMIT 45
-#define SAME_AXIS_LIMIT 5
-
-#define OVERFLOW_MIN 800
-#define OVERFLOW_MAX 1250
-
-#define THRESHOLD_LANDSCAPE  25
-#define THRESHOLD_PORTRAIT  20
-
-#define DISCARD_TIME 750000
-#define AVG_BUFFER_MAX_SIZE 10
 
 OrientationInterpreter::OrientationInterpreter() :
         accDataSink(this, &OrientationInterpreter::accDataAvailable),
@@ -89,7 +74,7 @@ void OrientationInterpreter::accDataAvailable(unsigned, const AccelerationData*
     // Avoids bloating when timestamps don't make sense
     if (dataBuffer.count() > AVG_BUFFER_MAX_SIZE)
     {
-        dataBuffer.erase(dataBuffer.begin(), dataBuffer.end()-10);
+        dataBuffer.erase(dataBuffer.begin(), dataBuffer.end() - 10);
     }
 
     // Clear old values from buffer.
@@ -175,13 +160,10 @@ void OrientationInterpreter::processTopEdge()
 
 void OrientationInterpreter::processFace()
 {
-
-    PoseData newFace;
-
-
-    if  (abs(data.z_) >= 300)
+    if (abs(data.z_) >= 300)
     {
-        newFace.orientation_ = ((data.z_>=0)? PoseData::FaceDown : PoseData::FaceUp);
+        PoseData newFace;
+        newFace.orientation_ = ((data.z_ >= 0) ? PoseData::FaceDown : PoseData::FaceUp);
 
         if (newFace.orientation_ == PoseData::FaceDown)
         {
@@ -195,8 +177,7 @@ void OrientationInterpreter::processFace()
             face.orientation_ = PoseData::FaceUp;
         }
 
-
-       if (face.orientation_ != previousFace.orientation_)
+        if (face.orientation_ != previousFace.orientation_)
         {
             previousFace.orientation_ = face.orientation_;
             face.timestamp_ = data.timestamp_;
@@ -205,7 +186,6 @@ void OrientationInterpreter::processFace()
     }
 }
 
-
 void OrientationInterpreter::processOrientation()
 {
     PoseData newPose;

diff --git a/filters/orientationinterpreter/orientationinterpreter.h b/filters/orientationinterpreter/orientationinterpreter.h
@@ -28,7 +28,7 @@
 #define ORIENTATIONINTERPRETER_H
 
 #include <QObject>
-#include <QTimer>
+#include <math.h>
 #include <sensord/filter.h>
 #include <datatypes/orientationdata.h>
 #include <datatypes/posedata.h>
@@ -81,6 +81,20 @@ class OrientationInterpreter : public QObject, public PropertyTracker, public Fi
 
     PoseData o_;
 
+    static const int DEFAULT_THRESHOLD = 250;
+    static const float RADIANS_TO_DEGREES = 180.0/M_PI;
+    static const int ANGLE_LIMIT = 45;
+    static const int SAME_AXIS_LIMIT = 5;
+
+    static const int OVERFLOW_MIN = 800;
+    static const int OVERFLOW_MAX = 1250;
+
+    static const int THRESHOLD_LANDSCAPE = 25;
+    static const int THRESHOLD_PORTRAIT = 20;
+
+    static const int DISCARD_TIME = 750000;
+    static const int AVG_BUFFER_MAX_SIZE = 10;
+
 public:
     /**
      * Factory method.

diff --git a/sensord/dbusemitter.h b/sensord/dbusemitter.h
@@ -43,7 +43,7 @@ class DbusEmitter : public RingBufferReader<TYPE>
         addSource(&source_, "source");
     }
 
-    ~DbusEmitter()
+    virtual ~DbusEmitter()
     {
         delete[] chunk_;
     }

diff --git a/sensord/ringbuffer.h b/sensord/ringbuffer.h
@@ -54,6 +54,7 @@ friend class RingBuffer<TYPE>;
 
 public:
     RingBufferReader() : readCount_(0) {}
+    virtual ~RingBufferReader() {};
 
     int typeId() { return 0; } // TODO
 

diff --git a/sensord/sockethandler.cpp b/sensord/sockethandler.cpp
@@ -245,16 +245,7 @@ bool SocketHandler::removeSession(int sessionId)
     if (socket) {
         disconnect(socket, SIGNAL(readyRead()), this, SLOT(socketReadable()));
         disconnect(socket, SIGNAL(disconnected()), this, SLOT(socketDisconnected()));
-
-        // NOTE NOTE NOTE NOTE NOTE NOTE KLUDGE KLUDGE
-        // Due to some timing issues that we have not been able to figure out,
-        // deleting the socket right away does not work if the session is closed
-        // after a client has been lost. Thus, socket deletion is pushed forward
-        // 2 seconds to allow all Qt internal async stuff to finish..
-        //delete socket;
-        // NOTE NOTE NOTE NOTE NOTE NOTE KLUDGE KLUDGE
-        m_tmpSocks.append(socket);
-        QTimer::singleShot(2000, this, SLOT(killSocket()));
+        socket->deleteLater();
     }
 
     delete m_idMap.take(sessionId);
@@ -318,16 +309,6 @@ void SocketHandler::socketDisconnected()
     emit lostSession(sessionId);
 }
 
-void SocketHandler::killSocket()
-{
-    if (m_tmpSocks.size() > 0) {
-        sensordLogT() << "[SocketHandler]: Deleting socket pointer:" << m_tmpSocks.at(0);
-        delete m_tmpSocks.takeAt(0);
-    } else {
-        sensordLogW() << "[SocketHandler]: Ugly hack just went bad.. attempting to delete nonexisting pointer.";
-    }
-}
-
 int SocketHandler::getSocketFd(int sessionId) const
 {
     QMap<int, SessionData*>::const_iterator it = m_idMap.find(sessionId);

diff --git a/sensord/sockethandler.h b/sensord/sockethandler.h
@@ -110,13 +110,11 @@ private slots:
     void newConnection();
     void socketReadable();
     void socketDisconnected();
-    void killSocket();
 
 private:
 
     QLocalServer*            m_server;
     QMap<int, SessionData*>  m_idMap;
-    QList<QLocalSocket*>     m_tmpSocks;
 };
 
 #endif // SOCKETHANDLER_H
diff --git a/sensors/accelerometersensor/accelerometersensor.cpp b/sensors/accelerometersensor/accelerometersensor.cpp
@@ -39,11 +39,7 @@ AccelerometerSensorChannel::AccelerometerSensorChannel(const QString& id) :
 
     accelerometerChain_ = sm.requestChain("accelerometerchain");
     Q_ASSERT( accelerometerChain_ );
-    if (!accelerometerChain_->isValid()) {
-        isValid_ = false;
-    } else {
-        isValid_ = true;
-    }
+    isValid_ = accelerometerChain_->isValid();
 
     accelerometerReader_ = new BufferReader<AccelerationData>(128);
 
@@ -79,7 +75,7 @@ AccelerometerSensorChannel::~AccelerometerSensorChannel()
     disconnectFromSource(accelerometerChain_, "accelerometer", accelerometerReader_);
 
     sm.releaseChain("accelerometerchain");
- 
+
     delete accelerometerReader_;
     delete outputBuffer_;
     delete marshallingBin_;

diff --git a/sensors/accelerometersensor/accelerometersensor.h b/sensors/accelerometersensor/accelerometersensor.h
@@ -60,7 +60,7 @@ class AccelerometerSensorChannel :
     {
         AccelerometerSensorChannel* sc = new AccelerometerSensorChannel(id);
         new AccelerometerSensorChannelAdaptor(sc);
-        
+
         return sc;
     }
 
@@ -69,24 +69,24 @@ class AccelerometerSensorChannel :
 public Q_SLOTS:
     bool start();
     bool stop();
-    
+
 signals:
     /**
      * Sent when new measurement data has become available.
      * @param data Newly measured data.
      */
     void dataAvailable(const XYZ& data);
-    
+
 protected:
     AccelerometerSensorChannel(const QString& id);
-    ~AccelerometerSensorChannel();
-    
+    virtual ~AccelerometerSensorChannel();
+
 private:
     static double                    aconv_[3][3];
     Bin*                             filterBin_;
     Bin*                             marshallingBin_;
     AbstractChain*                   accelerometerChain_;
-    BufferReader<AccelerationData>*  accelerometerReader_; 
+    BufferReader<AccelerationData>*  accelerometerReader_;
     RingBuffer<AccelerationData>*    outputBuffer_;
     AccelerationData                 previousSample_;
 

diff --git a/sensors/accelerometersensor/accelerometersensor_a.h b/sensors/accelerometersensor/accelerometersensor_a.h
@@ -28,7 +28,6 @@
 
 #include <QtDBus/QtDBus>
 
-//#include "datatypes/orientation.h"
 #include "datatypes/xyz.h"
 #include "abstractsensor_a.h"
 

diff --git a/sensors/alssensor/alssensor.h b/sensors/alssensor/alssensor.h
@@ -65,7 +65,7 @@ class ALSSensorChannel :
     {
         ALSSensorChannel* sc = new ALSSensorChannel(id);
         new ALSSensorChannelAdaptor(sc);
-        
+
         return sc;
     }
 
@@ -78,24 +78,24 @@ class ALSSensorChannel :
 public Q_SLOTS:
     bool start();
     bool stop();
-    
+
 signals:
     /**
      * Sent when a change in measured data is observed.
      * @param value Measured value.
      */
     void ALSChanged(const Unsigned& value);
-    
+
 protected:
     ALSSensorChannel(const QString& id);
-    ~ALSSensorChannel();
-    
+    virtual ~ALSSensorChannel();
+
 private:
     TimedUnsigned                 previousValue_;
     Bin*                          filterBin_;
     Bin*                          marshallingBin_;
     DeviceAdaptor*                alsAdaptor_;
-    BufferReader<TimedUnsigned>*  alsReader_; 
+    BufferReader<TimedUnsigned>*  alsReader_;
     RingBuffer<TimedUnsigned>*    outputBuffer_;
 
     void emitToDbus(const TimedUnsigned& value);

diff --git a/sensors/compasssensor/compasssensor.cpp b/sensors/compasssensor/compasssensor.cpp
@@ -39,13 +39,9 @@ CompassSensorChannel::CompassSensorChannel(const QString& id) :
 {
     SensorManager& sm = SensorManager::instance();
 
-    isValid_ = true;
-
     compassChain_ = sm.requestChain("compasschain");
     Q_ASSERT( compassChain_ );
-    if (!compassChain_->isValid()) {
-        isValid_ = false;
-    }
+    isValid_ = compassChain_->isValid();
 
     inputReader_ = new BufferReader<CompassData>(128);
 
@@ -103,15 +99,15 @@ void CompassSensorChannel::setDeclination(bool enable) {
         rb = compassChain_->findBuffer("magneticnorth");
         Q_ASSERT(rb);
         rb->unjoin(inputReader_);
-        
+
         rb = compassChain_->findBuffer("truenorth");
         Q_ASSERT(rb);
         rb->join(inputReader_);
     } else {
         rb = compassChain_->findBuffer("truenorth");
         Q_ASSERT(rb);
         rb->unjoin(inputReader_);
-        
+
         rb = compassChain_->findBuffer("magneticnorth");
         Q_ASSERT(rb);
         rb->join(inputReader_);
@@ -121,7 +117,7 @@ void CompassSensorChannel::setDeclination(bool enable) {
     signalPropertyChanged("usedeclination");
 }
 
-quint16 CompassSensorChannel::declinationAngle() { 
+quint16 CompassSensorChannel::declinationAngle() const {
     return qvariant_cast< int >
         (compassChain_->property("declinationvalue"));
 }

diff --git a/sensors/compasssensor/compasssensor.h b/sensors/compasssensor/compasssensor.h
@@ -66,7 +66,7 @@ class CompassSensorChannel :
 
     bool declination() const { return declinationCorrection_; }
     void setDeclination(bool enable);
-    quint16 declinationAngle();
+    quint16 declinationAngle() const;
 
     Compass get() const { return compassData; }
 
@@ -79,7 +79,7 @@ public Q_SLOTS:
 
 protected:
     CompassSensorChannel(const QString& id);
-    ~CompassSensorChannel();
+    virtual ~CompassSensorChannel();
 
 private:
     bool declinationCorrection_;