printf: fix panic on width >65535 for numeric conversions - #14185
printf: fix panic on width >65535 for numeric conversions#14185Talha-Dmr wants to merge 1 commit into
Conversation
878d191 to
c8f0bc3
Compare
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Fixes printf panics when formatting numeric conversions with widths larger than u16::MAX by avoiding Rust’s dynamic-width formatter limitation and adding regression coverage for large numeric widths.
Changes:
- Add regression tests ensuring
%65536d/%65536xno longer panic and produce correctly padded output. - Replace
write!dynamic-width formatting in numeric output with manual padding logic for large widths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/by-util/test_printf.rs | Adds regression tests for numeric widths above u16::MAX to prevent panics. |
| src/uucore/src/lib/features/format/num_format.rs | Implements manual padding/zero-padding in write_output to support large widths without formatter panics. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
c8f0bc3 to
56fa417
Compare
56fa417 to
34f61b0
Compare
34f61b0 to
838c2cd
Compare
|
please close the issues that have been addressed |
|
Thanks @sylvestre — resolved the addressed Copilot threads (chunked padding, DRY/unused var cleanup, checked_add + target width check, generic pad_byte handling, and InvalidInput kind). The remaining |
838c2cd to
87320d2
Compare
|
GNU testsuite comparison: |
Merging this PR will degrade performance by 10.16%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | split_lines |
9.2 ms | 10.6 ms | -13.16% |
| ❌ | Simulation | split_numeric_suffix |
9.7 ms | 11.1 ms | -12.55% |
| ❌ | Simulation | split_bytes |
644.5 µs | 675.1 µs | -4.53% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing Talha-Dmr:fix/printf-large-width-num-format (38a5d7b) with main (0d8310c)
Footnotes
-
50 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
87320d2 to
73f9755
Compare
73f9755 to
bc7a726
Compare
Avoid Rust formatter panic 'Formatting argument out of range' when width exceeds u16::MAX. Use manual chunked padding in write_output instead of format! dynamic width, similar to zero_pad_to for large precisions. Handles Left, RightSpace and RightZero alignments up to MAX_FORMAT_WIDTH without allocating a large buffer. Avoid allocation in sign branch and validate requested width directly. Fixes uutils#13850.
bc7a726 to
38a5d7b
Compare
|
Duplicate of #13857 |
Fixes #13850
printf panicked with 'Formatting argument out of range' for numeric widths above 65535. Rust's formatter limits width to u16::MAX. Use manual padding in write_output instead of format! dynamic width, similar to zero_pad_to for large precisions.
Test: printf '%65536d' 5 now outputs 65536 bytes (was panic), printf tests pass.