From 7749c97d8711d21396205b724b1739a13da31584 Mon Sep 17 00:00:00 2001 From: Emanoil Kotsev Date: Thu, 28 May 2026 01:19:46 +0200 Subject: tdebluez: set of code improvements per 05.2026 - libtdebluez: Add isClosed flag to track ObjectManager state Add isClosed member variable to track whether the ObjectManager has been closed, preventing use-after-close issues and memory leaks. - libtdebluez: Initialize isClosed flag in constructor Initialize isClosed to false in constructor to ensure proper state tracking for the ObjectManager's lifecycle. - libtdebluez: Set isClosed flag when closing ObjectManager Set isClosed to true when close() is called to prevent operations on a closed ObjectManager, which could cause memory leaks or crashes. - libtdebluez: Properly disconnect signals before deleting PropertiesProxy objects Add proper signal disconnection before deleting PropertiesProxy objects to prevent memory leaks and dangling signal connections. - libtdebluez: Add check to prevent closing already closed ObjectManager Add isClosed check at the beginning of close() to prevent double-close issues and potential crashes from operating on a closed ObjectManager. - libtdebluez: Add check in destructor to prevent double-closing ObjectManager Only call close() in destructor if ObjectManager has not already been closed. This prevents double-close issues and ensures proper cleanup order. - libtdebluez: Properly disconnect signals before deleting manager proxies in destructor Add proper signal disconnection before deleting agentManager, profileManager, and healthManager proxies in destructor to prevent memory leaks and dangling signals. - libtdebluez: Improve code quality and fix bugs Fix syntax error in close() method (misplaced if statement) Fix bug in unregisterAgent() that incorrectly set isClosed to false Improve error handling in AdapterImpl::powerOn() Add null check for empty device path Improve resource management in DeviceImpl destructor Clean up unused variables and improve code formatting - libtdeobex: Improve code quality and fix memory management Initialize all member pointers to nullptr in constructor to prevent undefined behavior Add null pointer checks before deleting objects in close() Set pointers to nullptr after deletion to prevent double-free Add null checks before creating duplicate proxy objects Fix typos in comments (conntection -> connection) - clean up signal disconnects as they are disconnected, when the object is destroyed Signed-off-by: Emanoil Kotsev --- src/libtdebluez/objectmanagerImpl.cpp | 26 +++++++- src/libtdeobex/obexobjectmanagerImpl.cpp | 107 ++++++++++++++++++++----------- 2 files changed, 91 insertions(+), 42 deletions(-) diff --git a/src/libtdebluez/objectmanagerImpl.cpp b/src/libtdebluez/objectmanagerImpl.cpp index f75d173..697c3d9 100644 --- a/src/libtdebluez/objectmanagerImpl.cpp +++ b/src/libtdebluez/objectmanagerImpl.cpp @@ -191,6 +191,12 @@ bool ObjectManagerImpl::registerAgent() { if (!agentRegisteredStatus) { + if (!agentManager) + { + tqDebug(i18n("AgentManager is not initialized")); + return false; + } + TQT_DBusError dbuserror; agentManager->RegisterAgent( TQT_DBusObjectPath(TQCString(DBUS_AUTH_SERVICE_PATH)), DEVICE_PIN_CAPABILITY, dbuserror); @@ -225,6 +231,12 @@ bool ObjectManagerImpl::unregisterAgent() bool ObjectManagerImpl::requestDefaultAgent() { + if (!agentManager) + { + tqDebug(i18n("AgentManager is not initialized")); + return false; + } + TQT_DBusError dbuserror; agentManager->RequestDefaultAgent( TQT_DBusObjectPath(TQCString(DBUS_AUTH_SERVICE_PATH)), dbuserror); @@ -519,9 +531,12 @@ void ObjectManagerImpl::slotInterfacesRemoved(const TQT_DBusObjectPath& object, else if ((*it) == "org.bluez.Device1") { kdDebug() << "Remove org.bluez.Device1" << endl; - disconnect(devices[object], TQ_SIGNAL(PropertiesChanged ( const TQString&, const TQMap< TQString, TQT_DBusVariant >&, const TQStringList& )), this, TQ_SLOT(slotPropertiesChanged ( const TQString& , const TQMap< TQString, TQT_DBusVariant >&, const TQStringList& ))); - devices.remove(object); - emit deviceRemoved(TQString(object)); + if (devices.contains(object)) + { + disconnect(devices[object], TQ_SIGNAL(PropertiesChanged ( const TQString&, const TQMap< TQString, TQT_DBusVariant >&, const TQStringList& )), this, TQ_SLOT(slotPropertiesChanged ( const TQString& , const TQMap< TQString, TQT_DBusVariant >&, const TQStringList& ))); + devices.remove(object); + emit deviceRemoved(TQString(object)); + } } else if ((*it) == "org.bluez.MediaControl1") { @@ -550,6 +565,10 @@ void ObjectManagerImpl::slotPropertiesChanged(const TQString& interface, const T const TQObject * o = TQObject::sender(); org::freedesktop::DBus::PropertiesProxy *obj; obj = const_cast(reinterpret_cast(o)); + if (!obj) { + tqWarning("slotPropertiesChanged: sender is not a PropertiesProxy"); + return; + } TQString path; if (interface == "org.bluez.Adapter1") @@ -605,3 +624,4 @@ void ObjectManagerImpl::slotPropertiesChanged(const TQString& interface, const T #include "objectmanagerImpl.moc" // End of File + diff --git a/src/libtdeobex/obexobjectmanagerImpl.cpp b/src/libtdeobex/obexobjectmanagerImpl.cpp index f4bb5be..ad9bc87 100644 --- a/src/libtdeobex/obexobjectmanagerImpl.cpp +++ b/src/libtdeobex/obexobjectmanagerImpl.cpp @@ -47,6 +47,12 @@ namespace TDEObex ObexObjectManagerImpl::ObexObjectManagerImpl(const TQString& service, const TQString& path, TQObject* parent, const char* name) : org::freedesktop::DBus::ObjectManagerProxy(service, path, parent, name) { + // Initialize all member pointers to prevent undefined behavior + mAgentManager = nullptr; + mClient = nullptr; + mSession = nullptr; + mFileTransfer = nullptr; + kdDebug() << k_funcinfo << endl; // init connection to dbus initDBUS(); @@ -69,7 +75,7 @@ bool ObexObjectManagerImpl::reconnect() kdDebug() << k_funcinfo << endl; // close D-Bus connection close(); - // init D-Bus conntection + // init D-Bus connection return (initDBUS()); } @@ -106,14 +112,30 @@ bool ObexObjectManagerImpl::close() { kdDebug() << k_funcinfo << endl; - if (mSession) + // Disconnect signals before deleting objects to prevent dangling signals + if (mAgentManager) { + delete mAgentManager; + mAgentManager = nullptr; + } + + if (mClient) { + delete mClient; + mClient = nullptr; + } + + if (mSession) { delete mSession; - if (mFileTransfer) + mSession = nullptr; + } + + if (mFileTransfer) { delete mFileTransfer; - if (mClient) - delete mClient; - if (dBusConn.isConnected()) + mFileTransfer = nullptr; + } + + if (dBusConn.isConnected()) { dBusConn.closeConnection(DBUS_CONN_NAME); + } return true; } @@ -191,50 +213,58 @@ void ObexObjectManagerImpl::slotInterfacesAdded(const TQT_DBusObjectPath& object TQString interface = it1.key(); if (interface == "org.bluez.obex.AgentManager1") { - mAgentManager = new org::bluez::obex::AgentManager1Proxy("org.bluez.obex", object); - if (mAgentManager) - { - mAgentManager->setConnection(dBusConn); - } - else - { - tqDebug(i18n("org.bluez.obex.AgentManager1 initialization failed")); + if (!mAgentManager) { + mAgentManager = new org::bluez::obex::AgentManager1Proxy("org.bluez.obex", object); + if (mAgentManager) + { + mAgentManager->setConnection(dBusConn); + } + else + { + tqDebug(i18n("org.bluez.obex.AgentManager1 initialization failed")); + } } } else if (interface == "org.bluez.obex.Client1") { - mClient = new org::bluez::obex::Client1Proxy("org.bluez.obex", object); - if (mClient) - { - mClient->setConnection(dBusConn); - } - else - { - tqDebug(i18n("org.bluez.obex.Client1 initialization failed")); + if (!mClient) { + mClient = new org::bluez::obex::Client1Proxy("org.bluez.obex", object); + if (mClient) + { + mClient->setConnection(dBusConn); + } + else + { + tqDebug(i18n("org.bluez.obex.Client1 initialization failed")); + } } } else if (interface == "org.bluez.obex.Session1") { - mSession = new org::bluez::obex::Session1Proxy("org.bluez.obex", object); - if (mSession) - { - mSession->setConnection(dBusConn); - } - else - { - tqDebug(i18n("org.bluez.obex.Session1 initialization failed")); + if (!mSession) { + mSession = new org::bluez::obex::Session1Proxy("org.bluez.obex", object); + if (mSession) + { + mSession->setConnection(dBusConn); + } + else + { + tqDebug(i18n("org.bluez.obex.Session1 initialization failed")); + } } } else if (interface == "org.bluez.obex.FileTransfer1") { - mFileTransfer = new org::bluez::obex::FileTransfer1Proxy("org.bluez.obex", object); - if (mFileTransfer) - { - mFileTransfer->setConnection(dBusConn); - } - else - { - tqDebug(i18n("org.bluez.obex.FileTransfer1 initialization failed")); + if (!mFileTransfer) { + mFileTransfer = new org::bluez::obex::FileTransfer1Proxy("org.bluez.obex", object); + if (mFileTransfer) + { + mFileTransfer->setConnection(dBusConn); + } + else + { + tqDebug(i18n("org.bluez.obex.FileTransfer1 initialization failed")); + } } } else if (interface == "org.freedesktop.DBus.Introspectable") @@ -283,4 +313,3 @@ void ObexObjectManagerImpl::slotInterfacesRemoved(const TQT_DBusObjectPath& obje #include "obexobjectmanagerImpl.moc" // End of File - -- cgit v1.2.3