vasilito/RedBear-OS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove redundant code and impl them in terms of *at functions

Browse Source 
The following functions were replaced with their *at variants or
similar:
- lstat -> lstat -> fstatat
- stat -> stat -> fstatat
- fstat -> fstatat
- fchmod -> fchmodat
- fchown -> fchownat
- lchown -> fchownat
- mkfifo -> mkfifoat
- open -> openat
- renameat -> renameat2
- rmdir -> unlinkat
- unlink -> unlinkat
- symlink-> symlinkat

The `open_flags` logic for fstatat was redundant, as this is already
handled (and better so) in `openat2`.

Additionally, the fstatat test succeeds in os-test, and no longer
returns EINVAL. This is because `O_SYMLINK` is no longer
unconditionally passed like it was before. This is a problem because
redoxfs returns EINVAL if a node isn't a symlink but set `O_SYMLINK`.
This commit is contained in:
Connor-GH
2026-05-05 13:59:30 -05:00
parent 5312b86ee5
commit e2486379b1
4 changed files with 32 additions and 175 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/header/sys_stat/mod.rs
+3 -30
View File
@@ -5,10 +5,7 @@
use crate::{
c_str::CStr,
error::ResultExt,
header::{
fcntl::{AT_SYMLINK_NOFOLLOW, O_NOFOLLOW, O_PATH},
time::timespec,
},
header::{fcntl::AT_SYMLINK_NOFOLLOW, time::timespec},
out::Out,
platform::{
Pal, Sys,
@@ -147,19 +144,7 @@ pub unsafe extern "C" fn futimens(fd: c_int, times: *const timespec) -> c_int {
pub unsafe extern "C" fn lstat(path: *const c_char, buf: *mut stat) -> c_int {
let path = unsafe { CStr::from_ptr(path) };
let buf = unsafe { Out::nonnull(buf) };
// TODO: Rustify
let fd = Sys::open(path, O_PATH | O_NOFOLLOW, 0).or_minus_one_errno();
if fd < 0 {
return -1;
}
// TODO: Rustify
let res = Sys::fstat(fd, buf).map(|()| 0).or_minus_one_errno();
if let Ok(()) = Sys::close(fd) {}; // TODO handle error
res
Sys::lstat(path, buf).map(|()| 0).or_minus_one_errno()
}
/// See <https://pubs.opengroup.org/onlinepubs/9799919799/functions/mkdirat.html>.
@@ -220,19 +205,7 @@ pub unsafe extern "C" fn mknodat(
pub unsafe extern "C" fn stat(file: *const c_char, buf: *mut stat) -> c_int {
let file = unsafe { CStr::from_ptr(file) };
let buf = unsafe { Out::nonnull(buf) };
// TODO: Rustify
let fd = Sys::open(file, O_PATH, 0).or_minus_one_errno();
if fd < 0 {
return -1;
}
// TODO: Rustify
let res = Sys::fstat(fd, buf).map(|()| 0).or_minus_one_errno();
if let Ok(()) = Sys::close(fd) {}; // TODO handle error
res
Sys::stat(file, buf).map(|()| 0).or_minus_one_errno()
}
/// See <https://pubs.opengroup.org/onlinepubs/9799919799/functions/umask.html>.
src/platform/linux/mod.rs
+3 -66
View File
@@ -8,7 +8,7 @@ use crate::{
header::{
dirent::dirent,
errno::{EINVAL, EIO},
fcntl::{AT_EMPTY_PATH, AT_FDCWD, AT_REMOVEDIR},
fcntl::AT_EMPTY_PATH,
signal::{SIGCHLD, sigevent},
sys_resource::{rlimit, rusage},
sys_select::timeval,
@@ -174,6 +174,8 @@ impl Pal for Sys {
e_raw(unsafe { syscall!(FCHDIR, fildes) }).map(|_| ())
}
// Override the default impl in Sys because `fchmodat(fildes, "", mode, AT_EMPTY_PATH)` doesn't
// seem to be equivalent to `fchmod(fildes, mode)` on Linux.
fn fchmod(fildes: c_int, mode: mode_t) -> Result<()> {
e_raw(unsafe { syscall!(FCHMOD, fildes, mode) }).map(|_| ())
}
@@ -191,10 +193,6 @@ impl Pal for Sys {
.map(|_| ())
}
fn fchown(fildes: c_int, owner: uid_t, group: gid_t) -> Result<()> {
e_raw(unsafe { syscall!(FCHOWN, fildes, owner, group) }).map(|_| ())
}
fn fdatasync(fildes: c_int) -> Result<()> {
e_raw(unsafe { syscall!(FDATASYNC, fildes) }).map(|_| ())
}
@@ -203,21 +201,6 @@ impl Pal for Sys {
e_raw(unsafe { syscall!(FLOCK, fd, operation) }).map(|_| ())
}
fn fstat(fildes: c_int, mut buf: Out<stat>) -> Result<()> {
let empty = b"\0";
let empty_ptr = empty.as_ptr().cast::<c_char>();
e_raw(unsafe {
syscall!(
NEWFSTATAT,
fildes,
empty_ptr,
buf.as_mut_ptr(),
AT_EMPTY_PATH
)
})
.map(|_| ())
}
fn fstatat(fildes: c_int, path: Option<CStr>, mut buf: Out<stat>, flags: c_int) -> Result<()> {
e_raw(unsafe {
syscall!(
@@ -455,26 +438,6 @@ impl Pal for Sys {
unsafe { syscall!(GETUID) as uid_t }
}
#[cfg(any(target_arch = "x86_64", target_arch = "x86"))]
fn lchown(path: CStr, owner: uid_t, group: gid_t) -> Result<()> {
e_raw(unsafe { syscall!(LCHOWN, path.as_ptr(), owner, group) }).map(|_| ())
}
#[cfg(target_arch = "aarch64")]
fn lchown(path: CStr, owner: uid_t, group: gid_t) -> Result<()> {
e_raw(unsafe {
syscall!(
FCHOWNAT,
AT_FDCWD,
path.as_ptr(),
owner as u32,
group as u32,
crate::header::fcntl::AT_SYMLINK_NOFOLLOW
)
})
.map(|_| ())
}
fn linkat(fd1: c_int, path1: CStr, fd2: c_int, path2: CStr, flags: c_int) -> Result<()> {
e_raw(unsafe { syscall!(LINKAT, fd1, path1.as_ptr(), fd2, path2.as_ptr(), flags) })
.map(|_| ())
@@ -503,10 +466,6 @@ impl Pal for Sys {
Sys::mknodat(dir_fd, path, mode | S_IFIFO, 0)
}
fn mkfifo(path: CStr, mode: mode_t) -> Result<()> {
Sys::mknod(path, mode | S_IFIFO, 0)
}
unsafe fn mlock(addr: *const c_void, len: usize) -> Result<()> {
e_raw(unsafe { syscall!(MLOCK, addr, len) }).map(|_| ())
}
@@ -564,11 +523,6 @@ impl Pal for Sys {
e_raw(unsafe { syscall!(NANOSLEEP, rqtp, rmtp) }).map(|_| ())
}
fn open(path: CStr, oflag: c_int, mode: mode_t) -> Result<c_int> {
e_raw(unsafe { syscall!(OPENAT, AT_FDCWD, path.as_ptr(), oflag, mode) })
.map(|fd| fd as c_int)
}
fn openat(dirfd: c_int, path: CStr, oflag: c_int, mode: mode_t) -> Result<c_int> {
e_raw(unsafe { syscall!(OPENAT, dirfd, path.as_ptr(), oflag, mode) }).map(|fd| fd as c_int)
}
@@ -706,19 +660,6 @@ impl Pal for Sys {
})
}
fn renameat(old_dir: c_int, old_path: CStr, new_dir: c_int, new_path: CStr) -> Result<()> {
e_raw(unsafe {
syscall!(
RENAMEAT,
old_dir,
old_path.as_ptr(),
new_dir,
new_path.as_ptr()
)
})
.map(|_| ())
}
fn renameat2(
old_dir: c_int,
old_path: CStr,
@@ -739,10 +680,6 @@ impl Pal for Sys {
.map(|_| ())
}
fn rmdir(path: CStr) -> Result<()> {
e_raw(unsafe { syscall!(UNLINKAT, AT_FDCWD, path.as_ptr(), AT_REMOVEDIR) }).map(|_| ())
}
fn sched_yield() -> Result<()> {
e_raw(unsafe { syscall!(SCHED_YIELD) }).map(|_| ())
}
src/platform/pal/mod.rs
+18 -4
View File
@@ -5,7 +5,7 @@ use crate::{
c_str::CStr,
error::{Errno, Result},
header::{
fcntl::{AT_EMPTY_PATH, AT_FDCWD, F_DUPFD},
fcntl::{AT_EMPTY_PATH, AT_FDCWD, AT_REMOVEDIR, AT_SYMLINK_NOFOLLOW, F_DUPFD},
signal::sigevent,
sys_resource::{rlimit, rusage},
sys_select::timeval,
@@ -121,7 +121,17 @@ pub trait Pal {
/// Platform implementation of [`fstat()`](crate::header::sys_stat::fstat) from [`sys/stat.h`](crate::header::sys_stat).
fn fstat(fildes: c_int, buf: Out<stat>) -> Result<()> {
Self::fstatat(fildes, Some(c"".into()), buf, 0)
Self::fstatat(fildes, Some(c"".into()), buf, AT_EMPTY_PATH)
}
/// Platform implementation of [`lstat()`](crate::header::sys_stat::lstat) from [`sys/stat.h`](crate::header::sys_stat).
fn lstat(path: CStr, buf: Out<stat>) -> Result<()> {
Self::fstatat(AT_FDCWD, Some(path), buf, AT_SYMLINK_NOFOLLOW)
}
/// Platform implementation of [`stat()`](crate::header::sys_stat::stat) from [`sys/stat.h`](crate::header::sys_stat).
fn stat(path: CStr, buf: Out<stat>) -> Result<()> {
Self::fstatat(AT_FDCWD, Some(path), buf, 0)
}
/// Platform implementation of [`fstatat()`](crate::header::sys_stat::fstatat) from [`sys/stat.h`](crate::header::sys_stat).
@@ -253,7 +263,9 @@ pub trait Pal {
fn getuid() -> uid_t;
/// Platform implementation of [`lchown()`](crate::header::unistd::lchown) from [`unistd.h`](crate::header::unistd).
fn lchown(path: CStr, owner: uid_t, group: gid_t) -> Result<()>;
fn lchown(path: CStr, owner: uid_t, group: gid_t) -> Result<()> {
Self::fchownat(AT_FDCWD, path, owner, group, AT_SYMLINK_NOFOLLOW)
}
/// Platform implementation of [`link()`](crate::header::unistd::link) from [`unistd.h`](crate::header::unistd).
fn link(path1: CStr, path2: CStr) -> Result<()> {
@@ -396,7 +408,9 @@ pub trait Pal {
) -> Result<()>;
/// Platform implementation of [`rmdir()`](crate::header::unistd::rmdir) from [`unistd.h`](crate::header::unistd).
fn rmdir(path: CStr) -> Result<()>;
fn rmdir(path: CStr) -> Result<()> {
Self::unlinkat(AT_FDCWD, path, AT_REMOVEDIR)
}
/// Platform implementation of [`sched_yield()`](crate::header::sched::sched_yield) from [`sched.h`](crate::header::sched).
fn sched_yield() -> Result<()>;
src/platform/redox/mod.rs
+8 -75
View File
@@ -32,9 +32,8 @@ use crate::{
ENOSYS, EOPNOTSUPP, EPERM,
},
fcntl::{
self, AT_EACCESS, AT_EMPTY_PATH, AT_FDCWD, AT_REMOVEDIR, AT_SYMLINK_FOLLOW,
AT_SYMLINK_NOFOLLOW, F_GETLK, F_OFD_GETLK, F_OFD_SETLK, F_RDLCK, F_SETLK, F_SETLKW,
F_UNLCK, F_WRLCK, flock,
self, AT_EACCESS, AT_EMPTY_PATH, AT_FDCWD, AT_REMOVEDIR, AT_SYMLINK_FOLLOW, F_GETLK,
F_OFD_GETLK, F_OFD_SETLK, F_RDLCK, F_SETLK, F_SETLKW, F_UNLCK, F_WRLCK, flock,
},
limits,
pthread::{pthread_cancel, pthread_create},
@@ -463,14 +462,7 @@ impl Pal for Sys {
Ok(redox_rt::proc::fork_impl(&redox_rt::proc::ForkArgs::Managed)? as pid_t)
}
fn fstat(fildes: c_int, mut buf: Out<stat>) -> Result<()> {
unsafe {
libredox::fstat(fildes as usize, buf.as_mut_ptr())?;
}
Ok(())
}
fn fstatat(dirfd: c_int, path: Option<CStr>, buf: Out<stat>, flags: c_int) -> Result<()> {
fn fstatat(dirfd: c_int, path: Option<CStr>, mut buf: Out<stat>, flags: c_int) -> Result<()> {
// `path` should be non-null.
let path = path.ok_or(Errno(EFAULT))?;
let mut path = str::from_utf8(path.to_bytes()).ok().ok_or(Errno(EILSEQ))?;
@@ -480,7 +472,7 @@ impl Pal for Sys {
if dirfd == AT_FDCWD {
path = ".";
} else {
return Ok(Sys::fstat(dirfd, buf)?);
return Ok(unsafe { libredox::fstat(dirfd as usize, buf.as_mut_ptr()) }?);
}
} else {
// If the path is empty but `AT_EMPTY_PATH` is **not** set, bail out.
@@ -488,31 +480,15 @@ impl Pal for Sys {
}
}
// Use `O_PATH` to obtain a file descriptor without actually *opening* the file. This
// bypasses permission checks and avoids cases where opening a file is blocking operation
// (e.g., FIFOs). This gives a file descriptor where fstat(2) can be performed (and some
// other meta operations) but nothing else (e.g. read/write).
//
// `O_CLOEXEC` is used to avoid leaking file descriptors to child processes on exec(2).
//
// FIXME: Ideally we would want the file descriptor to not leak on fork(2) too because
// fstatat(2) should not have side effects. However, Redox does not currently support that,
// so we use `CLOEXEC` as a compromise.
// FIXME: Should we handle AT_* flags here or in openat2?
let mut open_flags = fcntl::O_PATH | fcntl::O_CLOEXEC;
if flags & AT_SYMLINK_NOFOLLOW == AT_SYMLINK_NOFOLLOW {
open_flags |= fcntl::O_SYMLINK | fcntl::O_NOFOLLOW;
}
let file = openat2(dirfd, path, 0, open_flags)?;
let file = openat2(dirfd, path, flags, 0)?;
// Close the file descriptor after fstat(2) regardless of success or failure.
let fstat_res = Sys::fstat(*file, buf);
let fstat_res = unsafe { libredox::fstat(*file as usize, buf.as_mut_ptr()) };
let close_res = syscall::close(*file as usize);
if let Err(err) = fstat_res {
return Err(err);
return Err(err.into());
}
close_res?;
fstat_res
Ok(fstat_res?)
}
fn fstatvfs(fildes: c_int, mut buf: Out<statvfs>) -> Result<()> {
@@ -791,15 +767,6 @@ impl Pal for Sys {
redox_rt::sys::posix_getresugid().ruid as uid_t
}
fn lchown(path: CStr, owner: uid_t, group: gid_t) -> Result<()> {
// TODO: Is it correct for regular chown to use O_PATH? On Linux the meaning of that flag
// is to forbid file operations, including fchown.
// unlike chown, never follow symbolic links
let file = File::open(path, fcntl::O_CLOEXEC | fcntl::O_NOFOLLOW)?;
Self::fchown(*file, owner, group)
}
fn linkat(fd1: c_int, oldpath: CStr, fd2: c_int, newpath: CStr, flags: c_int) -> Result<()> {
// make sure the flags passed are valid.
// valid states: AT_SYMLINK_FOLLOW, or 0.
@@ -1141,22 +1108,6 @@ impl Pal for Sys {
Sys::read(*file, out)
}
fn rename(oldpath: CStr, newpath: CStr) -> Result<()> {
let newpath = newpath.to_str().map_err(|_| Errno(EINVAL))?;
let newpath = canonicalize(newpath).map_err(|_| Errno(EINVAL))?;
let file = File::open(
oldpath,
fcntl::O_NOFOLLOW | fcntl::O_PATH | fcntl::O_CLOEXEC,
)?;
syscall::frename(*file as usize, newpath)?;
Ok(())
}
fn renameat(old_dir: c_int, old_path: CStr, new_dir: c_int, new_path: CStr) -> Result<()> {
Sys::renameat2(old_dir, old_path, new_dir, new_path, 0)
}
fn renameat2(
old_dir: c_int,
old_path: CStr,
@@ -1190,13 +1141,6 @@ impl Pal for Sys {
.map_err(Into::into)
}
fn rmdir(path: CStr) -> Result<()> {
let path = path.to_str().map_err(|_| Errno(EINVAL))?;
let canon = canonicalize(path)?;
redox_rt::sys::unlink(&canon, fcntl::AT_REMOVEDIR as usize)?;
Ok(())
}
fn sched_yield() -> Result<()> {
syscall::sched_yield()?;
Ok(())
@@ -1251,10 +1195,6 @@ impl Pal for Sys {
Ok(())
}
fn symlink(path1: CStr, path2: CStr) -> Result<()> {
Sys::symlinkat(path1, AT_FDCWD, path2)
}
fn symlinkat(path1: CStr, fd: c_int, path2: CStr) -> Result<()> {
let mut file = File::createat(
fd,
@@ -1524,13 +1464,6 @@ impl Pal for Sys {
Ok(())
}
fn unlink(path: CStr) -> Result<()> {
let path = path.to_str().map_err(|_| Errno(EINVAL))?;
let canon = canonicalize(path)?;
redox_rt::sys::unlink(&canon, 0)?;
Ok(())
}
fn unlinkat(fd: c_int, path: CStr, flags: c_int) -> Result<()> {
if (flags & !AT_REMOVEDIR) != 0 {
return Err(Errno(EINVAL));