fix: 修复内存泄露

[ut003640]

使用lamdba表达式捕获智能指针，会导致该智能指针引用计数+1，引起对象无法释放，导致不断占用dbus资源，当达到一定的数量后，资源不可用，无法收到dbus信号

Log: 修复内存泄露
Influence: 任务栏网络状态
PMS: BUG-301423

[sourcery-ai]

Sorry @ut003640, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[deepin-ci-robot]

deepin pr auto review

这段代码主要是对 NetworkManagerProcesser 类进行重构，将原本在 onDeviceAdded 函数内部的 Lambda 表达式（如 deviceExist, createDevice, deviceCreateOrRemove）提取为类的成员函数。这是一个很好的重构方向，提高了代码的可读性和复用性。

以下是对这段 diff 的详细审查意见，包括语法逻辑、代码质量、代码性能和代码安全四个方面：

1. 语法逻辑

• 逻辑正确性：重构后的逻辑与原 Lambda 表达式保持一致。将设备添加、移除、创建的逻辑提取出来，使得 onDeviceAdded 函数更加清晰，专注于信号处理和初始化连接。
• Lambda 捕获的变更：
  • 原代码：connect(..., [ currentDevice, deviceCreateOrRemove ] { ... })。这里捕获了 currentDevice 的强引用 Ptr（智能指针），保证了在 Lambda 执行时设备对象不会被销毁。
  • 新代码：connect(..., [ uni, this ] { createOrRemoveDevice(uni); })。这里捕获了 uni (QString) 和 this 指针。
  • 潜在风险：在 createOrRemoveDevice 内部，第一行是 NetworkManager::Device::Ptr device = NetworkManager::findNetworkInterface(path);。如果 NetworkManager 的底层实现中，findNetworkInterface 依赖于某种全局状态或者缓存，且在信号触发时该设备已被彻底移除（不仅仅是状态改变），可能会导致 device.isNull() 为 true，从而提前返回。不过，考虑到这是响应 interfaceFlagsChanged 或 managedChanged 信号，设备通常应该还存在，逻辑上是合理的。但相比原来直接持有 currentDevice 智能指针的写法，新的写法稍微增加了一点查找设备的开销和不确定性（尽管很小）。
• 重复逻辑：在 onDeviceAdded 的末尾（第 348-353 行），依然有一段创建设备并发射信号的代码：

  if (newDevice) {
      m_devices << newDevice;
      // ... 发射信号 ...
      if (m_hotspotController) {
          m_hotspotController->updateDevices(m_devices);
      }
  }

  这段逻辑与 createOrRemoveDevice 函数中添加设备的逻辑几乎完全一致。虽然 onDeviceAdded 是初次添加，而 createOrRemoveDevice 是状态变更时的处理，但在 onDeviceAdded 中直接调用 createOrRemoveDevice(uni) 可能会更简洁，前提是 createOrRemoveDevice 内部的逻辑能够兼容“设备已存在则不重复添加”的判断（目前代码中有 if (!deviceExist(device->uni())) 判断，所以是兼容的）。
  建议：可以考虑在 onDeviceAdded 的最后直接调用 createOrRemoveDevice(uni) 来替代重复的代码块，或者将公共逻辑进一步封装。

2. 代码质量

• 可读性：显著提升。将复杂的 Lambda 逻辑转换为具名的成员函数（createOrRemoveDevice, deviceExist, createDevice），使得函数名即文档，代码意图一目了然。
• 可维护性：提升。如果需要修改设备创建或移除的逻辑，现在只需要修改 createOrRemoveDevice 这一处，而不需要在 Lambda 中修改。
• 代码风格：符合 Qt/C++ 的常规命名规范。使用了 nullptr 代替 NULL 或 0（虽然部分地方仍用了 Q_NULLPTR，这是兼容旧版 Qt 的写法，可以接受）。

3. 代码性能

• 查找开销：
  • deviceExist 函数使用了线性遍历 m_devices 列表：for (NetworkDeviceBase *device : m_devices)。
  • 在 createOrRemoveDevice 的移除逻辑中也使用了线性遍历查找。
  • 建议：如果 m_devices 列表中的设备数量非常多（虽然通常网卡数量很少，一般 < 10），频繁的线性查找可能会影响性能。考虑到设备数量通常很少，目前的性能是可以接受的。但如果这是一个通用框架，可以考虑使用 QMap<QString, NetworkDeviceBase*> 或 QHash 来存储设备，以 path 为 key，将查找复杂度从 O(N) 降为 O(1)。
