Skip to content

Fix integer a^b and luaA_strtol's endptr - #9

Merged
rixnobis merged 1 commit into
mainfrom
fix-pow-strtol
Oct 6, 2026
Merged

rixnobis merged 1 commit into
mainfrom
fix-pow-strtol

Conversation

@rixnobis

@rixnobis rixnobis commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

Fixes #8.

a^b computed a^(b+1), and a negative exponent looped about 2^32 times. Now it squares and multiplies; a negative exponent gives 0 unless |a| is 1. luaA_strtol returned without setting endptr on a leading 0 in base 10, so 010 failed to compile. Now endptr is always set, to nptr when no digit was read, and the sign is read before the 0x prefix.

tests/numbers.c checks both on the host (16 failures on the old code) and runs in CI. tests/sample.lua now has 2^10 + 010: the old luac.ps-exe gives "malformed number near '010'", the new one stores the constant 1034.

Compiled bytecode changes for any script using ^ on constants, so splashedit-ng's host compiler and fixtures need the same change.

Summary by CodeRabbit

  • Bug Fixes
    • Improved string-to-integer conversion to handle leading whitespace, optional signs, and base detection, including hexadecimal prefixes. Invalid bases and inputs now report the correct stopping position.
    • Updated integer exponentiation to use a more efficient calculation while preserving wrapping behavior for nonnegative powers. Negative exponents now return defined results for bases 1 and -1, and zero for other bases.

luai_numpowimpl started from r = a, so a^b was a^(b+1), and a negative
b looped about 2^32 times on the console. It now squares and multiplies;
a negative exponent truncates to 0 unless |a| is 1.

luaA_strtol returned early on a leading 0 in base 10 without setting
endptr, which luaO_str2d then read, so 010 failed to compile. It now
skips spaces and the sign before looking for a prefix, and sets endptr
on every return, to nptr when no digit was read.

tests/numbers.c checks both on the host and runs in CI. Fixes #8.
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 45895945-09f8-4e5a-98fd-14db9273fb6f
📥 Commits

Reviewing files that changed from the base of the PR and between b1f2991 and ae62a07.

📒 Files selected for processing (5)
  • .github/workflows/build.yml
  • src/llibc.c
  • src/luaconf.h
  • tests/numbers.c
  • tests/sample.lua

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The integer power calculation and string-to-integer parser have changed. A standalone C test program checks both primitives, and the build workflow compiles and runs that program. The Lua sample adds an expression using integer power and an octal literal.

Changes

Integer number primitives

Layer / File(s) Summary
String-to-integer parsing
src/llibc.c
luaA_strtol now skips leading whitespace, accepts an optional sign, and converts digits for bases up to 36. It selects a base when the input base is zero and recognizes a hexadecimal prefix only when followed by a valid hexadecimal digit. It initializes *endptr to nptr and advances it when digits are parsed.
Integer power calculation
src/luaconf.h
luai_numpowimpl now uses exponentiation by squaring with unsigned intermediate multiplication. For negative exponents, it returns 1 for base 1, returns -1 or 1 for base -1 based on exponent parity, and returns 0 for other bases.
Number primitive checks and execution
tests/numbers.c, .github/workflows/build.yml, tests/sample.lua
The C test program checks power results and string parsing, including consumed-character counts. The build workflow compiles and runs the test program. The Lua sample assigns 2^10 + 010 to squares.folded and adds a compile-time-folding comment.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to ae62a

The number-primitive fixes and host checks are mergeable after normal checks; no actionable regression is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to ae62a

The changes remain within numeric primitives and automated checks, with no demonstrated new privilege or trust-boundary exposure. Corrected arithmetic changes computed constants; compatibility with separately maintained compilers and existing bytecode remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected production exposure is numeric interpretation and arithmetic within an embedding Lua process. The routed test entrypoint takes no command-line arguments and supplies fixed inputs; it does not create a production input channel. Tenant, service, or datastore exposure depends on embedding applications not supplied here.

Trust Boundaries and Controls

  • observed — The numeric consumer retains its rejection of no-digit conversion and trailing non-whitespace characters. Initializing the parser's end pointer before invalid-base or no-digit returns makes the consumer's existing validation deterministic for those cases.

Resilience and Maintainability Implications

  • observed — The host checks assert both parser values and consumed-character offsets, plus zero and negative power cases. The workflow stops this step on a nonzero test exit. These are configured checks; their execution results were not independently verified.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly names both main fixes: integer exponentiation and luaA_strtol's endptr handling.
Linked Issues check ✅ Passed Issue #8 requests fixes for integer exponentiation and luaA_strtol end-pointer handling. src/luaconf.h now computes powers by squaring and handles negative exponents without a long loop. `src/llib…
Out of Scope Changes check ✅ Passed The changes in src/luaconf.h, src/llibc.c, the number tests, the CI workflow, and tests/sample.lua all support issue #8 or verify its fixes. The PR introduces no demonstrated unrelated changes.
Full details: Docstring Coverage

Explanation

Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

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.

Integer build: a^b computes a^(b+1), and luaA_strtol leaves endptr unset

1 participant