Skip to content

Show mode_t as octal in std::fs Debug impls #122812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Show mode_t as octal in std::fs Debug impls #122812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 115 additions & 5 deletions library/std/src/sys/pal/unix/fs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
// miri has some special hacks here that make things unused.
#![cfg_attr(miri, allow(unused))]

#[cfg(test)]
mod tests;

use crate::os::unix::prelude::*;

use crate::ffi::{CStr, OsStr, OsString};
use crate::fmt;
use crate::fmt::{self, Write as _};
use crate::io::{self, BorrowedCursor, Error, IoSlice, IoSliceMut, SeekFrom};
use crate::mem;
use crate::os::unix::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd};
Expand Down Expand Up @@ -354,7 +357,7 @@ pub struct DirEntry {
entry: dirent64,
}

#[derive(Clone, Debug)]
#[derive(Clone)]
pub struct OpenOptions {
// generic
read: bool,
Expand All @@ -368,7 +371,7 @@ pub struct OpenOptions {
mode: mode_t,
}

#[derive(Clone, PartialEq, Eq, Debug)]
#[derive(Clone, PartialEq, Eq)]
pub struct FilePermissions {
mode: mode_t,
}
Expand All @@ -381,7 +384,7 @@ pub struct FileTimes {
created: Option<SystemTime>,
}

#[derive(Copy, Clone, Eq, Debug)]
#[derive(Copy, Clone, Eq)]
pub struct FileType {
mode: mode_t,
}
Expand All @@ -398,11 +401,13 @@ impl core::hash::Hash for FileType {
}
}

#[derive(Debug)]
pub struct DirBuilder {
mode: mode_t,
}

#[derive(Copy, Clone)]
struct Mode(mode_t);
Copy link
Member

Choose a reason for hiding this comment

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

is there a reason for not using Mode as a field and continuing to derive the impls?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question — I started out this change that way in fact. Here is what it looks like: #123127. I switched to the way in this PR because I found the other one too invasive to the rest of the code in this module which involves arithmetic on the mode, but either way is fine with me. Let me know if you have a preference based on reading both. There is also a third possibility which omits those std::ops impls on Mode.


cfg_has_statx! {{
impl FileAttr {
fn from_stat64(stat: stat64) -> Self {
Expand Down Expand Up @@ -673,12 +678,26 @@ impl FileType {
}
}

impl fmt::Debug for FileType {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let FileType { mode } = self;
f.debug_struct("FileType").field("mode", &Mode(*mode)).finish()
}
}

impl FromInner<u32> for FilePermissions {
fn from_inner(mode: u32) -> FilePermissions {
FilePermissions { mode: mode as mode_t }
}
}

impl fmt::Debug for FilePermissions {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let FilePermissions { mode } = self;
f.debug_struct("FilePermissions").field("mode", &Mode(*mode)).finish()
}
}

impl fmt::Debug for ReadDir {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// This will only be called from std::fs::ReadDir, which will add a "ReadDir()" frame.
Expand Down Expand Up @@ -1116,6 +1135,23 @@ impl OpenOptions {
}
}

impl fmt::Debug for OpenOptions {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let OpenOptions { read, write, append, truncate, create, create_new, custom_flags, mode } =
self;
f.debug_struct("OpenOptions")
.field("read", read)
.field("write", write)
.field("append", append)
.field("truncate", truncate)
.field("create", create)
.field("create_new", create_new)
.field("custom_flags", custom_flags)
.field("mode", &Mode(*mode))
.finish()
}
}

impl File {
pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<File> {
run_path_with_cstr(path, &|path| File::open_c(path, opts))
Expand Down Expand Up @@ -1402,6 +1438,13 @@ impl DirBuilder {
}
}

impl fmt::Debug for DirBuilder {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let DirBuilder { mode } = self;
f.debug_struct("DirBuilder").field("mode", &Mode(*mode)).finish()
}
}

