Skip to content

fix(installer): add partition device existence check to handle physic…#119

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
pppanghu77:bug1
May 12, 2026
Merged

fix(installer): add partition device existence check to handle physic…#119
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
pppanghu77:bug1

Conversation

@pppanghu77
Copy link
Copy Markdown

…al USB removal

  • Add QFile::exists() check for partition device in QtBaseInstaller::checkError() before disk lookup to handle physical USB removal gracefully

修复(installer): 增加分区设备存在性检查以处理 USB 物理拔出场景

  • 在 QtBaseInstaller::checkError() 中调用磁盘查找前增加 QFile::exists() 检查分区设备是否存在,优雅处理 USB 被物理拔出的情况

Log: 在安装器错误检查流程中增加分区设备存在性检查,防止 USB 被物理拔出后继续执行导致异常
Bug: https://pms.uniontech.com/bug-view-354921.html

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Please try again later or upgrade to continue using Sourcery

…al USB removal

- Add QFile::exists() check for partition device in QtBaseInstaller::checkError() before disk lookup to handle physical USB removal gracefully

修复(installer): 增加分区设备存在性检查以处理 USB 物理拔出场景

- 在 QtBaseInstaller::checkError() 中调用磁盘查找前增加 QFile::exists() 检查分区设备是否存在,优雅处理 USB 被物理拔出的情况

Log: 在安装器错误检查流程中增加分区设备存在性检查,防止 USB 被物理拔出后继续执行导致异常
Bug: https://pms.uniontech.com/bug-view-354921.html
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX,你的智能编程助手。我已经仔细审查了你提供的 Git Diff 输入。

本次代码修改主要包含两部分:一是更新了版权声明中的年份范围;二是在 checkError 函数中新增了对分区设备是否存在的检查,以处理物理 USB 被拔出的异常场景。

整体来看,这个修改的逻辑方向是正确的,提前拦截了设备不存在的情况,避免了后续调用 GetPartitionDisk 时可能出现的错误。但针对语法逻辑、代码质量、代码性能和代码安全,我提出以下改进意见:

1. 语法与逻辑

  • 拼写错误:变量名 m_strPartionNamePartion 存在拼写错误,正确应为 Partition(即 m_strPartitionName)。虽然这是历史遗留问题,但建议在后续的重构中统一修正。
  • 竞态条件QFile::exists(m_strPartionName) 的检查与后续 XSys::DiskUtil::GetPartitionDisk(m_strPartionName) 的调用之间存在时间差。在极端情况下,检查时设备存在,但在获取磁盘信息前设备被拔出,依然可能导致后续逻辑异常。因此,后续的 GetPartitionDisk 等底层调用仍需具备处理设备不存在(或返回空/错误)的容错能力。

2. 代码质量

  • 日志信息不规范qCritical() << "Error::get(Error::USBMountFailed) device removed:" << m_strPartionName; 这行日志中,Error::get(Error::USBMountFailed) 看起来像是想调用某个静态方法获取错误描述,但实际并没有调用,而是直接输出了字面量。这会导致日志可读性差。
    • 建议:如果 Error::get 是获取错误描述的方法,应改为 Error::get(Error::USBMountFailed) 的实际调用结果;如果只是想标记错误类型,建议改为更自然的描述,例如:"Device removed, causing USBMountFailed:"
  • 状态一致性:在 emit progressfinished 信号时,使用了 m_progressStatus。需要确认在设备拔出导致错误时,m_progressStatus 的当前值是否符合预期,是否需要在此处重置或赋值为一个特定的“失败/中断”状态,以避免前端 UI 状态显示异常。

3. 代码性能

  • IO 操作的开销QFile::exists 底层会调用操作系统的 stat 系统调用,这是一个阻塞的磁盘 I/O 操作。如果 checkError 函数是在主线程(UI线程)中被调用,或者被高频轮询调用,可能会引起微小的卡顿。
    • 建议:如果这是高频调用的轮询检查,建议将相关逻辑放入工作线程,或者考虑使用 QFileSystemWatcher 等事件驱动机制来监听设备的插拔,而不是依赖 QFile::exists 主动轮询。

4. 代码安全

  • 符号链接安全QFile::exists 默认会跟随符号链接。如果 m_strPartionName 指向的路径被恶意替换为指向其他敏感分区或文件的符号链接,QFile::exists 会返回 true,从而绕过这个 USB 拔出的检查逻辑。
    • 建议:如果此安装器对设备路径的真实性有严格要求,建议在检查前先判断是否为符号链接,或者使用 QFileInfo::exists 并结合对设备类型的判断(如是否为块设备 S_ISBLK),确保检查的目标确实是物理分区。

💡 改进后的代码建议

