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`.
MR https://gitlab.redox-os.org/redox-os/relibc/-/merge_requests/1250 was
done in order to try and patch out an `EINVAL` error, and it was
successful in doing so. Unfortunately, fixing one failure caused
another.
I will continue to do what I can to make relibc's FS layer and redoxFS
smaller, as they have way too much duplicated code.
Additionally, this adds the `SYMLOOP_MAX` constant in `limits.h` to
replace the `MAX_LEVEL` magic number used during symlink resolution.
`fstat` was kept because it is more difficult than the others to do a
drop-in replacement in terms of `fstatat` for since a naive approach
would cause mutual recursion.
Inside of `openat2`, we are assuming that if we are passed
`AT_SYMLINK_NOFOLLOW` that we are also passed a symlink. This is simply
not the case, as POSIX defines the flag to only have a noticable effect
if the resolved path is a symlink. Therefore, we cannot assume that we
have a symlink if we see `AT_SYMLINK_NOFOLLOW`. The previous behavior
caused an `EINVAL` in redoxfs because we do a consistency check to
error out if we are passed `O_SYMLINK` (which was added because
`AT_SYMLINK_NOFOLLOW` was observed) and aren't a symlink.
Hmm, maybe a special errno like `ENOTLNK` should be deployed for this? It's
specific enough that it could possibly be added to a future POSIX.
Most of these changes are very simple. Among the changes made involve
taking advantage of auto-deref (`(*val).foo()` -> `val.foo()`) and
removing instances where we create a ref and immediately dereference it
(`&*val` -> `val`). There was a pretty neat case in `posix_openpt` where
some pointer verbosity was able to be reduced by using the more modern C
strings rather than the byte strings with an explicit NUL at the end.
Additionally, `exit()` now calls `unreachable!()` at the end. We
previously did `loop {}`, but clippy didn't like this. It can be up for
debate whether we want to make this `unreachable_unchecked` or similar.
There is only one change that might cause any sort of concern, and that
is the change from `.skip_while(!p).next()` -> `.find(p)`. This, like
everything else, was caught in a Clippy lint but I believe it deserves
some explanation because it isn't immediately obvious. Info about the
lint is here: https://rust-lang.github.io/rust-clippy/rust-1.89.0/index.html#skip_while_next
I noticed that the Linux and Redox PALs both do very similar things for
a given FS function and its *at variant. For example, `mkdir()` is just
a call to `mkdirat()`. POSIX requires these to be equivalent.
Additionally, we use AT_EMPTY_PATH in some places but note that this is
not POSIX and is instead an extension that Linux (and Redox) implement.
That doesn't really matter though, since this is an implementation
detail. Implementations can choose to implement these functions anyway
and ignore the default impl. Such a case is `fstat`, because the current
Redox impl of `fstatat` relies on `fstat`, and this would cause infinite
recursion.
Future work:
POSIX says that `fcntl(fd, F_DUPFD, 0);` shall be equivalent to
`dup(fd);`. `dup2` might have cases where it can be implemented using
`dup3`. `dup` seemingly cannot be implemented in terms of `dup2`, so the
`fcntl` default implementation is sufficient. `pipe(fds)` is equivalent to
`pipe2(fds, 0);`.
Additionally, we were using the `access` syscall for x86_64 for our
linux PAL. I looked at the linux source and found that using the
`access`
syscall is equivalent to using `faccessat` as we do now: https://elixir.bootlin.com/linux/v6.17.5/source/fs/open.c#L550
Also, some of the `fcntl.h` functions were accidentally implemented and exported from `unistd.h`. They are supposed to be implemented in `fcntl.h` and exported to `unistd.h`. This is now the behavior.
`linkat(2)` doesn't have `AT_EMPTY_PATH` as a valid flag in this
implementation because it isn't POSIX. We have it (and have
support for it), but it is more effort to add it. If we need it at some
point, it can be added in about 3 lines.
`openat(2)` previously wasn't exposed, and William was not aware of
`Sys::openat`'s existence. We use it under the hood for `mkfifoat(2)`
and friends, so expose it as a libc API. This helps to pass more os-test tests.
Lastly, the 3 implemented syscalls here help pass some os-test tests.
Wildan M