Skip to content

fix(cli): validate numeric arguments without truncation - #346

Open
blackdragoon26 wants to merge 1 commit into
1a1a11a:developfrom
blackdragoon26:fix/cli-numeric-parsing
Open

blackdragoon26 wants to merge 1 commit into
1a1a11a:developfrom
blackdragoon26:fix/cli-numeric-parsing

Conversation

@blackdragoon26

Copy link
Copy Markdown

What does this PR do?

Fixes numeric option parsing in cachesim. Malformed values such as --hashpower=8junk and --sample-ratio=nan now produce an error instead of silently changing simulation settings.

Request limits are parsed as 64-bit integers, so --num-req=4294967306 no longer becomes 10. Existing supported values, including --num-req=-1, are preserved.

Related to #206.

Type of change

  • Bug fix
  • New eviction / admission / prefetch algorithm
  • New trace reader or trace format
  • Performance improvement
  • Documentation
  • Build / CI
  • Other:

How was it tested?

Tested on macOS with a Debug build. All 10 CTest targets pass with ctest --test-dir _build_dbg --output-on-failure --parallel 4, including all 260 CLI checks.

The 56 new numeric checks produce 36 failures with the original parser and none with this fix. Formatting, shell syntax, and whitespace checks pass.

Linux was not tested locally. The existing warning in libCacheSim/bin/debug/main.cpp remains.

  • ctest --test-dir _build --output-on-failure passes, tested using _build_dbg
  • Build is warning-free (CI uses -Wall -Wextra -Werror)

Results

Invalid numeric arguments are rejected with a clear error. Large request limits retain their full value, and valid inputs continue to work.

Checklist

  • Formatted with clang-format (or the pre-commit hook from scripts/setup_hooks.sh)
  • Added or updated tests
  • Added or updated documentation
  • New algorithms are registered in the CLI and listed in the README

This branch has not been deployed

No deployments
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.

1 participant