综合以上意见,如果你希望快速改进当前代码,可以参考以下修改:

    qInfo() << "begin check error";

    // Check if the partition device still exists (handles physical USB removal)
    // 注意:建议后续重构将 m_strPartionName 修正为 m_strPartitionName
    QFileInfo partionInfo(m_strPartionName);
    if (!partionInfo.exists()) {
        // 修正日志输出,使其更具可读性
        qCritical() << "Device removed, causing USBMountFailed:" << m_strPartionName;
        // 建议:确保 m_progressStatus 在此处处于合理的错误状态
        emit progressfinished(m_progressStatus, BMHandler::ErrorType::USBMountFailed);
        return;
    }

    QString strDisk = XSys::DiskUtil::GetPartitionDisk(m_strPartionName);
    // ... 后续逻辑

如果你有更多代码需要审查或讨论,随时告诉我!

@pppanghu77
Copy link
Copy Markdown
Author

deepin pr auto review

你好!我是CodeGeeX,你的智能编程助手。我已经仔细审查了你提供的 Git Diff 输入。

本次代码修改主要包含两部分:一是更新了版权声明中的年份范围;二是在 checkError 函数中新增了对分区设备是否存在的检查,以处理物理 USB 被拔出的异常场景。

整体来看,这个修改的逻辑方向是正确的,提前拦截了设备不存在的情况,避免了后续调用 GetPartitionDisk 时可能出现的错误。但针对语法逻辑、代码质量、代码性能和代码安全,我提出以下改进意见:

1. 语法与逻辑

  • 拼写错误:变量名 m_strPartionNamePartion 存在拼写错误,正确应为 Partition(即 m_strPartitionName)。虽然这是历史遗留问题,但建议在后续的重构中统一修正。
  • 竞态条件QFile::exists(m_strPartionName) 的检查与后续 XSys::DiskUtil::GetPartitionDisk(m_strPartionName) 的调用之间存在时间差。在极端情况下,检查时设备存在,但在获取磁盘信息前设备被拔出,依然可能导致后续逻辑异常。因此,后续的 GetPartitionDisk 等底层调用仍需具备处理设备不存在(或返回空/错误)的容错能力。

2. 代码质量

  • 日志信息不规范qCritical() << "Error::get(Error::USBMountFailed) device removed:" << m_strPartionName; 这行日志中,Error::get(Error::USBMountFailed) 看起来像是想调用某个静态方法获取错误描述,但实际并没有调用,而是直接输出了字面量。这会导致日志可读性差。

    • 建议:如果 Error::get 是获取错误描述的方法,应改为 Error::get(Error::USBMountFailed) 的实际调用结果;如果只是想标记错误类型,建议改为更自然的描述,例如:"Device removed, causing USBMountFailed:"
  • 状态一致性:在 emit progressfinished 信号时,使用了 m_progressStatus。需要确认在设备拔出导致错误时,m_progressStatus 的当前值是否符合预期,是否需要在此处重置或赋值为一个特定的“失败/中断”状态,以避免前端 UI 状态显示异常。

3. 代码性能

  • IO 操作的开销QFile::exists 底层会调用操作系统的 stat 系统调用,这是一个阻塞的磁盘 I/O 操作。如果 checkError 函数是在主线程(UI线程)中被调用,或者被高频轮询调用,可能会引起微小的卡顿。

    • 建议:如果这是高频调用的轮询检查,建议将相关逻辑放入工作线程,或者考虑使用 QFileSystemWatcher 等事件驱动机制来监听设备的插拔,而不是依赖 QFile::exists 主动轮询。

4. 代码安全

  • 符号链接安全QFile::exists 默认会跟随符号链接。如果 m_strPartionName 指向的路径被恶意替换为指向其他敏感分区或文件的符号链接,QFile::exists 会返回 true,从而绕过这个 USB 拔出的检查逻辑。

    • 建议:如果此安装器对设备路径的真实性有严格要求,建议在检查前先判断是否为符号链接,或者使用 QFileInfo::exists 并结合对设备类型的判断(如是否为块设备 S_ISBLK),确保检查的目标确实是物理分区。

💡 改进后的代码建议

综合以上意见,如果你希望快速改进当前代码,可以参考以下修改:

    qInfo() << "begin check error";

    // Check if the partition device still exists (handles physical USB removal)
    // 注意:建议后续重构将 m_strPartionName 修正为 m_strPartitionName
    QFileInfo partionInfo(m_strPartionName);
    if (!partionInfo.exists()) {
        // 修正日志输出,使其更具可读性
        qCritical() << "Device removed, causing USBMountFailed:" << m_strPartionName;
        // 建议:确保 m_progressStatus 在此处处于合理的错误状态
        emit progressfinished(m_progressStatus, BMHandler::ErrorType::USBMountFailed);
        return;
    }

    QString strDisk = XSys::DiskUtil::GetPartitionDisk(m_strPartionName);
    // ... 后续逻辑

如果你有更多代码需要审查或讨论,随时告诉我!

本次修改不涉及上述问题

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: max-lvs, pppanghu77

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

@pppanghu77
Copy link
Copy Markdown
Author

/forcemerge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented May 12, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot Bot merged commit 661051a into linuxdeepin:release/eagle May 12, 2026
19 of 22 checks passed
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