fix(systeminfo): Remove CPU count and enhance frequency logic #1010
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideRefines how CPU/processor information is reported by removing CPU count suffixes from processor strings, centralizing CPU info initialization, and introducing a helper to supplement missing frequency data using current or max CPU speed, with tests updated accordingly. Sequence diagram for SystemInfo initialization with supplemented CPU frequencysequenceDiagram
participant SystemInfo
participant CPUInfoModule
participant LscpuRunner
participant FrequencySupplementHelper
SystemInfo->>SystemInfo: init()
SystemInfo->>SystemInfo: getVersion()
SystemInfo-->>SystemInfo: Version
SystemInfo->>SystemInfo: getMemInfo()
SystemInfo-->>SystemInfo: Memory fields
SystemInfo->>SystemInfo: getDiskCapacity()
SystemInfo-->>SystemInfo: Disk fields
SystemInfo->>CPUInfoModule: GetCPUInfo(/proc/cpuinfo)
CPUInfoModule-->>SystemInfo: Processor
SystemInfo->>LscpuRunner: runLscpu()
LscpuRunner-->>SystemInfo: lscpuRes (map)
SystemInfo->>CPUInfoModule: getCPUMaxMHzByLscpu(lscpuRes)
CPUInfoModule-->>SystemInfo: CPUMaxMHz
SystemInfo->>CPUInfoModule: getCPUMHzCurrent(lscpuRes)
CPUInfoModule-->>SystemInfo: CurrentSpeed
SystemInfo->>FrequencySupplementHelper: supplementProcessorFrequency(Processor, CurrentSpeed, CPUMaxMHz)
FrequencySupplementHelper-->>SystemInfo: Processor (with frequency if missing)
Class diagram for updated SystemInfo CPU initialization and helpersclassDiagram
class SystemInfo {
+string Processor
+string Version
+uint64 CurrentSpeed
+float64 CPUMaxMHz
+init()
+isValidity() bool
}
class CPUInfoModule {
+GetCPUInfo(path string) string
+getProcessorByLscpuExt(data map_string_string, freq float64) (string, error)
+getProcessorByLscpu(data map_string_string) (string, error)
+getCPUMaxMHzByLscpu(data map_string_string) (float64, error)
+swCPUInfo(data map_string_string) string
+hwKirinCPUInfo(data map_string_string) string
+getCPUInfoFromMap(nameKey string, numKey string, data map_string_string) (string, error)
+getCPUName(key string, data map_string_string) (string, error)
+getCPUHz(key string, data map_string_string) (float64, error)
}
class FrequencySupplementHelper {
+supplementProcessorFrequency(processor string, currentSpeed uint64, cpuMaxMHz float64) string
}
SystemInfo --> CPUInfoModule : uses
SystemInfo --> FrequencySupplementHelper : uses
class RemovedCPUCountLogic {
-getCPUNumber(key string, data map_string_string) (int, error)
-lscpuKeyCount : string
}
CPUInfoModule ..|> RemovedCPUCountLogic : CPU count logic removed
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
electricface
force-pushed
the
swt/fix-bug274991
branch
from
8ab904b to
470f489
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
supplementProcessorFrequency, consider precompiling the frequency regex as a package-levelregexp.Regexpinstead of callingregexp.MatchStringeach time to avoid repeated compilation on everyinit. - The debug log in
supplementProcessorFrequencyonly prints the original processor string and source; including the final formatted value with the appended frequency would make the log more informative for debugging.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `supplementProcessorFrequency`, consider precompiling the frequency regex as a package-level `regexp.Regexp` instead of calling `regexp.MatchString` each time to avoid repeated compilation on every `init`.
- The debug log in `supplementProcessorFrequency` only prints the original processor string and source; including the final formatted value with the appended frequency would make the log more informative for debugging.
## Individual Comments
### Comment 1
<location> `systeminfo1/info.go:297` </location>
<code_context>
+ func supplementProcessorFrequency(processor string, currentSpeed uint64, cpuMaxMHz float64) string {
</code_context>
<issue_to_address>
**issue:** Frequency detection only triggers on the "@ ... Hz" pattern, which can lead to duplicated frequency when the model name already includes GHz without '@'.
Because the regexp only matches `@ <num><unit>Hz`, it misses model strings that already contain a bare `<num>GHz` (e.g. from `/proc/cpuinfo`). In those cases `matched` stays false and we append a second frequency, yielding names like `Intel(R) ... 3.20GHz @ 2.30GHz`. It would be better to detect any existing `<num>\s*(M|G)Hz` (with or without `@`) or, at minimum, check for existing `GHz`/`MHz` tokens before appending.
</issue_to_address>
### Comment 2
<location> `systeminfo1/info.go:309-318` </location>
<code_context>
+ } else {
+ source = "CPUMaxMHz"
+ }
+ logger.Debugf("Added frequency to Processor: %s (from %s)", processor, source)
+ return fmt.Sprintf("%s @ %.2fGHz", processor, freqGHz)
+ }
</code_context>
<issue_to_address>
**suggestion:** Debug log only prints the original processor string, not the final value with frequency appended.
Here `processor` is still the unmodified string, so the log doesn’t match the final returned value. To keep diagnostics accurate, either include `freqGHz` in the log or log the fully formatted string by assigning it to a variable used for both logging and returning.
```suggestion
if freqGHz > 0 {
var source string
if currentSpeed != 0 {
source = "CurrentSpeed"
} else {
source = "CPUMaxMHz"
}
formattedProcessor := fmt.Sprintf("%s @ %.2fGHz", processor, freqGHz)
logger.Debugf("Added frequency to Processor: %s (from %s)", formattedProcessor, source)
return formattedProcessor
}
```
</issue_to_address>
### Comment 3
<location> `systeminfo1/info_test.go:106-115` </location>
<code_context>
+func TestDoReadCache(t *testing.T) {
</code_context>
<issue_to_address>
**suggestion (testing):** Extend `TestDoReadCache` to cover processor frequency supplementation behavior
The new test is a good improvement, but it only verifies round-tripping `SystemInfo` as-is. To validate the new `init()` behavior that calls `supplementProcessorFrequency`, please add a test that writes a cached `SystemInfo` whose `Processor` lacks a frequency (e.g. `"Intel(R) Core(TM) i5-4570 CPU"`) but has nonzero `CurrentSpeed`/`CPUMaxMHz`, then:
- Reads it back via `doReadCache`, and
- Either calls `supplementProcessorFrequency` on the loaded struct and asserts the `Processor` gains the `@ xx.xxGHz` suffix, or
- Goes through the public initialization path and asserts the final `Processor` value.
This will ensure the regression (missing frequency from cached data) is properly covered by tests.
Suggested implementation:
```golang
func TestDoReadCache(t *testing.T) {
tmpDir := t.TempDir()
cacheFile := filepath.Join(tmpDir, "systeminfo_cache.json")
// 创建测试数据（包含频率，验证正常的回写、读取）
expectedInfo := &SystemInfo{
Version: "20 专业版",
DistroID: "uos",
DistroDesc: "UnionTech OS 20",
DistroVer: "20",
Processor: "Intel(R) Core(TM) i5-4570 CPU @ 3.20GHz",
DiskCap: uint64(500107862016),
MemoryCap: uint64(8280711168),
}
data, err := json.Marshal(expectedInfo)
assert.NoError(t, err)
err = os.WriteFile(cacheFile, data, 0o644)
assert.NoError(t, err)
// 通过 doReadCache 读回并验证与写入时一致
got, err := doReadCache(cacheFile)
assert.NoError(t, err)
assert.Equal(t, expectedInfo, got)
}
func TestDoReadCache_SupplementsProcessorFrequency(t *testing.T) {
tmpDir := t.TempDir()
cacheFile := filepath.Join(tmpDir, "systeminfo_cache.json")
// 缓存中的 SystemInfo：CPU 型号没有频率后缀
cachedInfo := &SystemInfo{
Version: "20 专业版",
DistroID: "uos",
DistroDesc: "UnionTech OS 20",
DistroVer: "20",
Processor: "Intel(R) Core(TM) i5-4570 CPU",
DiskCap: uint64(500107862016),
MemoryCap: uint64(8280711168),
// 这里还应设置表示 CPU 频率的字段（例如 CurrentSpeed / CPUMaxMHz），见 additional_changes
}
data, err := json.Marshal(cachedInfo)
assert.NoError(t, err)
err = os.WriteFile(cacheFile, data, 0o644)
assert.NoError(t, err)
// 通过 doReadCache 读回缓存数据
got, err := doReadCache(cacheFile)
assert.NoError(t, err)
// 调用补全逻辑，期望为 Processor 补上频率后缀
err = supplementProcessorFrequency(got)
assert.NoError(t, err)
assert.Equal(t, "Intel(R) Core(TM) i5-4570 CPU @ 3.20GHz", got.Processor)
```
1. Imports:
- Ensure `systeminfo1/info_test.go` imports the following packages (add if missing):
- `encoding/json`
- `os`
- `path/filepath`
Example (adjust to your existing import block style):
```go
import (
"encoding/json"
"os"
"path/filepath"
"github.com/stretchr/testify/assert"
)
```
2. `doReadCache` signature:
- The above tests assume `doReadCache` has the signature:
```go
func doReadCache(cacheFile string) (*SystemInfo, error)
```
- If your actual `doReadCache` has a different signature (e.g. no parameters and uses a global cache path), change the test calls accordingly:
- Use the same invocation form as the existing/old `TestDoReadCache`.
- If `doReadCache` does not accept a path, you should:
- Either temporarily override the global cache path within the test (saving and restoring it with `defer`), or
- Use whatever helper your original test was using to point `doReadCache` at the temporary file.
3. `supplementProcessorFrequency` signature:
- The test assumes:
```go
func supplementProcessorFrequency(info *SystemInfo) error
```
- If the actual signature returns no error, adjust to:
```go
supplementProcessorFrequency(got)
```
and remove the `assert.NoError(t, err)` line.
- If its parameter type or name differs, adapt the call accordingly but keep the semantics: it should operate on the `SystemInfo` instance loaded from `doReadCache`.
4. CPU frequency fields:
- The comment in `TestDoReadCache_SupplementsProcessorFrequency` mentions setting fields such as `CurrentSpeed`/`CPUMaxMHz`. Modify the `cachedInfo` literal to set the correct fields and types used in your actual `SystemInfo` definition, for example:
```go
cachedInfo := &SystemInfo{
// ...
CurrentSpeedMHz: 3200,
CPUMaxMHz: 3200,
}
```
- Align the field names and units (MHz/GHz, float vs int) with your existing struct and with what `supplementProcessorFrequency` expects.
5. If you want to keep the original “round-trip without modification” behavior in addition to the new checks:
- Confirm whether an older version of `TestDoReadCache` had additional assertions or setup beyond what is visible in the provided snippet.
- If so, reintroduce those assertions into the new implementation of `TestDoReadCache` above, just before the final `assert.Equal(t, expectedInfo, got)`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
electricface
changed the title
electricface
force-pushed
the
swt/fix-bug274991
branch
2 times, most recently
from
961d0de to
7f85654
Compare
- Removed code related to CPU count retrieval; now consistently returns CPU model information without quantity - Modified the Processor string to append frequency information, preventing duplicate frequency entries - Updated relevant test cases by removing assertions for CPU count and verifying correct frequency appending --- fix(systeminfo): 删除CPU计数并增加频率补充逻辑 - 移除获取CPU数量相关代码，统一返回无数量的CPU型号信息 - 修改Processor字符串补充频率的实现，避免重复添加频率信息 - 更新相关测试用例，去除对CPU数量的断言，验证频率补充正确 PMS: BUG-274991 Influence: 控制中心->系统->关于本机->处理器
electricface
force-pushed
the
swt/fix-bug274991
branch
from
7f85654 to
566dd76
Compare
deepin pr auto reviewGit Diff 代码审查报告总体评价该代码修改主要是重构了 CPU 信息处理逻辑，移除了 CPU 核心数的显示，并增加了频率信息补充功能。整体代码结构清晰，但存在一些可以改进的地方。 详细审查意见1. 代码逻辑问题1.1 频率单位转换问题在 if currentSpeed != 0 {
freqGHz = float64(currentSpeed) / 1000
} else if !isFloatEqual(cpuMaxMHz, 0.0) {
freqGHz = cpuMaxMHz / 1000
}问题： 建议：明确变量单位或添加注释说明： // cpuMaxMHz: max CPU frequency in MHz (note: variable name is historical, actual unit is MHz)
if currentSpeed != 0 {
freqGHz = float64(currentSpeed) / 1000
} else if !isFloatEqual(cpuMaxMHz, 0.0) {
freqGHz = cpuMaxMHz / 1000 // Convert MHz to GHz
}1.2 正则表达式匹配问题freqPattern := `\d+\.?\d*\s*[MGT]Hz`问题：这个正则表达式可能匹配到不完整的频率信息，如 "1.2G" 而不是 "1.2GHz"。 建议：修改正则表达式确保完整匹配： freqPattern := `\d+\.?\d*\s*[MGT]Hz\b` // 添加 \b 确保单词边界2. 代码性能问题2.1 正则表达式编译matched, err := regexp.MatchString(freqPattern, processor)问题：每次调用 建议：将正则表达式预编译为包级变量： var freqRegex = regexp.MustCompile(`\d+\.?\d*\s*[MGT]Hz\b`)
func supplementProcessorFrequency(...) string {
// ...
matched := freqRegex.MatchString(processor)
// ...
}3. 代码安全问题3.1 日志信息泄露logger.Debugf("Added frequency to Processor: %s (from %s)",
formattedProcessor, source)问题：日志中记录了完整的处理器信息，可能泄露系统信息。 建议：考虑在敏感环境中减少日志详细程度或添加日志级别控制。 4. 代码质量问题4.1 测试用例改进在 // 测试极端频率值
result = supplementProcessorFrequency("High Speed CPU", 999999, 0)
assert.Equal(t, "High Speed CPU @ 999.999GHz", result)
// 测试负值输入（虽然实际不会发生）
result = supplementProcessorFrequency("Negative CPU", -1000, 0)
assert.Equal(t, "Negative CPU", result) // 应该忽略负值4.2 错误处理if err != nil {
logger.Warning("Failed to match frequency pattern:", err)
return processor
}问题：正则表达式匹配失败时直接返回原始字符串，可能导致频率信息缺失。 建议：考虑更健壮的错误处理或降级策略。 5. 测试代码改进5.1 TestDoReadCache 改进当前测试创建临时文件的方式可以更安全： func TestDoReadCache(t *testing.T) {
// 使用 t.TempDir() 创建临时目录
tmpDir := t.TempDir()
tmpFile := filepath.Join(tmpDir, "systeminfo.cache")
// 创建测试数据
expectedInfo := &SystemInfo{...}
// 保存并验证
err := doSaveCache(expectedInfo, tmpFile)
assert.NoError(t, err)
// 读取并验证
ret, err := doReadCache(tmpFile)
assert.NoError(t, err)
// 使用 assert.Equal 比较整个结构体
assert.Equal(t, expectedInfo, ret)
}总结
整体而言，代码修改方向正确，测试覆盖充分，但建议在上述几个方面进行改进以提高代码质量和性能。 |
fly602
approved these changes
Feb 2, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: electricface, fly602 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix(systeminfo): 删除CPU计数并增加频率补充逻辑
PMS: BUG-274991
Influence: 控制中心->系统->关于本机->处理器