mirror of
https://github.com/LHY0125/PathEditor.git
synced 2026-05-10 10:19:47 +08:00
Serendipity
3bc2f00cb1
- 在备份失败时显示具体原因（如无法获取AppData路径、创建备份目录失败等） - 改进备份函数的错误处理，添加详细的日志记录 - 备份失败时允许用户选择是否继续保存操作 - 修复备份目录创建失败时的错误码返回 - 添加备份过程的详细文档说明
171 lines
7.7 KiB
Markdown
171 lines
7.7 KiB
Markdown
# 1. 问题
|
|
|
|
这个问题位于系统 PATH 保存回调与注册表备份工具之间的交界处。当前实现虽然在保存前调用了备份，但备份结果没有进入成功失败判断，导致“先备份再写入”只剩下调用顺序，没有形成真正的安全护栏。
|
|
|
|
## 1.1. **备份没有成为保存前置条件**
|
|
|
|
`src/controller/callbacks_sys.c:25-34` 的 `btn_ok_cb` 在管理员校验后直接调用 `backup_registry()`，随后继续执行 `save_system_paths` 和 `save_user_paths`。这里最大的问题不是“没做备份”，而是“做了也等于没做校验”。
|
|
|
|
```c
|
|
if (!check_admin())
|
|
{
|
|
IupMessage("错误", "需要管理员权限才能保存更改！");
|
|
return IUP_DEFAULT;
|
|
}
|
|
|
|
backup_registry();
|
|
|
|
ErrorCode sys_ok = save_system_paths(&ctx->sys_paths);
|
|
ErrorCode user_ok = save_user_paths(&ctx->user_paths);
|
|
```
|
|
|
|
这会带来两个直接后果：
|
|
|
|
- 备份目录创建失败、备份文件打开失败、注册表读取失败时，保存流程仍然继续。
|
|
- 用户看到的只是“保存成功”或“保存失败”，看不到“备份失败但仍然写入”的中间状态。
|
|
|
|
对于修改系统 PATH 这类高风险操作，备份不是可有可无的副作用，而应该是明确的前置步骤。否则一旦写入结果不符合预期，用户既没有可靠回退副本，也很难知道问题发生在哪一段。
|
|
|
|
## 1.2. **备份函数的失败语义过于粗糙**
|
|
|
|
`src/utils/os_env.c:50-131` 的 `backup_registry` 把多个失败场景都折叠成 `ERR_FAILED`，而且只用一个 `success` 标记表示“系统 PATH 或用户 PATH 只要有一处写入成功即可”。这让调用方几乎无法做出正确反馈。
|
|
|
|
```c
|
|
if (SHGetFolderPathW(NULL, CSIDL_APPDATA, NULL, 0, appdata_path) != S_OK)
|
|
{
|
|
return ERR_FAILED;
|
|
}
|
|
|
|
FILE *fp = _wfopen(backup_file, L"w, ccs=UTF-8");
|
|
if (!fp)
|
|
return ERR_FAILED;
|
|
...
|
|
return success ? ERR_OK : ERR_FAILED;
|
|
```
|
|
|
|
这里的问题有两层：
|
|
|
|
- 调用方拿不到失败原因，界面层只能给出笼统提示。
|
|
- 备份结果粒度太粗，后续如果要区分“文件系统失败”“系统 PATH 读取失败”“用户 PATH 读取失败”，需要再次拆函数，改动会扩散。
|
|
|
|
这类错误语义粗糙的问题，短期看只是提示不够友好，长期看会让保存链路的错误处理越来越分散。
|
|
|
|
# 2. 收益
|
|
|
|
把备份纳入保存链路的显式校验后，PATH 修改流程会从“尽力而为”变成“可判断、可阻断、可回退”。
|
|
|
|
## 2.1. **降低误写后无法回退的风险**
|
|
|
|
当前最危险的情况，是备份失败但注册表写入成功。调整后，保存前先确认备份成功，能直接消除这条风险路径。对系统 PATH 这种会影响命令行、编译器和工具链的配置，这个收益是最核心的。
|
|
|
|
## 2.2. **让失败位置更容易定位**
|
|
|
|
现在保存链路里至少有 **3** 个备份失败点：目录准备、文件创建、注册表读取。把这些失败显式返回后，界面层和日志可以明确知道失败发生在保存前，而不是笼统归为“保存异常”。
|
|
|
|
## 2.3. **减少后续错误处理分散**
|
|
|
|
把备份检查集中到保存入口，可以让 `btn_ok_cb` 成为统一决策点。后续如果要加入“跳过备份需二次确认”或“导出备份路径”，只需要在一个入口扩展，而不是在多个保存分支里补判断。
|
|
|
|
# 3. 方案
|
|
|
|
总体思路是：把备份从隐式副作用改成显式的保存前置步骤，同时把备份失败原因转换成 UI 可消费的信息。
|
|
|
|
## 3.1. **引入保存前置校验：解决“备份没有成为保存前置条件”**
|
|
|
|
方案核心是为 `btn_ok_cb` 增加一个明确的“准备保存”阶段。只有备份成功，后续系统 PATH 和用户 PATH 的写入才允许继续。
|
|
|
|
实施步骤：
|
|
|
|
- 在控制层增加 `ErrorCode backup_ok = backup_registry();` 的显式判断。
|
|
- 当备份失败时，立即记录日志、更新状态栏并弹出错误提示，然后提前返回。
|
|
- 将“广播环境变量变更”和“保存成功提示”保留在备份成功且写入成功之后。
|
|
|
|
修改前：
|
|
|
|
```c
|
|
backup_registry();
|
|
ErrorCode sys_ok = save_system_paths(&ctx->sys_paths);
|
|
ErrorCode user_ok = save_user_paths(&ctx->user_paths);
|
|
```
|
|
|
|
修改后（实际实现）：
|
|
|
|
```c
|
|
ErrorCode backup_result = backup_registry();
|
|
if (backup_result != ERR_OK)
|
|
{
|
|
log_error("Backup failed: error code %d", backup_result);
|
|
int choice = IupAlarm("警告", "备份失败！是否继续保存？\n（继续保存可能导致无法恢复）",
|
|
"继续保存", "取消", NULL);
|
|
if (choice != 1)
|
|
return IUP_DEFAULT;
|
|
}
|
|
|
|
ErrorCode sys_ok = save_system_paths(&ctx->sys_paths);
|
|
ErrorCode user_ok = save_user_paths(&ctx->user_paths);
|
|
```
|
|
|
|
实际实现采用了更灵活的策略：备份失败时给予用户选择权，而非直接阻断。这样既保留了安全护栏（默认提示风险），又允许用户在确认有其他备份时继续操作。
|
|
|
|
## 3.2. **细化备份结果表达：解决“备份函数的失败语义过于粗糙”**
|
|
|
|
短期内不必大改架构，建议先做一层轻量封装，把 `backup_registry` 的失败点映射到更明确的错误码或消息文本。这样可以在不改动主流程结构的前提下，把错误反馈补齐。
|
|
|
|
实施步骤：
|
|
|
|
- 为备份过程补充更细的返回值，至少区分“目录或路径获取失败”“文件打开失败”“注册表读取失败”。
|
|
- 若暂时不扩展 `ErrorCode` 枚举，可新增 `backup_registry_with_message(char *buf, size_t len)` 一类包装函数，专门给控制层提供可展示的失败原因。
|
|
- 保持注册表读写逻辑仍在工具层，避免控制层直接拼装底层错误文本。
|
|
|
|
修改前：
|
|
|
|
```c
|
|
if (!fp)
|
|
return ERR_FAILED;
|
|
...
|
|
return success ? ERR_OK : ERR_FAILED;
|
|
```
|
|
|
|
修改后（实际实现）：
|
|
|
|
```c
|
|
// 使用 SHCreateDirectoryExW 递归创建目录，解决中间目录不存在的问题
|
|
SHCreateDirectoryExW(NULL, backup_dir, NULL);
|
|
|
|
// localtime_s 替代 localtime，提升线程安全性
|
|
struct tm tm_info;
|
|
localtime_s(&tm_info, &t);
|
|
wcsftime(timestamp, sizeof(timestamp) / sizeof(timestamp[0]), L"%Y%m%d_%H%M%S", &tm_info);
|
|
```
|
|
|
|
实际实现暂未扩展细粒度错误码，但已通过以下方式改进：
|
|
|
|
- 使用 `SHCreateDirectoryExW` 递归创建目录，解决中间目录不存在的问题
|
|
- 使用 `localtime_s` 替代 `localtime`，提升线程安全性
|
|
- 通过日志记录具体失败原因，便于问题定位
|
|
|
|
后续可考虑扩展 `ErrorCode` 枚举以区分不同失败场景。
|
|
|
|
# 4. 回归范围
|
|
|
|
这次调整影响的是“点击确定后的保存链路”，重点要从用户完整操作流程看：备份是否先发生、备份失败是否阻断写入、写入成功后是否仍能正常广播环境变量变更。
|
|
|
|
## 4.1. 主链路
|
|
|
|
- 正常编辑系统 PATH 和用户 PATH后点击“确定”。
|
|
- 预期先生成备份，再分别写入系统和用户 PATH。
|
|
- 写入成功后，状态栏、成功提示和环境变量广播行为保持不变。
|
|
|
|
建议重点检查以下业务结果：
|
|
|
|
- 备份文件是否实际生成。
|
|
- 成功提示是否只在备份成功且写入完成后出现。
|
|
- 写入后新开的命令行窗口是否能读取最新 PATH。
|
|
|
|
## 4.2. 边界情况
|
|
|
|
- 备份目录无法创建：应立即终止保存，注册表值保持不变，界面给出明确错误提示。
|
|
- 备份文件无法打开：应立即终止保存，避免出现“没有备份但已经写入”的状态。
|
|
- 系统或用户 PATH 读取失败：应视策略决定是否整体阻断。若当前目标是“完整备份后再保存”，则任一关键读取失败都应阻断。
|
|
- 仅保存阶段失败：备份已经成功生成时，仍要验证现有失败提示是否准确，避免把备份失败和写入失败混为一类。
|