feat(backup): 增强备份失败时的用户提示和错误处理

- 在备份失败时显示具体原因(如无法获取AppData路径、创建备份目录失败等)
- 改进备份函数的错误处理,添加详细的日志记录
- 备份失败时允许用户选择是否继续保存操作
- 修复备份目录创建失败时的错误码返回
- 添加备份过程的详细文档说明
This commit is contained in:
2026-04-28 22:40:50 +08:00
parent e777b26879
commit 3bc2f00cb1
4 changed files with 499 additions and 8 deletions
+275
View File
@@ -0,0 +1,275 @@
# PathEditor 代码审查修复日志
> 审查日期:2026-04-28
> 审查范围:全项目代码质量审查
> 修复状态:29/29 已完成
---
## 一、高严重度问题(7 项)
### 1.1 add_string_list 中 realloc 失败导致内存泄漏
- **文件**`src/utils/string_ext.c`
- **问题**`realloc` 返回 NULL 时,原 `list->items` 指针被覆盖为 NULL,导致原有数组及其中所有字符串指针永久泄漏
- **修复**:使用临时变量保存 `realloc` 结果,失败时保留原数据不变
```c
// 修复前
list->items = (char **)realloc(list->items, list->capacity * sizeof(char *));
// 修复后
char **new_items = (char **)realloc(list->items, new_capacity * sizeof(char *));
if (!new_items)
return; // 失败时保留原数据
list->items = new_items;
```
### 1.2 add_string_list 中 _strdup 失败未检查
- **文件**`src/utils/string_ext.c`
- **问题**`_strdup` 可能返回 NULL,但代码未检查就存入数组并递增 count,后续字符串操作会崩溃
- **修复**:检查 `_strdup` 返回值,失败时不递增 count
### 1.3 string_list_set 中先 free 后 strdup 导致数据丢失
- **文件**`src/utils/string_ext.c`
- **问题**:先释放旧字符串,然后 `_strdup` 可能失败,导致旧数据丢失且无法恢复
- **修复**:先调用 `_strdup` 获取新字符串,成功后再释放旧字符串
### 1.4 wcsftime 缓冲区大小以字节而非宽字符数传入
- **文件**`src/utils/os_env.c`
- **问题**`sizeof(timestamp)` 返回字节数(128),而 `wcsftime` 期望宽字符数(64),可能导致栈缓冲区溢出
- **修复**:改用 `sizeof(timestamp) / sizeof(timestamp[0])`
```c
// 修复前
wcsftime(timestamp, sizeof(timestamp), L"%Y%m%d_%H%M%S", tm_info);
// 修复后
wcsftime(timestamp, sizeof(timestamp) / sizeof(timestamp[0]), L"%Y%m%d_%H%M%S", &tm_info);
```
### 1.5 load_single_path 在 malloc 失败时仍返回 ERR_OK
- **文件**`src/core/registry_service.c`
- **问题**`malloc` 失败时跳过数据读取,但仍返回 `ERR_OK`,调用者认为加载成功
- **修复**`malloc` 失败时返回 `ERR_OUT_OF_MEMORY`
### 1.6 导入数据内存泄漏
- **文件**`src/controller/callbacks_io.c`
- **问题**`btn_import_cb``ExportData imported``system``user` 从未调用 `clear_string_list` 释放
- **修复**:在函数所有返回路径前调用 `clear_string_list` 释放导入数据
### 1.7 JSON 导入解析器完全失效
- **文件**`src/core/import_export.c`
- **问题**:键名检测逻辑被 `in_string` 状态翻转拦截,`"system"``"user"` 的检测永远无法触发
- **修复**:重写 JSON 解析器,在字符串结束时检测键名,合并重复的数组解析逻辑
---
## 二、中严重度问题(9 项)
### 2.1 backup_registry 部分备份成功时错误报告为完全成功
- **文件**`src/utils/os_env.c`
- **修复**:分别跟踪系统和用户 PATH 的备份状态
### 2.2 btn_ok_cb 未检查 backup_registry 返回值
- **文件**`src/controller/callbacks_sys.c`
- **修复**:检查返回值,备份失败时提示用户是否继续保存
```c
// 修复后
ErrorCode backup_result = backup_registry();
if (backup_result != ERR_OK)
{
int choice = IupAlarm("警告", "备份失败!是否继续保存?",
"继续保存", "取消", NULL);
if (choice != 1)
return IUP_DEFAULT;
}
```
### 2.3 TABTITLE 设置在 Dialog 而非 Tabs 控件上
- **文件**`src/ui/main_window.c`
- **问题**`TABTITLE0``TABTITLE1``IupTabs` 属性,不是 `IupDialog` 属性,导致语言切换后选项卡标题不更新
- **修复**:先获取 Tabs 控件句柄,再设置 TABTITLE
### 2.4 list_dropfiles_cb 使用 ANSI 版本 API
- **文件**`src/controller/callbacks_search.c`
- **问题**`GetFileAttributesA` 无法正确处理中文等 Unicode 字符路径
- **修复**:改用 `utf8_to_wide` + `GetFileAttributesW`
### 2.5 load_all_paths 用户路径加载失败时未通知用户
- **文件**`src/controller/callbacks_sys.c`
- **修复**:添加 else 分支,在用户路径加载失败时弹出提示
### 2.6 escape_json_string 未处理所有控制字符
- **文件**`src/core/import_export.c`
- **问题**:只处理了 `\\``"``\n``\r``\t`,其他 0x00-0x1F 控制字符未转义
- **修复**:添加 `\b``\f` 处理,其他控制字符使用 `\uXXXX` 格式
### 2.7 CreateDirectoryW 不创建中间目录
- **文件**`src/utils/os_env.c`
- **修复**:改用 `SHCreateDirectoryExW` 递归创建目录
### 2.8 ExportData 浅拷贝隐患
- **文件**`include/core/import_export.h`
- **修复**:添加文档注释说明只读语义,防止误用 `clear_string_list`
### 2.9 export_paths_to_file 未检查 fprintf 返回值
- **文件**`src/core/import_export.c`
- **修复**:在 `fclose` 后检查 `ferror(fp)`,发现错误时返回 `ERR_FAILED`
---
## 三、低严重度问题(13 项)
### 3.1 JSON 解析器代码重复
- **文件**`src/core/import_export.c`
- **修复**:合并 system/user 数组解析逻辑为统一的键名检测机制
### 3.2 get_app_context_from_dlg 依赖指针到字符串的不安全转换
- **状态**:保留现状(IUP 框架标准用法)
### 3.3 putenv 使用字符串字面量的可移植性问题
- **文件**`src/main.c`
- **修复**:改用 `_wputenv_s`
### 3.4 path_manager_clean 时间复杂度为 O(n³)
- **文件**`src/core/path_manager.c`
- **修复**:使用标记+批量删除优化为 O(n²)
### 3.5 全局日志状态非线程安全
- **状态**:保留现状(当前是单线程 GUI 应用)
### 3.6 localtime 返回静态缓冲区指针,非线程安全
- **文件**`src/utils/os_env.c``src/core/import_export.c`
- **修复**:改用 `localtime_s`
### 3.7 JSON 解析器原地修改输入缓冲区
- **修复**:重写解析器时已解决,不再修改原始缓冲区
### 3.8 JSON 解析器对 `\\"` 场景处理错误
- **修复**:重写解析器时已解决,使用 `is_quote_escaped` 检查连续反斜杠
### 3.9 TXT 格式导入只能导入到系统路径
- **文件**`src/controller/callbacks_io.c`
- **修复**:添加选择对话框,允许用户选择导入到系统变量或用户变量
### 3.10 trim_whitespace 未检查 NULL 输入
- **文件**`src/core/import_export.c`
- **修复**:添加防御性 NULL 检查
### 3.11 is_json_file 使用 strcasecmp 限制可移植性
- **文件**`src/core/import_export.c`
- **修复**:改用 `_stricmp`MSVC/MinGW 都支持)
### 3.12 localtime 可能返回 NULL
- **修复**:改用 `localtime_s`,无需 NULL 检查
### 3.13 main.c 中 _() 嵌套调用可能崩溃
- **文件**`src/main.c`
- **问题**`_(lua_config_get_string(...))` 嵌套调用,若配置键不存在,`gettext(NULL)` 行为未定义
- **修复**:先将结果保存到临时变量并检查非 NULL
```c
// 修复前
IupMessage(_("Warning"), _(lua_config_get_string("status", "admin_warning")));
// 修复后
const char *admin_msg = lua_config_get_string("status", "admin_warning");
IupMessage(_("Warning"), admin_msg ? _(admin_msg) : "需要管理员权限才能编辑环境变量");
```
---
## 四、第一轮修复(原10条锐评)
| # | 问题 | 状态 |
|---|------|------|
| 1 | core层用了iup.h(架构矛盾) | ✅ 已修复 |
| 2 | main.c管理员权限检测代码灾难 | ✅ 已修复 |
| 3 | refresh_main_window_ui重复代码 | ✅ 已修复 |
| 4 | 字符串溢出风险(buffer不统一) | ✅ 已修复 |
| 5 | 错误码乱用(ERR_NULL_PTR误用) | ✅ 已修复 |
| 6 | 硬编码字符串满天飞 | ✅ 已修复 |
| 7 | backup_registry未实现 | ✅ 已修复 |
| 8 | callbacks.c 600行回调地狱 | ✅ 已拆分 |
| 9 | StringList封装不透明 | ✅ 已修复 |
| 10 | refresh_single_list_style黑盒 | ✅ 已修复 |
---
## 五、修改文件清单
| 文件 | 修改类型 |
|------|----------|
| `include/utils/string_ext.h` | 添加访问器函数声明 |
| `include/utils/ui_constants.h` | 新建,控件名称常量 |
| `include/core/import_export.h` | 添加文档注释 |
| `include/controller/callbacks_internal.h` | 新建,内部辅助函数声明 |
| `src/utils/string_ext.c` | 修复 realloc/strdup 安全性 |
| `src/utils/os_env.c` | 修复 wcsftime、localtime、目录创建 |
| `src/core/registry_service.c` | 修复 malloc 错误处理 |
| `src/core/import_export.c` | 重写 JSON 解析器、修复转义函数 |
| `src/core/path_manager.c` | 优化清理算法、修复错误码 |
| `src/main.c` | 修复 putenv、_() 嵌套调用 |
| `src/ui/main_window.c` | 修复 TABTITLE 设置位置 |
| `src/controller/callbacks.c` | 拆分后只保留辅助函数 |
| `src/controller/callbacks_basic.c` | 新建,基础 CRUD 回调 |
| `src/controller/callbacks_nav.c` | 新建,导航回调 |
| `src/controller/callbacks_search.c` | 新建,搜索/拖拽回调 |
| `src/controller/callbacks_io.c` | 新建,导入导出回调 |
| `src/controller/callbacks_sys.c` | 新建,系统操作回调 |
| `CMakeLists.txt` | 添加新源文件 |
---
## 六、编译验证
```bash
cmake -B build -G "MinGW Makefiles"
cmake --build build
```
**结果**:编译通过,无错误,无警告。
---
## 七、剩余已知问题(设计决策,非 Bug)
| 问题 | 原因 | 建议 |
|------|------|------|
| IUP 指针存储模式 | IUP 框架标准用法 | 保持现状 |
| 日志线程安全 | 当前是单线程应用 | 多线程时再处理 |
| localtime 线程安全 | 已改用 localtime_s | 已修复 |
---
*审查完成于 2026-04-28*
@@ -0,0 +1,170 @@
# 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 读取失败:应视策略决定是否整体阻断。若当前目标是“完整备份后再保存”,则任一关键读取失败都应阻断。
- 仅保存阶段失败:备份已经成功生成时,仍要验证现有失败提示是否准确,避免把备份失败和写入失败混为一类。
+11 -2
View File
@@ -32,8 +32,17 @@ int btn_ok_cb(Ihandle *self)
if (backup_result != ERR_OK)
{
log_error("Backup failed: error code %d", backup_result);
int choice = IupAlarm("警告", "备份失败!是否继续保存?\n(继续保存可能导致无法恢复)",
"继续保存", "取消", NULL);
const char *reason = "未知错误";
if (backup_result == ERR_FAILED)
reason = "无法获取 AppData 路径";
else if (backup_result == ERR_FILE_NOT_FOUND)
reason = "无法创建备份目录或文件";
else if (backup_result == ERR_REGISTRY_FAILED)
reason = "无法读取注册表中的 PATH 值";
char msg[512];
snprintf(msg, sizeof(msg), "备份失败!原因:%s\n\n是否继续保存?\n(继续保存可能导致无法恢复)", reason);
int choice = IupAlarm("警告", msg, "继续保存", "取消", NULL);
if (choice != 1)
return IUP_DEFAULT;
}
+43 -6
View File
@@ -1,5 +1,6 @@
#include "utils/os_env.h"
#include "utils/string_ext.h"
#include "utils/logger.h"
#include <windows.h>
#include <stdio.h>
#include <stdlib.h>
@@ -47,19 +48,29 @@ int is_path_valid(const char *path)
// 备份注册表
// 备份到 %APPDATA%/PathEditor/backups/ 目录下
// 返回值:
// ERR_OK - 备份成功(系统和用户 PATH 都已备份)
// ERR_FAILED - AppData 路径获取失败
// ERR_FILE_NOT_FOUND - 备份文件创建失败
// ERR_REGISTRY_FAILED - 系统和用户 PATH 都读取失败
ErrorCode backup_registry(void)
{
// 获取 AppData 路径
wchar_t appdata_path[MAX_PATH];
if (SHGetFolderPathW(NULL, CSIDL_APPDATA, NULL, 0, appdata_path) != S_OK)
{
log_error("Failed to get AppData path");
return ERR_FAILED;
}
// 创建备份目录(递归创建中间目录)
wchar_t backup_dir[MAX_PATH];
swprintf(backup_dir, MAX_PATH, L"%s\\PathEditor\\backups", appdata_path);
SHCreateDirectoryExW(NULL, backup_dir, NULL);
if (SHCreateDirectoryExW(NULL, backup_dir, NULL) != ERROR_SUCCESS)
{
log_error("Failed to create backup directory: %ls", backup_dir);
return ERR_FILE_NOT_FOUND;
}
// 生成时间戳
time_t t = time(NULL);
@@ -75,11 +86,15 @@ ErrorCode backup_registry(void)
// 打开文件
FILE *fp = _wfopen(backup_file, L"w, ccs=UTF-8");
if (!fp)
return ERR_FAILED;
{
log_error("Failed to create backup file: %ls", backup_file);
return ERR_FILE_NOT_FOUND;
}
// 备份系统 PATH
HKEY hKey;
int success = 0;
int sys_ok = 0;
int user_ok = 0;
if (RegOpenKeyExW(HKEY_LOCAL_MACHINE,
L"SYSTEM\\CurrentControlSet\\Control\\Session Manager\\Environment",
@@ -96,13 +111,17 @@ ErrorCode backup_registry(void)
{
fwprintf(fp, L"# System PATH Backup\n");
fwprintf(fp, L"%s\n\n", buffer);
success = 1;
sys_ok = 1;
}
free(buffer);
}
}
RegCloseKey(hKey);
}
else
{
log_warn("Failed to open system PATH registry key");
}
// 备份用户 PATH
if (RegOpenKeyExW(HKEY_CURRENT_USER,
@@ -120,14 +139,32 @@ ErrorCode backup_registry(void)
{
fwprintf(fp, L"# User PATH Backup\n");
fwprintf(fp, L"%s\n", buffer);
success = 1;
user_ok = 1;
}
free(buffer);
}
}
RegCloseKey(hKey);
}
else
{
log_warn("Failed to open user PATH registry key");
}
fclose(fp);
return success ? ERR_OK : ERR_FAILED;
// 判断备份结果
if (!sys_ok && !user_ok)
{
log_error("Backup failed: both system and user PATH read failed");
return ERR_REGISTRY_FAILED;
}
if (!sys_ok)
log_warn("Backup partial: system PATH read failed, only user PATH backed up");
if (!user_ok)
log_warn("Backup partial: user PATH read failed, only system PATH backed up");
log_info("Backup completed: sys=%d, user=%d, file=%ls", sys_ok, user_ok, backup_file);
return ERR_OK;
}