Skip to content

Further work ... #45

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.

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

Draft
wants to merge 70 commits into
base: master
Choose a base branch
from
Draft

Further work ... #45

wants to merge 70 commits into from

Conversation

cehteh
Copy link
Contributor

@cehteh cehteh commented Nov 19, 2021

I created a fork named 'openat_ct' on crates.io now and working towards adding things I need for my own projects. This PR will stay as Draft to track this work, my goal would be that my work might be eventually merged back here.

cehteh and others added 30 commits April 17, 2021 21:06
This anticipates the statx() interface I'll add soon.

Breaking change: Adds a 'SimpleType::Unknown' which can be returned when the filetype can't be
determinded. Code matching on SimpleType may add a clause for this when there is no catch-all.
dir.list() consumes the dir and creates a DirIter. The underlying 'dup()' operation now
becomes explicit, one may need to call try_clone() first.

the list_self() and list_dir() using this now but are merely convinience wrappers and may (or
may not) deprecated in future.

Rationale: this simplifies the code a bit (list::open_dir() removed) and is closer to the low
level semantics.
* Forgot to forget the Dir handle, thus it would been dropped.
* On linux O_PATH was passed when Dir descriptors where opened. This is generally a good thing
  but broke the refactored list(). This also shown that O_PATH has different enough semantics
  to be problematic vs. opening handles without (as on other OS'es). Changed this to
  Dir::open() and Dir::sub_dir() default to opening without O_PATH (but O_DIRECTORY see
  remarks).
* added Dir::open_path() and Dir::sub_dir_path() with O_PATH (urgh, better idea
  for naming? open_protected() or something like that?) which offer the O_PATH feature on linux.
'Lite' file descriptors are possibly a better naming of what O_PATH does. This introduces then and
implements them portable. On systems which do not support O_PATH normal file descriptors are
used.
* With FdType/fd_type() one can determine the kind of an underlying file descriptor.
  Lite descriptors are implemented only in Linux (for now).
  When O_DIRECTORY is supported it uses fcntl() in favo over stat()

* clone_dirfd() tries to do 'the right thing' for duplicating FD's

* libc_ok() is a simple wraper for libc calls that return -1 on error, I will refactor the
  code in the next commits to make use of that more.

Please test this! So far it works for me on Linux.
This simplifies the code a bit, in 'release' the same output is generated.
debug builds may not inline the libc_ok() and be slightly larger.
This up/downgrade cloning converts into normal/lite handles which was missing before.

I hope this fixes tailhook#34 finally.
the d_ino field is mandatory in POSIX and clients (me) really need it when passing data
around. Keeping it in the Entry saves an extra stat()/metadata query.
Instead compiling conditionally on the target_os this defines features for every aspect that
can be different. Tested only on linux so far. Eventually the presence/absence of these
features should be determined by a build.rs script.

Meanwhile there are OS specific lists (only linux so far, please contribute more) which can be
activated when building with '--feature osname'.
This will allow using `DirIter` with rayon's `par_bridge`.

It may not be thread-safe to call readdir concurrently from multiple
threads on a single `DIR*`, but all `Send` requires is that we can call
it from different threads non-concurrently - so this is fine.

`man readdir` says:

> It is expected that a future version of POSIX.1 will require that
> `readdir()` be thread-safe when concurrently employed on different
> directory streams.

so in the future we may also be able to implement `Sync`.  The `std`
implements both `Send` and `Sync` on [`ReadDir`], but they implement
iteration using the deprecated [`readdir_r`].

To be sure I've also looked at the implementation of readdir on [glibc],
[musl] and [freebsd] and they all use per `struct DIR` allocations for
the dirent buffer.

[`ReadDir`]: https://github.com/rust-lang/rust/blob/7e11f3a8f3c1b2683125e7def0acb68a6d684f92/library/std/src/sys/unix/fs.rs#L478-L511
[`readdir_r`]: https://man7.org/linux/man-pages/man3/readdir_r.3.html
[glibc]: https://code.woboq.org/userspace/glibc/sysdeps/posix/readdir.c.html
[musl]: https://git.musl-libc.org/cgit/musl/tree/src/dirent/readdir.c
[freebsd]: https://github.com/freebsd/freebsd-src/blob/373ffc62c158e52cde86a5b934ab4a51307f9f2e/lib/libc/gen/opendir.c#L337
This should resolve tailhook#27. It does not fix it by adding a expensive stat() check on opening, but by
giving the opportunity to explicitly check a Dir handle. When really required a programmer can
enforce consistency between platforms by using 'is_dir()'. Failing to do so won't cause any
harm because improper handles (on platforms without O_DIRECTORY) would report errors at later
time.

This also removes the test_open_file() test from the testsuite, as it depended on presence of
O_DIRECTORY which makes he testsuite non deterministic testing only where its oblivious that
the OS wouldn't open files as directories.
Allows to build/pass custom flags when opening a 'Dir', see tests/flagsbuilder.rs

Should fix/conclude tailhook#29

Notice: this removes the 'const BASE_OPEN_FLAGS' as it makes flags handling a bit more
elaborate:
 1. DirFlags is created with 'O_CLOEXEC' (I considered O_NOFOLLOW but turned that down)
 2. The user may modify the flags to his liking
 3. The final open() calls will set the required flags (O_DIRECTORY, O_PATH)

All functions that used BASE_OPEN_FLAGS fixed up, but semantics stay the same.
having Dir::new() returning DirFlags and not Dir would be a tpp surprising API
With this it becomes possible to construct and pass open flags to the various member
functions. Also list_self() and list_dir() will use O_SEARCH when supported.

Other than the Dir flags builder, this flags builder for member functions defaults to
O_NOFOLLOW (and O_CLOEXEC) to prevent symlink attacks by default. When this is not desired one
can still clear O_NOFOLLOW.
This adds an abstraction layer redefining the types used. Later this will be used to use this
for adapting to the OS/Library configuration giving an consistent type interface to the
outside.
only for the most common/important fields, may change at any time.
cehteh added 11 commits February 2, 2022 18:56
 * DirHandle impls Drop thus the last reference closes the handle
 * add explicit DirIter::close() and Entry::stop() methods to close the handle
   early and abort iteration
non atomic yet, will be changed soon to atomic
This allows early closing from threads.
use +nightly for reformatting
There is no reason to mark this function unsafe since it doesn't violate rusts memory safety.
* Code cleanups
* Update dependencies
* Descriptors are handled atomic now, this allows explicit closing to free system resources
  even when some (Arc) copies still exist
* deprecated functions got removed
* reformatting with rustfmt.toml
* clippy is happy \o/
revert making the Entry members public.
cehteh and others added 8 commits February 4, 2022 08:22
Was a bad idea, simplify the code, explicit closing is better done on application side
duplicate fd's with F_DUPFD_CLOEXEC when available, otherwise fall back to F_DUPFD+FD_CLOEXEC
It calls fsync() on the file descriptor.  This is necessary with some
operating systems and/or file systems to ensure that changes to the
contents of the directory make it to persistent storage — for the same
reason it is sometimes necessary to use `std::fs::File::sync_{all,contents}`.

Note that this method will fail if used on a `Dir` opened with `O_PATH`.
(Same issue as `list_dir`; see tailhook#34.)

Closes tailhook#47.
New method `Dir::sync`
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