Skip to content

mktemp: Fix template validation to require trailing consecutive Xs#10224

Merged
cakebaker merged 5 commits into
uutils:mainfrom
Xylphy:mktemp_consecutive_x
Jan 15, 2026
Merged

mktemp: Fix template validation to require trailing consecutive Xs#10224
cakebaker merged 5 commits into
uutils:mainfrom
Xylphy:mktemp_consecutive_x

Conversation

@Xylphy

@Xylphy Xylphy commented Jan 13, 2026

Copy link
Copy Markdown
Contributor

Closes #10187

The validation logic now checks only the trailing placeholder block, as GNU mktemp requires. It walks backward from the end of the template and counts consecutive X bytes. The template is accepted only if the trailing run has length ≥ 3.

Problem
The previous implementation accepted templates containing "XXX" anywhere, which incorrectly allowed cases where the template does not end with a sufficient run of Xs (e.g. XXX_foo) or where an earlier "XXX" masked an invalid suffix.

Implementation details

  • Scans only the suffix: starts at s.len() and walks left while bytes are b'X'.
  • No allocations; runtime proportional to the trailing X run.
  • UTF-8 safe: we only compare ASCII X bytes and compute indices from the end boundary.

Result

  • Rejects templates without ≥3 trailing Xs
  • Aligns behavior with GNU mktemp / coreutils validation rules

@sylvestre

Copy link
Copy Markdown
Contributor

Please add tests to make sure we don't regress in the future

Comment thread src/uu/mktemp/src/mktemp.rs Outdated
Comment thread tests/by-util/test_mktemp.rs Outdated
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

GNU test failed: tests/fmt/non-space. tests/fmt/non-space is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/misc/mktemp. tests/misc/mktemp is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/sort/sort-debug-warn. tests/sort/sort-debug-warn is passing on 'main'. Maybe you have to rebase?

@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

GNU test failed: tests/misc/mktemp. tests/misc/mktemp is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)

Change logic to only accept templates where the run of Xs containing the final 'X' is at least 3 characters long. This fixes cases like "XXX_XX", which should error with "too few X's in template", and allows "tempXXXlate", matching GNU mktemp
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/shuf/shuf-reservoir (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/sort/sort-stale-thread-mem (fails in this run but passes in the 'main' branch)

Comment thread src/uu/mktemp/src/mktemp.rs Outdated
@codspeed-hq

codspeed-hq Bot commented Jan 15, 2026

Copy link
Copy Markdown

CodSpeed Performance Report

Merging this PR will degrade performance by 27.39%

Comparing Xylphy:mktemp_consecutive_x (b74bdec) with main (2c75e71)

Summary

⚡ 2 improved benchmarks
❌ 7 regressed benchmarks
✅ 273 untouched benchmarks
⏩ 38 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Memory sort_unique_locale[500000] 33.6 MB 39.8 MB -15.51%
Memory sort_accented_data[500000] 22.1 MB 28.3 MB -21.79%
Memory sort_numeric[500000] 75.5 MB 79.2 MB -4.67%
Memory sort_mixed_data[500000] 22.9 MB 27.1 MB -15.71%
Memory sort_key_field[500000] 47.8 MB 51.8 MB -7.62%
Memory sort_ascii_utf8_locale 6.7 MB 6.2 MB +7.93%
Memory sort_long_line[160000] 712.6 KB 981.4 KB -27.39%
Memory sort_ascii_only[500000] 22.2 MB 28.3 MB -21.77%
Memory du_deep_tree[(100, 3)] 157.9 KB 146.6 KB +7.72%

Footnotes

  1. 38 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.

@cakebaker
cakebaker merged commit 5fd3459 into uutils:main Jan 15, 2026
220 of 257 checks passed
@cakebaker

Copy link
Copy Markdown
Contributor

Thanks for your PR!

@Xylphy
Xylphy deleted the mktemp_consecutive_x branch January 15, 2026 20:58
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.

mktemp: does not validate consecutive X's at template end

3 participants