diff options
| author | Paul Buetow <paul@buetow.org> | 2026-05-23 22:54:34 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-05-23 22:54:34 +0300 |
| commit | 4fbbb78125b58a7360d9441fbd175e658e03f534 (patch) | |
| tree | f330e1fb83abcafec60acaee3e8c4462656a7614 | |
| parent | 976a47a21d9b04bd1b2f3f62137a1f18b34f947d (diff) | |
fix: prevent stack corruption in popTwo() and FastPower()
popTwo() used to pop b first, then a. If the second pop failed, b was
already gone — the stack was silently corrupted.
Fix: add ensureStackLength() check before any pop, preserving stack
state on error. If the second pop fails (should not happen after the
check, but added as belt-and-suspenders), restore b to the stack.
FastPower() had the same issue (two sequential popStack calls).
Replaced with popTwo() for consistency.
Also fixed rpn_test.go: TestParseAndEvaluateInsufficientOperands
shared a single RPN instance across test cases, which relied on the
old stack-corruption behavior. Now each test case gets a fresh instance.
| -rw-r--r-- | internal/rpn/operations_arithmetic.go | 7 | ||||
| -rw-r--r-- | internal/rpn/operations_helpers.go | 8 | ||||
| -rw-r--r-- | internal/rpn/rpn_test.go | 5 |
3 files changed, 11 insertions, 9 deletions
diff --git a/internal/rpn/operations_arithmetic.go b/internal/rpn/operations_arithmetic.go index 8504d34..fd1b0fa 100644 --- a/internal/rpn/operations_arithmetic.go +++ b/internal/rpn/operations_arithmetic.go @@ -233,12 +233,7 @@ func (o *Operations) Modulo(stack *Stack) error { // FastPower pops two values from stack, raises first to integer power of second (a ** b), and pushes result. // Uses binary exponentiation for efficiency with large integer exponents. func (o *Operations) FastPower(stack *Stack) error { - b, err := popStack(stack, "**") - if err != nil { - return err - } - - a, err := popStack(stack, "**") + a, b, err := popTwo(stack, "**") if err != nil { return err } diff --git a/internal/rpn/operations_helpers.go b/internal/rpn/operations_helpers.go index 3d5f944..afdfa0e 100644 --- a/internal/rpn/operations_helpers.go +++ b/internal/rpn/operations_helpers.go @@ -19,7 +19,12 @@ func popStack(stack *Stack, op string) (StackValue, error) { } // popTwo pops two values from the stack for binary operations. +// Uses ensureStackLength to prevent stack corruption on second Pop failure. func popTwo(stack *Stack, op string) (StackValue, StackValue, error) { + if err := ensureStackLength(stack, 2, op); err != nil { + return nil, nil, err + } + b, err := stack.Pop() if err != nil { return nil, nil, fmt.Errorf("insufficient operands for %s: %w", op, err) @@ -27,6 +32,9 @@ func popTwo(stack *Stack, op string) (StackValue, StackValue, error) { a, err := stack.Pop() if err != nil { + // Should not happen given ensureStackLength check above. + // If it does (concurrent modification), restore b and return error. + stack.Push(b) return nil, nil, fmt.Errorf("insufficient operands for %s: %w", op, err) } diff --git a/internal/rpn/rpn_test.go b/internal/rpn/rpn_test.go index 0460271..494e377 100644 --- a/internal/rpn/rpn_test.go +++ b/internal/rpn/rpn_test.go @@ -302,9 +302,6 @@ func TestParseAndEvaluateUnknownToken(t *testing.T) { } func TestParseAndEvaluateInsufficientOperands(t *testing.T) { - v := NewVariables() - r := NewRPN(v) - tests := []struct { name string input string @@ -316,6 +313,8 @@ func TestParseAndEvaluateInsufficientOperands(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + v := NewVariables() + r := NewRPN(v) _, err := r.ParseAndEvaluate(tt.input) if err == nil { t.Errorf("%q should return error for insufficient operands", tt.input) |
