Release/2500 - #1222
Closed
fly602 wants to merge 1 commit into
Closed
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
TAG Bot TAG: 6.1.99.1 |
Reviewer's GuideThis release fixes touchpad udev-rule recovery and durability: startup now explicitly rebuilds the rule from dconfig even when no logical state change is detected, while rule writes are synced to storage and errors are propagated before completion. Sequence diagram for durable touchpad udev-rule recoverysequenceDiagram
participant InputDevices
participant Touchpad
participant DConfig
participant UdevRuleFile
participant Storage
InputDevices->>DConfig: Value()
DConfig-->>InputDevices: enabled
InputDevices->>Touchpad: setTouchpadEnableViaUdev(enabled)
Touchpad->>UdevRuleFile: os.Create()
Touchpad->>UdevRuleFile: Write()
Touchpad->>UdevRuleFile: Sync()
UdevRuleFile->>Storage: persist rule data
Storage-->>UdevRuleFile: synced
Touchpad->>UdevRuleFile: Close()
UdevRuleFile-->>Touchpad: error or success
Touchpad-->>InputDevices: error or success
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="system/inputdevices1/touchpad.go" line_range="154" />
<code_context>
+ f.Close()
+ return err
+ }
+ err = f.Sync()
+ if err != nil {
+ f.Close()
</code_context>
<issue_to_address>
**issue (broader_impact):** `f.Sync()` flushes the file contents but does not flush the parent directory metadata, so after creating a previously missing rule file, a forced power loss can still lose the directory entry and leave the touchpad enabled after reboot.
**Triggers:** When `udevRuleFile` does not exist and power is lost before the parent directory is synchronized.
**Suggested fix:** Open and sync the parent directory after creating the rule file, or use an atomic replacement followed by a directory `fsync`.
</issue_to_address>
### Comment 2
<location path="system/inputdevices1/touchpad.go" line_range="145" />
<code_context>
- if err := os.WriteFile(udevRuleFile, []byte(udevRuleContent), 0644); err != nil {
+ // 创建或覆盖 udev 规则文件，使用 fsync 确保落盘，
+ // 防止强制关机（断电）时 page cache 丢失导致规则文件丢失
+ f, err := os.Create(udevRuleFile)
+ if err != nil {
+ return err
</code_context>
<issue_to_address>
**🚨 issue (security):** `os.Create` creates a new rule file with mode `0666` subject to the process umask, whereas the previous `os.WriteFile` explicitly requested `0644`; with a permissive umask, the udev rule becomes group- or world-writable.
**Triggers:** When the rule file is newly created on a system whose service umask does not remove group/world write permissions.
**Suggested fix:** Use `os.OpenFile(udevRuleFile, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)` and explicitly enforce the mode for existing files if needed.
```suggestion
f, err := os.OpenFile(udevRuleFile, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
if err != nil {
return err
}
if err := f.Chmod(0644); err != nil {
f.Close()
return err
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
fly602
force-pushed
the
release/2500
branch
2 times, most recently
from
August 26, 2026 06:43
b96960c to
92117d2
Compare
1. Replace os.WriteFile with os.Create + f.Sync() to ensure the udev rule file is flushed to disk before close 2. On startup, call setTouchpadEnableViaUdev directly instead of setTouchpadEnable to avoid skipped udev rebuild when changed=false due to pre-initialized Enable field 3. Prevents touchpad disable state loss after forced power-off Log: fix touchpad disabled state lost after forced power-off because udev rule file write was not fsynced and startup rebuild was skipped by changed=false check Influence: 1. Disable touchpad, force power off, reboot and verify it stays disabled 2. Disable touchpad, normal reboot and verify it stays disabled 3. Enable touchpad, force power off, reboot and verify it stays enabled fix(inputdevices): 修复强制关机后触控板禁用状态丢失 1. 将 os.WriteFile 改为 os.Create + f.Sync() 确保写入 udev 规则文件时强制刷盘，防止断电丢失 2. 启动恢复时直接调用 setTouchpadEnableViaUdev 重建 udev 规 则，避免 changed=false 导致跳过写入 3. 修复强制关机后触控板禁用失效的问题 Log: 修复强制关机后触控板禁用状态丢失，原因是 udev 规则文件 写入未调用 fsync 导致断电丢失，且启动恢复逻辑因 changed=false 跳过了 udev 规则重建 Influence: 1. 禁用触控板后强制关机，重启后确认触控板仍为禁用状态 2. 禁用触控板后正常重启，确认触控板仍为禁用状态 3. 启用触控板后强制关机，重启后确认触控板仍为启用状态 PMS: BUG-374789
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
📋 变更概览
🔍 详细分析1. 语法逻辑 ✅评分： 23/25 ✓ 评价： 语法正确，逻辑清晰 潜在问题：
建议： 建议对 dconfig 返回值使用 comma-ok 类型断言，防止异常类型导致 panic 2. 代码质量 ✅评分： 24/25 ✓ 评价： 代码结构清晰，注释完整 潜在问题：
建议： 建议根据 isNew 标志输出不同日志： 3. 代码性能 ✅评分： 20/20 ✓ 评价： 性能良好，资源使用合理 潜在问题： 建议： fsync 调用带来的 I/O 延迟是修复断电数据丢失的必要代价，设计合理。fast path 在内容未变时跳过写入和 fsync，性能表现良好。O_EXCL 方式避免了额外的 Stat 调用，syncDirBestEffort 仅在需要时调用。 4. 代码安全 🔒评分： 30/30 ✓ 评价： 存在0个安全漏洞
安全漏洞详情： 漏洞对比统计：新增漏洞 0 个，减少漏洞 0 个，持平 0 个 安全分析：
💡 改进建议代码示例// inputdevices.go - 建议使用 comma-ok 类型断言
enabled, ok := v.Value().(bool)
if !ok {
logger.Warning("invalid type for touchpadEnabled config")
return
}
err = m.touchpad.setTouchpadEnableViaUdev(enabled)
// touchpad.go - 建议区分日志消息
if isNew {
logger.Info("created udev rule file:", udevRuleFile)
} else {
logger.Info("updated udev rule file:", udevRuleFile)
}📝 审查结论本次 PR 旨在修复强制关机后触控板禁用状态丢失问题（PMS: BUG-374789），代码实现与 commit 目的完全一致：
代码质量优秀，注释详尽，安全无漏洞，建议合并。 本报告由 AI 代码审查工具自动生成 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Summary by Sourcery
Make touchpad udev rule restoration reliable across restarts and unexpected shutdowns.
Bug Fixes:
Enhancements:
Build: