Skip to content

fix: add defensive checks in excel formula evaluation#46

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Johnson-zs:master
Nov 10, 2025
Merged

fix: add defensive checks in excel formula evaluation#46
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Johnson-zs:master

Conversation

@Johnson-zs
Copy link
Contributor

  1. Added null check for targetName.m_stack before accessing first element to prevent potential crashes
  2. Added error handling when stack is empty by returning oERR operand and setting error flag
  3. Improved exception handling by ensuring name.m_stack is never empty after exceptions
  4. Added proper error state flags (m_hasError) when errors occur

Log: Fixed potential crashes in Excel formula evaluation with invalid references

Influence:

  1. Test formulas referencing non-existent names or invalid references
  2. Verify error handling when formula stack becomes empty
  3. Test exception scenarios to confirm proper error state is maintained
  4. Verify calculations continue normally when references are valid

fix: 在Excel公式计算中添加防御性检查

  1. 在访问第一个元素前添加对 targetName.m_stack 的空检查以防止潜在崩溃
  2. 当栈为空时通过返回 oERR 操作数并设置错误标志添加错误处理
  3. 通过确保异常后 name.m_stack 不会为空来改进异常处理
  4. 发生错误时添加正确的错误状态标志(m_hasError)

Log: 修复了引用无效对象时Excel公式计算可能崩溃的问题

Influence:

  1. 测试引用不存在名称或无效引用的公式
  2. 验证当公式栈为空时的错误处理
  3. 测试异常场景以确认正确的错误状态被维护
  4. 当引用有效时验证计算是否正常继续

1. Added null check for targetName.m_stack before accessing first
element to prevent potential crashes
2. Added error handling when stack is empty by returning oERR operand
and setting error flag
3. Improved exception handling by ensuring name.m_stack is never empty
after exceptions
4. Added proper error state flags (m_hasError) when errors occur

Log: Fixed potential crashes in Excel formula evaluation with invalid
references

Influence:
1. Test formulas referencing non-existent names or invalid references
2. Verify error handling when formula stack becomes empty
3. Test exception scenarios to confirm proper error state is maintained
4. Verify calculations continue normally when references are valid

fix: 在Excel公式计算中添加防御性检查

1. 在访问第一个元素前添加对 targetName.m_stack 的空检查以防止潜在崩溃
2. 当栈为空时通过返回 oERR 操作数并设置错误标志添加错误处理
3. 通过确保异常后 name.m_stack 不会为空来改进异常处理
4. 发生错误时添加正确的错误状态标志(m_hasError)

Log: 修复了引用无效对象时Excel公式计算可能崩溃的问题

Influence:
1. 测试引用不存在名称或无效引用的公式
2. 验证当公式栈为空时的错误处理
3. 测试异常场景以确认正确的错误状态被维护
4. 当引用有效时验证计算是否正常继续
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Johnson-zs

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@deepin-ci-robot
Copy link

deepin pr auto review

我来对这段代码的修改进行审查:

  1. 语法逻辑:
  • 修改后的代码在语法上没有问题,增加了对空栈的检查,这是一个很好的防御性编程实践。
  • 异常处理部分也增加了必要的初始化,确保对象状态的一致性。
  1. 代码质量:
  • 优点:
    • 增加了对空栈的防御性检查,提高了代码的健壮性
    • 在异常处理中增加了状态重置,确保对象始终处于有效状态
    • 添加了适当的注释说明修改的目的
  • 建议改进:
    • 可以考虑将错误处理逻辑抽取为一个单独的函数,避免重复代码
    • 异常处理中的错误状态设置可以统一到一个地方处理
  1. 代码性能:
  • 增加的检查对性能影响很小,只是增加了一个简单的条件判断
  • 异常处理中的操作也只会在发生异常时执行,不会影响正常情况下的性能
  1. 代码安全:
  • 修改大大提高了代码的安全性:
    • 防止了对空栈的访问,避免了可能的程序崩溃
    • 确保了异常情况下对象状态的一致性
    • 增加了错误标志,便于上层调用者处理错误情况

改进建议:

  1. 可以考虑定义一个常量或枚举来表示错误状态,而不是直接使用 oERR
  2. 可以将错误处理逻辑封装成单独的函数,例如:
void handleError(Name& name) {
    if (name.m_stack.empty()) {
        name.m_stack.push_back(Operand(oERR));
    }
    name.m_hasError = true;
    name.m_evaluated = true;
}
  1. 考虑添加日志记录,记录错误发生的具体位置和原因,便于调试

总体来说,这是一个很好的改进,提高了代码的健壮性和安全性,同时保持了良好的性能。建议的改进主要是为了提高代码的可维护性和可读性。

@Johnson-zs
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Nov 10, 2025

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 5d509a2 into linuxdeepin:master Nov 10, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants