[eylulseyit]

GNU rejects comma-separated mode strings containing empty clauses (e.g., u+x,, '') or bare octal values mixed with other clauses (e.g., 644,u+x). Previously, these were accepted, leading to incorrect permissions, such as mkdir -m '' creating a 0777 directory or install -m '' a b leaving a 000 file.

This introduces a check_clause validation helper in uucore::mode, called from both parse_chmod (used by mkdir, install, mkfifo, and mknod) and from chmod. Octals with an explicit operator remain supported in lists (e.g. +644,u+x, u+x,=600) to match GNU behavior. Existing unit tests asserting the prior behavior have been updated to expect errors or use valid forms like =644,....
The errors are a plain "invalid mode", like GNU. The message is hardcoded like the existing invalid mode (...) in parse_symbolic, because adding a translated message to uucore makes dd's locale table grow (see the CodSpeed comment below).

The dedicated loop in chmod is retained for now to keep this change focused; consolidating the parsing paths can follow in a separate PR. Whitespace trimming around clauses is unchanged, and #12709 is out of scope.

Closes #14421

[codspeed]

Merging this PR will not alter performance

1 warning, 1 note

⚠️ 119 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

ℹ️ 1 benchmark has no flamegraph

The flamegraph wasn't generated: most of the time was spent in runtime internals, which would leave nothing useful to analyze.

See affected benchmarks

✅ 401 untouched benchmarks
⏩ 70 skipped benchmarks¹

---

_{Comparing eylulseyit:mode-reject-empty-and-mixed-octal (4eb297a) with main (1d6ca65)²}

Footnotes

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

2. No successful run was found on main (0aae922) during the generation of this report, so 1d6ca65 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[eylulseyit]

Hi, I looked into the CodSpeed report a bit. As far as I can tell, the dd memory regression isn't coming from the mode changes (dd doesn't use that code). It seems to come from the two new messages in uucore's en-US.ftl.
dd loads uucore's messages and its own into one hash map at startup. On main that's already 112 entries, which is right at the limit for a 128-bucket hashbrown table, and it's been like that since September. So any new message in uucore or dd would push it to 256 buckets (about 12 KB, once). I tried adding only the ftl lines on top of main and saw the same jump.
Is that acceptable here, or would you prefer I handle it differently?
For mv and hostname I only see a few thousand extra instructions on the first call (locale loading), so I'm not sure where the rest of the difference comes from. Am I missing something there?

[eylulseyit]

I deleted the two error messages I had added to the locale files. Errors now just say "invalid mode", like GNU. The message is hardcoded, like the existing invalid mode (...) in mode.rs. The dd, mv, and hostname regressions are gone now. Any new message in uucore will hit the same limit in dd, so it might be worth keeping in mind.

[sylvestre]

this is the same split/offset loop as in calculate_new_mode, could they share it?

[eylulseyit]

Yes. I deleted check_mode because calculate_new_mode already does the same job. Since #15166 was merged, calculate_new_mode is called once on main before any file is read, so an invalid mode is rejected before the files are touched.

[sylvestre]

for 644, the error says "octal mode 644 must be the whole mode", which is confusing. Could we check for empty clauses first?

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/misc/usage_vs_getopt (fails in this run but passes in the 'main' branch)