• 信号连接：新代码在 onDeviceAdded 中连接信号时，捕获了 [ uni, this ]。这比捕获智能指针更轻量一些，但也依赖于 NetworkManager::findNetworkInterface 的查找效率。

4. 代码安全

• 内存管理：
  • 代码中使用了 NetworkManager::Device::Ptr（通常是 QSharedPointer），管理得当。
  • 在移除设备时使用了 rmDevice->deleteLater()，这是 Qt 中安全的删除对象的方式，避免了在事件循环处理过程中直接删除对象导致崩溃。
  • rmDevice = nullptr; 这一行在 deleteLater 之后是多余的，因为 rmDevice 是栈上的局部指针变量，函数结束后自动销毁，置空没有实际安全意义，但也不算错误。
• 空指针检查：
  • createOrRemoveDevice 开头检查了 device.isNull()。
  • createDevice 返回值检查了 if (newDevice)。
  • m_hotspotController 使用前检查了 if (m_hotspotController)。
  • 这些检查都做得很好，防止了空指针解引用。
• Lambda 捕获 this：
  • connect(..., [ uni, this ] { ... }) 捕获了 this 指针。如果 NetworkManagerProcesser 对象在信号触发前被销毁，这会导致崩溃。
  • 分析：onDeviceAdded 是成员函数，通常由信号槽机制调用。如果 NetworkManagerProcesser 被销毁，它通常会断开所有连接。但在多线程或极其复杂的析构顺序中，仍需注意。不过，这是 Qt 信号槽的常规用法，通常认为是安全的。
• 类型转换：
  • device.staticCast<NetworkManager::WirelessDevice>()：使用 staticCast 在 QSharedPointer 之间进行转换，前提是类型确实兼容。由于前面有 if (device->type() == ...) 判断，这里是安全的。

改进建议总结

1. 消除重复代码：
   在 onDeviceAdded 函数的末尾，建议直接调用 createOrRemoveDevice(uni) 来替代手动添加设备并发射信号的代码块。这样可以确保设备添加的逻辑（包括热点更新、排序等）完全统一，避免未来修改一处而忘记修改另一处。

   修改建议：

   // 在 onDeviceAdded 末尾
   // 替换原来的 if (newDevice) { ... } 代码块
   createOrRemoveDevice(uni);

2. 考虑使用哈希表存储设备（可选，视设备规模而定）：
   如果未来可能管理大量虚拟设备或特殊场景，建议将 QList<NetworkDeviceBase*> m_devices 改为 QMap<QString, NetworkDeviceBase*> m_devices 或 QHash，以优化 deviceExist 和移除设备时的查找性能。

3. Lambda 捕获的安全性：
   虽然目前的写法是标准的，但为了防止 this 指针失效，可以确保在析构函数中正确断开与 NetworkManager 全局单例的连接，或者使用 QPointer (如果 NetworkManagerProcesser 继承自 QObject) 来保护 Lambda 中的 this（尽管在 connect 中通常不需要，因为 QObject 会自动处理连接断开，但在手动捕获 this 的 Lambda 中需留意）。

4. 常量引用：
   在 deviceExist(const QString &path) 中，参数 path 是按值传递的。QString 的隐式共享机制使得按值传递开销不大（写时复制），但考虑到这是一个查找函数，且 QString 可能被频繁调用，改为 const QString &path 是更符合 C++ 最佳实践的做法，可以避免不必要的引用计数增减。

   修改建议：

   bool NetworkManagerProcesser::deviceExist(const QString &path) const

   改为：

   bool NetworkManagerProcesser::deviceExist(const QString &path) const // 保持不变，或者改为 const QString &path

   （注：Qt 中 QString 传值通常也是被接受的，但传 const ref 在高频调用下略优）。

结论

这段代码重构整体上是优秀的，极大地提高了代码的可读性和可维护性。主要的改进点在于消除 onDeviceAdded 末尾的重复逻辑，以及微小的性能优化建议。代码逻辑严密，安全性考虑周全。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mhduiy, ut003640

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ut003640]

/forcemerge

[deepin-bot]

This pr force merged! (status: unstable)