fix(inputdevices): fsync udev rule and rebuild on boot for touchpad - #1225
Conversation
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
Reviewer's GuideThe PR hardens touchpad udev-rule persistence against forced power loss by syncing file data and creation metadata, and ensures startup always reconstructs the rule from the persisted setting rather than incorrectly skipping the write when the in-memory state already matches. Sequence diagram for durable touchpad udev rule restorationsequenceDiagram
participant Startup
participant InputDevices
participant Touchpad
participant UdevRule
participant DeviceManager
Startup->>InputDevices: init()
InputDevices->>Touchpad: setTouchpadEnableViaUdev(enabled)
alt enabled
Touchpad->>UdevRule: remove rule file
Touchpad->>DeviceManager: refreshTouchpadDevices()
else disabled
Touchpad->>UdevRule: writeUdevRuleFile(content)
UdevRule-->>UdevRule: f.Sync()
UdevRule-->>UdevRule: sync parent directory
alt content changed
Touchpad->>DeviceManager: refreshTouchpadDevices()
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 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="129-130" />
<code_context>
- logger.Debug("udev rule file already exists with correct content, skip writing")
- return nil
+ return t.refreshTouchpadDevices()
+ }
+
+ // 禁用:写入 udev 规则文件;若内容未变则无需刷新设备
</code_context>
<issue_to_address>
**issue (bug_risk):** The enabled path removes the udev rule but never synchronizes the parent directory, so a forced power-off after enabling can leave the unlink operation non-persistent and restore the old disabling rule on the next boot.
**Triggers:** When the touchpad is enabled and the machine is powered off before the directory metadata is durably committed.
**Suggested fix:** Call `syncDirBestEffort(filepath.Dir(udevRuleFile))` after a successful `os.Remove`, or synchronously propagate a directory `Sync` error if durability is required.
```suggestion
if err := os.Remove(udevRuleFile); err != nil {
if !os.IsNotExist(err) {
return err
}
} else {
syncDirBestEffort(filepath.Dir(udevRuleFile))
}
```
</issue_to_address>
### Comment 2
<location path="system/inputdevices1/inputdevices.go" line_range="101" />
<code_context>
+ // 不调用 setTouchpadEnable,因为 newTouchpad 已将 Enable 设为 dconfig 值,
+ // setPropEnable 会判定 changed=false 从而跳过 udev 写入,
+ // 导致强制关机后 udev 规则文件丢失但无法重建。
+ err = m.touchpad.setTouchpadEnableViaUdev(enabled)
if err != nil {
logger.Warning(err)
</code_context>
<issue_to_address>
**issue (bug_risk):** The startup goroutine uses a snapshot of the dconfig value without synchronizing with `SetTouchpadEnable`; a concurrent user request can be overwritten by the stale startup write, leaving the udev rule and the exposed `Enable` state inconsistent.
**Triggers:** When a touchpad enable/disable request arrives while the asynchronous startup restoration is running.
**Suggested fix:** Serialize startup restoration and `SetTouchpadEnable` with the touchpad mutex, or re-read and validate the current configuration before applying the startup udev state.
</issue_to_address>
### Comment 3
<location path="system/inputdevices1/touchpad.go" line_range="184-188" />
<code_context>
+ f.Close()
+ return false, err
+ }
+ if err := f.Truncate(0); err != nil {
+ f.Close()
+ return false, err
+ }
+ if _, err := f.Write([]byte(udevRuleContent)); err != nil {
+ f.Close()
+ return false, err
</code_context>
<issue_to_address>
**issue (bug_risk):** The replacement path truncates the live udev rule file before writing the new contents, so a process crash, power loss, or concurrent udev read during this window leaves an empty or partial rule file instead of the previous valid rule; the comment claiming an atomic write is false.
**Triggers:** When the machine loses power or another process reads the rule while the file is being rewritten.
**Suggested fix:** Write the contents to a temporary file in the same directory, `Sync` it, then atomically replace the rule with `Rename` and synchronize the parent directory.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 代码语法正确,编译无问题。逻辑路径清晰:O_EXCL 原子检测新文件 -> Chmod 先于 Truncate 避免数据丢失 -> Write -> Sync -> Close -> 父目录 Sync。错误处理路径均正确关闭文件句柄,无资源泄漏。inputdevices.go 中将 setTouchpadEnable 改为 setTouchpadEnableViaUdev 的逻辑修复正确,解决了 changed=false 导致 udev 规则跳过重建的问题。 2. 代码质量 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 注释质量优秀,writeUdevRuleFile 函数注释详细说明了 TOCTOU 竞态防护、Chmod 先于 Truncate 的原因、fsync 的必要性、父目录 fsync 的场景等安全考量。函数命名清晰(writeUdevRuleFile、syncDirBestEffort),语义准确。无重复代码,无残留调试代码。go.mod/go.sum 仅更新依赖版本,无质量问题。 3. 代码性能 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: fsync 引入的 I/O 延迟是必要的,确保断电数据持久化,符合本次修复目的。快速路径(内容已正确)避免不必要的写入和设备刷新,优化合理。此为配置路径非热路径,性能影响可忽略。 4. 代码安全 🔒评价: 优秀 ✅ 通过
安全漏洞详情: 建议: 安全处理优秀:1) O_EXCL 原子操作避免 TOCTOU 竞态;2) 文件权限 0644 正确设置且在快速路径中兜底收窄,防止外部篡改为 0666 等过宽权限;3) Chmod 先于 Truncate 避免权限修改失败时 O_TRUNC 已清空原内容导致规则丢失;4) fsync 文件数据 + 父目录 fsync 确保断电持久化,解决核心问题;5) 所有错误路径正确关闭文件句柄,无资源泄漏。 💡 改进建议代码示例// 暂无代码示例本报告由 AI 代码审查工具自动生成 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602, mhduiy 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 |
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:
fix(inputdevices): 修复强制关机后触控板禁用状态丢失
Log: 修复强制关机后触控板禁用状态丢失,原因是 udev 规则文件
写入未调用 fsync 导致断电丢失,且启动恢复逻辑因 changed=false
跳过了 udev 规则重建
Influence:
PMS: BUG-374789
Summary by Sourcery
Ensure touchpad udev rules are durably persisted and restored so the enabled or disabled state survives power loss and reboot.
Bug Fixes:
Enhancements:
Build: