Repository navigation
fix(libc): ignore non-wide lengths on character scans - #13049
Conversation
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed — looks good
The change normalizes ignored narrow-character scan lengths while preserving the single-l wide path, with regression coverage across string, stream, secure, and va-list entry points.
What I ran
Reviewed exact head ce32d855ba3817332ab44e454f95f6257836943d against current main e7d8e353362935a1f00e893d901b1debc671f5ce; the clean merge-preview tree is 6715ec4f2bcc30b5f5733ddbe0aa744f15f1fed1. Environment: Debian Linux x86-64, Python 3.11.2; no compiler or build was run.
git merge-tree --write-tree upstream/main refs/review/pr-13049-current
git diff --check upstream/main...refs/review/pr-13049-current
python3 tools/check_conventions.py --base upstream/main --head refs/review/pr-13049-current- The changed source, tests, call paths, and current CI were inspected. Conventions and whitespace checks passed.
- The PR body records author-run focused validation and its limitations; I did not independently rerun build-dependent tests.
Not verified
I did not build or run the C++ tests locally, and I did not verify physical-console or game behavior. Some hosted platform matrix jobs were still running or queued at review time.
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
A Windows run with GCC 16: Windows 11 Pro 10.0.22631, i9-9900K, MSYS2 UCRT64 GCC 16.1.0, Release,
AI-assisted: yes (Claude Code) |
|
Checked the diff against the scan paths: |
What
A character field such as
%llsin the title-bundled libc modules stores narrow bytes. Forwarding it to glibc can instead store 32-bit wide characters. The same discrepancy affects ignored length tokens on%cand%[fields.Normalize accepted non-wide lengths in host-backed character scans and in the secure scanner's generated directives. Preserve a single
l, numeric lengths, widths/suppression, literal%%and scanset members. Regression cases check consumed bytes and destination guards through ordinary, secure and va-list string/file APIs.Related work: #12428 supplies the Windows ordinary-parser length rule; #12468 supplies wide-field recognition/decoding. This implementation complements those paths. The new Windows cases exercise the secure paths changed here; the new Linux cases exercise all eight entry points.
Tested
Linux x86-64 Release, exact production/test source
ce32d855ba3817332ab44e454f95f6257836943d, based on main7a225bb303c1a452057000f32b70b56486625fd6. Clean merge preview against subsequently fetched main1189ec3003672bb364dd9f3119ead7b3976e3c83; public branches of related PRs were preserved.cmake --build build --parallel 2 --target libc guest_formatting_tests guest_wide_formatting_tests guest_safe_error_tests file_streams_tests ctest --test-dir build -V -R '^(guest_formatting|guest_wide_formatting|guest_safe_errors|file_streams)$'guest_formatting_testsandfile_streams_testsexecutables fail against coherent unmodified base libraries with the expected character-scan length failures. Restored candidate library bytes match the module used for reference comparison.sscanf/sscanf_sfscanf/fscanf_s700 full rows corrected; zero regressions in previously correct observed fields. Twenty further destination-prefix fields become correct while an existing short-field return mismatch remains. Existing EOF errno, wide/uppercase-conversion and short-character gaps remain explicit. The short-character gap is recorded in debt. This is not complete scanf coverage.
Authored probes, synthetic reference/main/change rows, positive/negative logs, source pins and hashes, SHA-256
7fceb0465f933407de4eab5d866804a957185ac902f1c8d32c483894d30cf233(download verified). No SDK binaries, game assets, saves or screenshots are included.Conventions/whitespace pass. Required overlap scan checked 1431 open PRs with no automatic positive overlap. Shared-file diffs of peers already conflicting with main were reviewed; their classifications are attached. No blanket peer-mergeability claim.
Windows/macOS execution is not locally qualified. Windows ordinary scans use the separate parser covered by #12428; this PR's Windows regression cases cover its four secure entry points. No console, title, graphics, audio or FPS claim.
Independent Windows validation, reported by bunbeans2-arch for topic
ce32d855ba3817332ab44e454f95f6257836943dagainst mainf7f03daf52: Windows 11 Pro 22631, i9-9900K, MSYS2 UCRT64 GCC 16.1, Release.file_streamsandguest_formattingpass 5/5; keeping the new tests while restoringFormatting.cppto main makes both fail 5/5; plain main passes 3/3. This is the contributor's run, not a new local Windows execution.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)