Let's be more accurate about what this function does: it checks whether
the underlying reported inode is the same. Internally, this already uses
a better named stat_inode_same() call, hence let's similarly name the
wrapping function following the same logic.
Similar for files_same_at() and path_equal_or_same_files().
No code changes, just some renaming.
Normal users do not have permissions to access /proc/1/root, so
'systemd-detect-virt -r' fails, but the output, even at debug level
is cryptic:
$ SYSTEMD_LOG_LEVEL=debug build/systemd-detect-virt -r
Failed to check for chroot() environment: Permission denied
Let's make this a bit easier to figure out:
$ SYSTEMD_LOG_LEVEL=debug build/systemd-detect-virt -r
Cannot stat /proc/1/root: Permission denied
Failed to check for chroot() environment: Permission denied
I looked over other users of files_same(), and I think in general the message
at debug level is OK for them too.
Chasing symlinks is a core function that's used in a lot of places
so it deservers a less verbose names so let's rename it to chase()
and chaseat().
We also slightly change the pattern used for the chaseat() helpers
so we get chase_and_openat() and similar.
-1 was used everywhere, but -EBADF or -EBADFD started being used in various
places. Let's make things consistent in the new style.
Note that there are two candidates:
EBADF 9 Bad file descriptor
EBADFD 77 File descriptor in bad state
Since we're initializating the fd, we're just assigning a value that means
"no fd yet", so it's just a bad file descriptor, and the first errno fits
better. If instead we had a valid file descriptor that became invalid because
of some operation or state change, the other errno would fit better.
In some places, initialization is dropped if unnecessary.
This is a follow-up for f470cb6d13 which in
turn is a follow-up for a068aceafb.
The latter started to honour hidden files when deciding whether a
directory is empty. The former reverted to the old behaviour to fix
issue #23220.
It introduced a bug though: when a directory contains a larger number of
hidden entries the getdents64() buffer will not suffice to read them,
since we just allocate three entries for it (which is definitely enough
if we just ignore the . + .. entries, but not ig we ignore more).
I think it's a bit confusing that dir_is_empty() can return true even if
rmdir() on the dir would return ENOTEMPTY. Hence, let's rework the
function to make it optional whether hidden files are ignored or not.
After all, I looking at the users of this function I am pretty sure in
more cases we want to honour hidden files.
No actual code changes, just splitting out of some dev_t handling
related calls from stat-util.[ch], they are quite a number already, and
deserve their own module now I think.
Also, try to settle on the name "devnum" as the name for the concept,
instead of "devno" or "dev" or "devid". "devnum" is the name exported in
udev APIs, hence probably best to stick to that. (this just renames a
few symbols to "devum", local variables are left untouched, to make the
patch not too invasive)
No actual code changes.
Apparently memory sanitizer doesn't grok getdents64() properly. Let's
address that by explicitly marken memory initialized by getdents64() as
unpoisoned.
That way we have a single syscall only for it, instead of the multiple
readdir() and friends do. And we can operate entirely on the stack, no
malloc() implicit.
That way we can fail earlier if the specified fd is not actually a
directory.
(Also, it's not exactly according to standards to open things without
either O_RDONLY/O_RDWR...)
Use the right FD, and do a fd_reopen instead of a dup, since the
latter will still share the internal pointer which then gets
moved by FOREACH_DIRENT, affecting the caller's FD.
Note that st_mtime member of struct stat is defined as follows,
#define st_mtime st_mtim.tv_sec
Hence we omitted checking nanosecond part of the timestamp (struct
timespec) and possibly would miss modifications that happened within the
same second.
This simplifies things quite a bit, and is reusable wherever we want to
use statx() later on. Not sure why I didn't do it like this right from
the beginning...
Some fdopendir() calls remain where safe_close() is manually
performed, those could be simplified as well by converting to
use the _cleanup_close_ machinery, but makes things less trivial
to review so left for a future cleanup.
chase_symlinks() would return negative on error, and either a non-negative status
or a non-negative fd when CHASE_OPEN was given. This made the interface quite
complicated, because dependning on the flags used, we would get two different
"types" of return object. Coverity was always confused by this, and flagged
every use of chase_symlinks() without CHASE_OPEN as a resource leak (because it
would this that an fd is returned). This patch uses a saparate output parameter,
so there is no confusion.
(I think it is OK to have functions which return either an error or an fd. It's
only returning *either* an fd or a non-fd that is confusing.)
Daan De Meyer
Lennart Poettering
Zbigniew Jędrzejewski-Szmek
Yu Watanabe
Luca Boccassi
Iago López Galeiras
Luca Boccassi
Michal Sekletár
Vito Caputo
Ryan Gonzalez