Skip to content

Refactor fstat into a wrapper in libutil#15111

Merged
Ericson2314 merged 1 commit intoNixOS:masterfrom
obsidiansystems:fstat-windows
Jan 28, 2026
Merged

Refactor fstat into a wrapper in libutil#15111
Ericson2314 merged 1 commit intoNixOS:masterfrom
obsidiansystems:fstat-windows

Conversation

@artemist
Copy link
Member

Motivation

We use a different fstat on posix and windows systems, and not all fstat users were using the correct one. Factor out fstat to make the change easier.

Context

See also a13de50 for other stat functions


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

We use a different fstat on posix and windows systems,
and not all fstat users were using the correct one.
Factor out fstat to make the change easier.

See also a13de50 for other stat
functions
PosixStat st;
if (fstat(fromDescriptorReadOnly(fd.get()), &st) == -1)
throw SysError("statting file");
auto st = nix::fstat(fromDescriptorReadOnly(fd.get()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, fromDescriptorReadOnly performs a destructive move out of HANDLE on windows and that HANDLE must not be used anymore or closed with CloseHandle. Instead it must use a the ::close from the C runtime, so this is kind of borked. A pre-existing issue, but still something to keep in mind :(

Copy link
Member

@Ericson2314 Ericson2314 Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ericson2314 Ericson2314 enabled auto-merge January 28, 2026 18:07
@Ericson2314 Ericson2314 added this pull request to the merge queue Jan 28, 2026
Merged via the queue into NixOS:master with commit 711e6b3 Jan 28, 2026
14 checks passed
@Ericson2314 Ericson2314 deleted the fstat-windows branch January 28, 2026 19:32
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.

3 participants