fix: fix memory leak in moveText when from equals to - #598
Conversation
1. Root cause: TextEdit::moveText() missing else branch for from == to, heap-allocated UndoList, DragInsertTextUndoCommand and DeleteBackCommand objects never get freed 2. Fix: add else branch to delete list, insertCommand and delCommand (if non-null) when from == to, no behavior change for other paths 3. Impact: only affects drag-drop text to same position scenario, no regression risk for normal move operations Influence: 1. Test drag selected text to the same position (from == to) 2. Test drag text forward (from < to) with undo 3. Test drag text backward (from > to) with undo fix: 修复moveText函数from等于to时的内存泄漏 1. 根因:TextEdit::moveText() 缺少 from == to 的 else 分支, 堆分配的 UndoList、DragInsertTextUndoCommand 和 DeleteBackCommand 对象未被释放导致内存泄漏 2. 方案:添加 else 分支,在 from == to 时释放 list、 insertCommand 及 delCommand(非空时),不影响其他路径 3. 影响:仅影响拖拽文本到原位置的场景,对正常移动操作无回归风险 Influence: 1. 测试拖拽选中文本到原位置(from == to) 2. 测试向前拖拽文本(from < to)并撤销 3. 测试向后拖拽文本(from > to)并撤销 PMS: BUG-185
There was a problem hiding this comment.
Sorry @pengfeixx, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 3 days and 16 hours by commenting @sourcery-ai review. Upgrade to get a review now.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThe PR fixes a memory leak in Flow diagram for moveText cleanup when source equals destinationflowchart TD
A["TextEdit::moveText(from, to, text, copy)"] --> B{from equals to}
B -->|No| C["appendCom(insertCommand)"]
C --> D["m_pUndoStack->push(list)"]
B -->|Yes| E["delete list"]
E --> F["delete insertCommand"]
F --> G{delCommand non-null}
G -->|Yes| H["delete delCommand"]
G -->|No| I["Cleanup complete"]
H --> I
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 语法正确,逻辑清晰。else 分支正确处理了 from == to 的情况,delete list、delete insertCommand 操作正确——这些对象通过 new 在堆上分配但从未被添加到撤销栈。if (delCommand) 空指针检查正确,因为 delCommand 仅在 !copy 时分配。无双重释放风险:在 from == to 路径中,命令对象从未被 appendCom 到 list 中。 2. 代码质量 ✅评价: 优秀 ✅ 通过 潜在问题:
建议: 代码结构清晰,注释完整。建议在 else 分支中添加 qDebug() << "Moving text with from == to (no-op, cleanup)"; 以保持与其他分支的调试日志一致性。 3. 代码性能 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 性能良好,资源使用合理。本次修复消除了 from == to 场景下的内存泄漏,堆分配的 UndoList、DragInsertTextUndoCommand 和 DeleteBackCommand 对象现在被正确释放。无不必要的操作。 4. 代码安全 🔒评价: 优秀 ✅ 通过
安全漏洞详情: 建议: 存在0个安全漏洞。本次变更未引入任何安全漏洞,修复内存泄漏提升了代码的内存安全性。 💡 改进建议代码示例// 建议在 else 分支添加调试日志以保持一致性
} else {
qDebug() << "Moving text with from == to (no-op, cleanup)";
delete list;
delete insertCommand;
if (delCommand) {
delete delCommand;
}
}本报告由 AI 代码审查工具自动生成 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, pengfeixx 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 |
fix: fix memory leak in moveText when from equals to
heap-allocated UndoList, DragInsertTextUndoCommand and
DeleteBackCommand objects never get freed
(if non-null) when from == to, no behavior change for other paths
regression risk for normal move operations
Influence:
fix: 修复moveText函数from等于to时的内存泄漏
堆分配的 UndoList、DragInsertTextUndoCommand 和
DeleteBackCommand 对象未被释放导致内存泄漏
insertCommand 及 delCommand(非空时),不影响其他路径
Influence:
PMS: BUG-185
Summary by Sourcery
Fix memory leaks in text moves where the source and destination positions are identical.
Bug Fixes: