Skip to content

fix: Fix get device info fail when init#139

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
wangrong1069:pr-0318
Mar 18, 2025
Merged

fix: Fix get device info fail when init#139
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
wangrong1069:pr-0318

Conversation

@wangrong1069
Copy link
Contributor

When deepin-diskmanager starts, it will start deepin-diskmanager-service first, and then obtain device information from deepin-diskmanager-service. If deepin-diskmanager-service takes too long to initialize due to the existence of external storage devices, deepin-diskmanager will fail to obtain device information. At this time, deepin-diskmanager-service has not registered the dbus service.
Let's re-initiate the request after failing to obtain device information.

Log: Fix get device info fail when init
Bug: https://pms.uniontech.com/bug-view-308533.html

When deepin-diskmanager starts, it will start deepin-diskmanager-service
first, and then obtain device information from deepin-diskmanager-service.
If deepin-diskmanager-service takes too long to initialize due to the
existence of external storage devices, deepin-diskmanager will fail to
obtain device information. At this time, deepin-diskmanager-service has
not registered the dbus service.
Let's re-initiate the request after failing to obtain device information.

Log: Fix get device info fail when init
Bug: https://pms.uniontech.com/bug-view-308533.html
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 代码重复getAllDevice 函数中使用了 QDBusPendingCallWatcher 来监视 getalldevice 的调用结果,但在 getDeviceInfo 函数中直接调用了 getAllDevice。如果 getDeviceInfo 也需要监视 getalldevice 的结果,那么 getAllDevice 函数中的代码应该被提取到一个公共函数中,以避免代码重复。

  2. 错误处理:在 getAllDevice 函数中,当 reply.isError()true 时,使用了 QTimer::singleShot(500, this, SLOT(getAllDevice())); 来重新调用 getAllDevice。这种递归调用可能会导致栈溢出,应该设置一个最大重试次数,或者使用循环来限制重试次数。

  3. 信号与槽的连接:在 QObject::connect 中使用了 lambda 表达式,这是好的做法,因为它可以避免使用全局变量。但是,如果 watcher 对象的生命周期比 QObject::connect 的生命周期长,那么在 watcher 对象被删除后,lambda 表达式中的 watcher 可能仍然会引用一个无效的指针,导致未定义行为。应该确保 watcher 对象的生命周期与 QObject::connect 的生命周期一致。

  4. 日志输出:在 getAllDevice 函数中,当 reply.isError()true 时,使用了 qDebug() 来输出错误信息。在生产环境中,应该使用更合适的日志记录机制,例如 QLoggingCategory,以便更好地控制日志级别和输出格式。

  5. 注释:在 getAllDevice 函数的注释中,应该说明重试机制的具体逻辑,例如重试次数和重试间隔。

  6. 代码风格:在 getAllDevice 函数中,QDBusPendingCallWatcher 对象的创建和删除应该使用智能指针(例如 QScopedPointerstd::unique_ptr),以避免内存泄漏。

  7. 函数命名getAllDevice 函数的命名不够明确,建议使用更具描述性的名称,例如 requestAllDevicesfetchDeviceList

  8. 错误处理:在 getAllDevice 函数中,当 reply.isError()true 时,只是简单地输出了错误信息并重试。应该考虑更复杂的错误处理逻辑,例如记录错误日志、通知用户等。

  9. 代码组织getAllDevice 函数中的 lambda 表达式可以提取到一个单独的函数中,以提高代码的可读性和可维护性。

  10. 线程安全:如果 getAllDevice 函数可能会在多个线程中同时调用,那么需要确保 m_dbus 对象的访问是线程安全的。可以考虑使用互斥锁(QMutex)来保护对 m_dbus 的访问。

综上所述,代码中存在一些潜在的问题,需要进一步优化和改进。

@github-actions
Copy link

TAG Bot

TAG: 6.0.2
EXISTED: no
DISTRIBUTION: unstable

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, wangrong1069

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

Details Needs approval from an approver in each of these files:

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

@wangrong1069
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Mar 18, 2025

This pr force merged! (status: unstable)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants