mkdir:Refactor: replace direct `libc` calls with `nix` wrappers for `umask`/`mkdirat` by mattsu2020 · Pull Request #11138 · uutils/coreutils

mattsu2020 · 2026-02-27T09:30:58Z

Summary

This PR replaces a few direct libc syscall usages with nix wrappers to reduce unsafe surface area and improve consistency in Unix-specific filesystem/process code.

github-actions · 2026-02-27T09:39:52Z

GNU testsuite comparison:

Skipping an intermittent issue tests/pr/bounded-memory (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/misc/io-errors is no longer failing!
Congrats! The gnu test tests/tail/tail-n0f is now passing!

anastygnome · 2026-02-27T10:35:52Z

src/uu/mkdir/src/mkdir.rs

@@ -10,6 +10,8 @@ use clap::parser::ValuesRef;
 use clap::{Arg, ArgAction, ArgMatches, Command};


Why are you putting this in this PR???

These imports are required for this refactor: UmaskGuard now uses nix::sys::stat::umask/Mode, and create_dir_with_mode uses DirBuilderExt::mode.

Each of these imports is only used ~~once~~twice, I think we can remove them and qualify the method calls instead for clarity.

Personally if it's less than three, I would just qualify them

xtqqczze · 2026-02-27T13:14:43Z

@mattsu2020 The change is sound, but the commit description isn't helpful. In general, replacing unsafe libc calls with nix in simple cases like this doesn't require extensive justification. I would also suggest that you squash your commits.

github-actions · 2026-02-27T13:33:39Z

GNU testsuite comparison:

GNU test failed: tests/tail/retry. tests/tail/retry is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/misc/io-errors is no longer failing!
Note: The gnu test tests/seq/seq-epipe is now being skipped but was previously passing.
Note: The gnu test tests/tail/pipe-f is now being skipped but was previously passing.
Congrats! The gnu test tests/tail/tail-n0f is now passing!

github-actions · 2026-02-27T22:56:32Z

GNU testsuite comparison:

Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/expand/bounded-memory is now being skipped but was previously passing.
Congrats! The gnu test tests/tail/tail-n0f is now passing!

codspeed-hq · 2026-02-27T22:59:45Z

Merging this PR will not alter performance

✅ 294 untouched benchmarks
⏩ 42 skipped benchmarks¹

_{Comparing mattsu2020:mkdir (95df9c8) with main (6de55b4)}

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

xtqqczze · 2026-02-27T23:42:32Z

@mattsu2020 Could you please run cargo fmt and then squash the commits into a single one? For the commit description, you might also ask your AI to make it concise :)

github-actions · 2026-02-27T23:59:29Z

GNU testsuite comparison:

GNU test failed: tests/rm/isatty. tests/rm/isatty is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

xtqqczze · 2026-02-28T11:36:10Z

src/uu/mkdir/src/mkdir.rs

+use nix::sys::stat::{Mode, umask};
 use std::ffi::OsString;
 use std::io::{Write, stdout};
+#[cfg(unix)]


this import can go back in create_dir_with_mode?

Thanks, good point. I’ll move DirBuilderExt back into create_dir_with_mode. Mode/umask will stay at module scope because UmaskGuard also uses them.

Moved the `std::os::unix::fs::DirBuilderExt` import from module-level to inside the `create_dir_with_mode` function where it's actually used. This improves code organization by keeping platform-specific imports closer to their usage and reduces the module's import surface.

github-actions · 2026-02-28T12:43:38Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/follow-name (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/date/resolution (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/cut/bounded-memory is now being skipped but was previously passing.
Congrats! The gnu test tests/expand/bounded-memory is now passing!
Note: The gnu test tests/env/env-signal-handler was skipped on 'main' but is now failing.

xtqqczze · 2026-02-28T13:40:34Z

src/uu/mkdir/src/mkdir.rs

-        let old_mask = unsafe { uucore::libc::umask(new_mask) };
-        Self(old_mask)
+    /// Set umask to 0 and return a guard that restores the original on drop.
+    fn set_zero() -> Self {


can rename this function to new, see existing pattern in:

coreutils/tests/by-util/test_env.rs

Lines 974 to 984 in 6100aee

struct SigpipeGuard {

previous: libc::sighandler_t,

}

#[cfg(unix)]

impl SigpipeGuard {

fn new() -> Self {

let previous = unsafe { libc::signal(libc::SIGPIPE, libc::SIG_IGN) };

Self { previous }

}

}

cakebaker mentioned this pull request Feb 27, 2026

Revert "Don't fail when sscache caused network error" #11140

Closed

anastygnome reviewed Feb 27, 2026

View reviewed changes

anastygnome suggested changes Feb 27, 2026

View reviewed changes

sylvestre force-pushed the mkdir branch from fe8933e to 125cc35 Compare February 27, 2026 13:23

mattsu2020 force-pushed the mkdir branch from 125cc35 to 9a59293 Compare February 27, 2026 22:46

mattsu2020 force-pushed the mkdir branch from 9a59293 to 4829bab Compare February 27, 2026 23:48

refactor(mkdir): use nix::umask and simplify guard

0840bb7

mattsu2020 force-pushed the mkdir branch from 4829bab to 0840bb7 Compare February 27, 2026 23:49

mattsu2020 requested a review from anastygnome February 28, 2026 06:40

xtqqczze reviewed Feb 28, 2026

View reviewed changes

		@@ -10,6 +10,8 @@ use clap::parser::ValuesRef;
		use clap::{Arg, ArgAction, ArgMatches, Command};

	struct SigpipeGuard {
	previous: libc::sighandler_t,
	}

	#[cfg(unix)]
	impl SigpipeGuard {
	fn new() -> Self {
	let previous = unsafe { libc::signal(libc::SIGPIPE, libc::SIG_IGN) };
	Self { previous }
	}
	}

Uh oh!

Conversation

mattsu2020 commented Feb 27, 2026

Summary

Uh oh!

github-actions bot commented Feb 27, 2026

Uh oh!

anastygnome Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

mattsu2020 Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

xtqqczze Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xtqqczze Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

xtqqczze commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 27, 2026

Uh oh!

github-actions bot commented Feb 27, 2026

Uh oh!

codspeed-hq bot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

xtqqczze commented Feb 27, 2026

Uh oh!

github-actions bot commented Feb 27, 2026

Uh oh!

xtqqczze Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

mattsu2020 Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 28, 2026

Uh oh!

xtqqczze Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xtqqczze Feb 28, 2026 •

edited

Loading

xtqqczze commented Feb 27, 2026 •

edited

Loading

codspeed-hq bot commented Feb 27, 2026 •

edited

Loading