impl AsInner<FileDesc> for File {
#[inline]
fn as_inner(&self) -> &FileDesc {
Expand Down Expand Up @@ -1574,6 +1617,73 @@ impl fmt::Debug for File {
}
}

// Format in octal, followed by the mode format used in `ls -l`.
//
// References:
// https://pubs.opengroup.org/onlinepubs/009696899/utilities/ls.html
// https://www.gnu.org/software/libc/manual/html_node/Testing-File-Type.html
// https://www.gnu.org/software/libc/manual/html_node/Permission-Bits.html
//
// Example:
// 0o100664 (-rw-rw-r--)
impl fmt::Debug for Mode {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self(mode) = *self;
write!(f, "0o{mode:06o}")?;

let entry_type = match mode & libc::S_IFMT {
libc::S_IFDIR => 'd',
libc::S_IFBLK => 'b',
libc::S_IFCHR => 'c',
libc::S_IFLNK => 'l',
libc::S_IFIFO => 'p',
libc::S_IFREG => '-',
_ => return Ok(()),
};

f.write_str(" (")?;
f.write_char(entry_type)?;

// Owner permissions
f.write_char(if mode & libc::S_IRUSR != 0 { 'r' } else { '-' })?;
f.write_char(if mode & libc::S_IWUSR != 0 { 'w' } else { '-' })?;
let owner_executable = mode & libc::S_IXUSR != 0;
let setuid = mode as c_int & libc::S_ISUID as c_int != 0;
f.write_char(match (owner_executable, setuid) {
(true, true) => 's', // executable and setuid
(false, true) => 'S', // setuid
(true, false) => 'x', // executable
(false, false) => '-',
})?;

// Group permissions
f.write_char(if mode & libc::S_IRGRP != 0 { 'r' } else { '-' })?;
f.write_char(if mode & libc::S_IWGRP != 0 { 'w' } else { '-' })?;
let group_executable = mode & libc::S_IXGRP != 0;
let setgid = mode as c_int & libc::S_ISGID as c_int != 0;
f.write_char(match (group_executable, setgid) {
(true, true) => 's', // executable and setgid
(false, true) => 'S', // setgid
(true, false) => 'x', // executable
(false, false) => '-',
})?;

// Other permissions
f.write_char(if mode & libc::S_IROTH != 0 { 'r' } else { '-' })?;
f.write_char(if mode & libc::S_IWOTH != 0 { 'w' } else { '-' })?;
let other_executable = mode & libc::S_IXOTH != 0;
let sticky = mode as c_int & libc::S_ISVTX as c_int != 0;
f.write_char(match (entry_type, other_executable, sticky) {
('d', true, true) => 't', // searchable and restricted deletion
('d', false, true) => 'T', // restricted deletion
(_, true, _) => 'x', // executable
(_, false, _) => '-',
})?;

f.write_char(')')
}
}

pub fn readdir(path: &Path) -> io::Result<ReadDir> {
let ptr = run_path_with_cstr(path, &|p| unsafe { Ok(libc::opendir(p.as_ptr())) })?;
if ptr.is_null() {
Expand Down
71 changes: 71 additions & 0 deletions library/std/src/sys/pal/unix/fs/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
use crate::sys::pal::unix::fs::FilePermissions;

#[test]
fn test_debug_permissions() {
for (expected, mode) in [
// typical directory
("FilePermissions { mode: 0o040775 (drwxrwxr-x) }", 0o04_0775),
// typical text file
("FilePermissions { mode: 0o100664 (-rw-rw-r--) }", 0o10_0664),
// setuid executable (/usr/bin/doas)
("FilePermissions { mode: 0o104755 (-rwsr-xr-x) }", 0o10_4755),
// char device (/dev/zero)
("FilePermissions { mode: 0o020666 (crw-rw-rw-) }", 0o02_0666),
// block device (/dev/vda)
("FilePermissions { mode: 0o060660 (brw-rw----) }", 0o06_0660),
// symbolic link
("FilePermissions { mode: 0o120777 (lrwxrwxrwx) }", 0o12_0777),
// fifo
("FilePermissions { mode: 0o010664 (prw-rw-r--) }", 0o01_0664),
// none
("FilePermissions { mode: 0o100000 (----------) }", 0o10_0000),
// unrecognized
("FilePermissions { mode: 0o000001 }", 1),
] {
assert_eq!(format!("{:?}", FilePermissions { mode }), expected);
}

for (expected, mode) in [
// owner readable
("FilePermissions { mode: 0o100400 (-r--------) }", libc::S_IRUSR),
// owner writable
("FilePermissions { mode: 0o100200 (--w-------) }", libc::S_IWUSR),
// owner executable
("FilePermissions { mode: 0o100100 (---x------) }", libc::S_IXUSR),
// setuid
("FilePermissions { mode: 0o104000 (---S------) }", libc::S_ISUID),
// owner executable and setuid
("FilePermissions { mode: 0o104100 (---s------) }", libc::S_IXUSR | libc::S_ISUID),
// group readable
("FilePermissions { mode: 0o100040 (----r-----) }", libc::S_IRGRP),
// group writable
("FilePermissions { mode: 0o100020 (-----w----) }", libc::S_IWGRP),
// group executable
("FilePermissions { mode: 0o100010 (------x---) }", libc::S_IXGRP),
// setgid
("FilePermissions { mode: 0o102000 (------S---) }", libc::S_ISGID),
// group executable and setgid
("FilePermissions { mode: 0o102010 (------s---) }", libc::S_IXGRP | libc::S_ISGID),
// other readable
("FilePermissions { mode: 0o100004 (-------r--) }", libc::S_IROTH),
// other writeable
("FilePermissions { mode: 0o100002 (--------w-) }", libc::S_IWOTH),
// other executable
("FilePermissions { mode: 0o100001 (---------x) }", libc::S_IXOTH),
// sticky
("FilePermissions { mode: 0o101000 (----------) }", libc::S_ISVTX),
// other executable and sticky
("FilePermissions { mode: 0o101001 (---------x) }", libc::S_IXOTH | libc::S_ISVTX),
] {
assert_eq!(format!("{:?}", FilePermissions { mode: libc::S_IFREG | mode }), expected);
}

for (expected, mode) in [
// restricted deletion ("sticky") flag is set, and search permission is not granted to others
("FilePermissions { mode: 0o041000 (d--------T) }", libc::S_ISVTX),
// sticky and searchable
("FilePermissions { mode: 0o041001 (d--------t) }", libc::S_ISVTX | libc::S_IXOTH),
] {
assert_eq!(format!("{:?}", FilePermissions { mode: libc::S_IFDIR | mode }), expected);
}
}
Loading