Skip to content

fix(device): correct disk size unit conversion from KB to MB in Comma…#118

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

fix(device): correct disk size unit conversion from KB to MB in Comma…#118
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
pppanghu77:bug2

Conversation

@pppanghu77
Copy link
Copy Markdown

…ndLsblkParse()

  • Fix disk size calculation by dividing by 1024 twice (KB to MB) instead of multiplying by 1024 then dividing by 1024, which produced incorrect values

修复(device): 修正 CommandLsblkParse 中磁盘容量 KB 到 MB 的单位转换

  • 修正磁盘容量计算,将 KB 到 MB 的转换从先乘 1024 再除 1024 修正为连续除以两次 1024,避免计算结果错误

Log: 修正 lsblk 解析中磁盘容量单位转换逻辑,将错误的乘除组合修正为连续除法以得到正确的 MB 值
Bug: https://pms.uniontech.com/bug-view-349331.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

…ndLsblkParse()

- Fix disk size calculation by dividing by 1024 twice (KB to MB) instead of multiplying by 1024 then dividing by 1024, which produced incorrect values

修复(device): 修正 CommandLsblkParse 中磁盘容量 KB 到 MB 的单位转换

- 修正磁盘容量计算,将 KB 到 MB 的转换从先乘 1024 再除 1024 修正为连续除以两次 1024,避免计算结果错误

Log: 修正 lsblk 解析中磁盘容量单位转换逻辑,将错误的乘除组合修正为连续除法以得到正确的 MB 值
Bug: https://pms.uniontech.com/bug-view-349331.html
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff,下面我将从语法逻辑、代码质量、代码性能和代码安全四个维度对这次代码变更进行详细的分析和评估。

变更概述

本次修改主要包含两处:

  1. 更新了版权声明中的截止年份(从2022改为2026)。
  2. 修改了磁盘容量单位的换算逻辑,将 reg.cap(3).toLongLong() * 1024 / 1024 改为 reg.cap(3).toLongLong() / 1024 / 1024

1. 语法与逻辑

  • 单位换算逻辑修复(核心变更)
    • 原代码* 1024 / 1024。从注释 // MB 和上下文来看,lsblk 命令默认输出的尺寸单位是字节。原代码先乘1024再除1024,相当于没有进行任何换算,赋值给 info.total 的仍然是字节数,这与注释中期望的“MB”不符,是一个逻辑Bug。
    • 新代码/ 1024 / 1024。将字节数除以两次1024(即除以 $1024 \times 1024 = 1,048,576$),正确地将单位从字节转换为了兆字节(MB)。这是一个正确的逻辑修复。
  • 整除截断风险
    • 由于 toLongLong() 返回的是64位整数,连续两次整数除法 (/ 1024 / 1024) 会导致小数部分被直接截断。对于磁盘容量而言,通常MB级别的精度足够,截断误差在可接受范围内,因此这种处理在业务逻辑上是合理的。

2. 代码质量

  • 魔法数字
    • 代码中直接使用了 1024 这个数字,属于“魔法数字”。虽然对于开发者来说 1024 是计算机领域众所周知的进制,但为了代码的可读性和可维护性,建议将其定义为具名常量。
  • 注释与代码的一致性
    • 注释 // MB 很好地解释了当前运算的目标,这点值得肯定。

3. 代码性能

  • 除法运算优化
    • 当前写法是 toLongLong() / 1024 / 1024,这会执行两次整除运算。在底层CPU指令中,除法运算的周期数通常远多于位运算。
    • 虽然在现代CPU上,两次64位整数除法的开销微乎其微,且此处并非性能热点,但从极致优化的角度,可以使用右移位运算来替代除以2的幂次方的运算,这不仅减少了指令数,还能避免潜在的除法异常。

4. 代码安全

  • 正则捕获组的健壮性
    • reg.cap(3) 获取正则匹配的字符串,如果正则匹配失败或该捕获组不存在,cap(3) 会返回空字符串 ""QString::toLongLong() 在遇到空字符串或非法字符时会返回 0,并且会设置 ok 标志为 false。当前代码没有检查转换是否成功,虽然不会引起程序崩溃,但可能会将无效的磁盘信息(容量为0)静默传递给上层,可能导致UI显示异常或逻辑误判。
  • 整数溢出风险
    • 在原代码 * 1024 / 1024 中,如果 toLongLong() 获取的值非常大,乘以 1024 时存在有符号64位整数溢出的风险(虽然当前硬盘容量还达不到这个极限,但作为防御性编程需要考虑)。
    • 修改为 / 1024 / 1024 后,彻底消除了乘法溢出的风险,这是一个显著的安全性提升。
  • 版权年份
    • 将版权截止年份修改为2026,请确保这符合贵公司的版权管理规范(通常如果是持续维护的代码,年份范围应包含当前发布年份,修改为2026如果是预留年份需确认法务合规)。

改进建议与重构代码

综合以上分析,我为你提供一份优化后的代码参考。主要改进点在于:引入常量消除魔法数字、使用位移优化性能、增加正则转换的合法性校验

// 建议在文件头或类内部定义常量,消除魔法数字
static const qint64 BYTES_PER_KB = 1024;
static const qint64 KB_PER_MB = 1024;
static const qint64 BYTES_PER_MB = BYTES_PER_KB * KB_PER_MB; // 1048576

// ... 在 CommandLsblkParse 函数中 ...
            deviceCount++;
            info.path = reg.cap(1);
            info.label = reg.cap(2);
            
            // 建议增加转换校验
            bool ok = false;
            qint64 bytesSize = reg.cap(3).toLongLong(&ok);
            if (ok && bytesSize >= 0) {
                // 使用右移20位替代除以 1024*1024,等价于 bytesSize / BYTES_PER_MB
                // 右移10位是除以1024,右移20位是除以1048576
                info.total = bytesSize >> 20; // MB
            } else {
                // 处理转换失败或非法数据的情况
                info.total = 0; 
                // 可选:qWarning() << "Failed to parse device size or invalid size for:" << info.path;
            }
            
            info.uuid = reg.cap(4);
            info.fstype = reg.cap(5);
            type = reg.cap(6);

总结:你的这次Diff修复了一个关键的容量计算逻辑Bug,并消除了原有的乘法溢出风险,方向是完全正确的。如果对代码健壮性有更高要求,可以参考我的改进建议增加校验逻辑和常量定义。

@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 cb3c60a into linuxdeepin:release/eagle May 12, 2026
18 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