Repository navigation
stat: reject a % directive with flags or a precision - #15352
ozenyunus35 wants to merge 6 commits into
Conversation
`%0%`, `%-%` or `%.3%` printed `?`. Only a bare `%%` is the escape; GNU rejects the others as an invalid directive, after printing the text before it. Part of uutils#15348
GNU prints both through out_uint, which drops the `+` and ` ` flags, so `%+s` is `7`, not `+7`. Closes uutils#15348
|
|
||
| // A `%` after flags, a width or a precision (`%0%`, `%-%`, `%.3%`) is | ||
| // not the `%%` escape; GNU rejects it as an invalid directive. | ||
| if chars[j] == '%' { |
There was a problem hiding this comment.
the width branch above already does the same chars[j] == '%' check, could be dedup, no?
There was a problem hiding this comment.
Yes, the width branch now only handles the end of the format and leaves the % case to the check below.
| } | ||
|
|
||
| #[test] | ||
| fn test_size_ignores_sign_flags() { |
There was a problem hiding this comment.
please add a test for %+S with -f too
There was a problem hiding this comment.
I've dropped the %s / %S part from this PR, so it's back to the % after flags fix only.
I suspect you are just pointing your LLM at GNU's source code and telling it to copy what it finds, which is frustrating. |
|
|
||
| // Reject directives like `%<NUMBER>` by checking if width has been parsed. | ||
| if j >= bound || chars[j] == '%' { | ||
| if j >= bound { |
There was a problem hiding this comment.
now that the % check moved below, this block does the same as the check_bound on the next line, could be removed, no?
There was a problem hiding this comment.
Right, removed it: check_bound on the next line reports the same directive when the width ends the format.
Merging this PR will regress 1 benchmark
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | factor_five_38_bit_primes |
1.8 s | 2 s | -10.73% |
| ⚡ | Simulation | factor_three_39_bit_primes |
800.7 ms | 511 ms | +56.69% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing ozenyunus35:stat-invalid-percent-directive (89cdef7) with main (2fb77e8)
Footnotes
-
70 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. ↩
|
Closing this, as I can't follow up on it further. Thanks for the reviews. |
3 participants
Closes #15348
A
%reached after flags, a width or a precision (%0%,%-%,%#%,%++-+%,%.3%) printed?. Only a bare%%is the escape; GNU rejects the others as an invalid directive after printing the text before it, e.g.stat: '%0%': invalid directive.handle_percent_casealready did this for a width (%9%); it now does it for flags and a precision too.Test:
test_percent_after_flags_is_an_invalid_directive(the formats from the issue, plus%%still printing%), which fails without the change.I used an AI assistant to help draft parts of the code and tests; I reviewed the changes and ran the tests myself.