Follow-up for 34c3d57474
O_RDONLY is dropped when O_DIRECTORY is specified, since
it's unnecessary and even arguably confusing here, as
the dir is modified.
This let's systemd-repart respect the `SOURCE_DATE_EPOCH` environment
variable when creating directories in the local tree through `CopyFiles`
or `MakeDirectories`.
To do this, we pass a timestamp `ts` to `mkdir_p_root`, which it will
use to fix up `mtime` and `atime` of the directory it creates as
well as the `mtime` of the directory it creates the other directory *in*,
as the `mtime` of the latter is modified when creating a directory in it.
For the same reason, it also needs to fixup the `mtime` of the upper
directory when copying a file into it through `CopyFiles`.
If `SOURCE_DATE_EPOCH`, times are left as is. (`UTIME_OMIT`)
This pulls this generally useful helper out of sysusers and into the
util lib, and updates the places throughout the codebase where it makes
sense to use it.
If we created the dir successfully, we let chmod_and_chown_at() do its thing
and shouldn't go into the part where we check if the existing directory has the
right permissions and ownership and possibly adjust them. The code was doing
that, by relying on the fact that chmod_and_chown_at() does not return -EEXIST.
That's probably true, but seems unnecessarilly complicated.
Follow-up for c1b1492a94.
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.
Before:
====
$ systemctl edit network.target
Failed to create directories for "/etc/systemd/system/network.target.d/override.conf": No such file or directory
====
After:
====
$ systemctl edit network.target
Failed to create directories for "/etc/systemd/system/network.target.d/override.conf": Permission denied
====
Fixes#26652.
-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 effectively disables warnings about type/mode/ownership of existing
directories when recursively creating parent directories. (Or files. If there's
a file in a place we expect a directory, the code will later try to create
a file and fail. This follows the general pattern where we do (void)mkdir()
if the mkdir() is immediately followed by opening of a file.)
I was recently debugging an issue with the fstab-generator [1], and it says:
'Directory "/tmp" already exists, but has mode 0777 that is too permissive (0644 was requested), refusing.'
which is very specific but totally wrong in this context.
This output was added in 37c1d5e97d, and I still
think it is worth to do it, because if you actually *do* want the directory, if
there's something wrong, the precise error message will make it much easier to
diagnose. And we can't easily pass the information what failed up the call chain
because there are multiple things we check (ownership, permission mask, type)…
So passing a param whether to warn or not down into the library code seems like
the best solution, despite not being very elegant.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=2051285
Fixes a regression caused by 3008a6f21c.
Before the commit, when `mkdir_parents_internal()` is called from `mkdir_p()`,
it uses `_mkdir()` as `flag` is zero. But after the commit, `mkdir_safe_internal()`
is always used. Hence, if the path contains a symlink, it fails with -ENOTDIR.
To fix the issue, this makes `mkdir_p()` calls `mkdir_parents_internal()` with
MKDIR_FOLLOW_SYMLINK flag.
Fixes#22334.
Let's reduce our code duplication, and let's focus on using xyzat()
style APIs more, hence drop mkdir_errno_wrapper() and stick to
mkdirar_errno_wrapper() wherever we can, it's a true superset of
functionality after all.
Let's define two helpers strdupa_safe() + strndupa_safe() which do the
same as their non-safe counterparts, except that they abort if called
with allocations larger than ALLOCA_MAX.
This should ensure that all our alloca() based allocations are subject
to this limit.
afaics glibc offers three alloca() based APIs: alloca() itself,
strndupa() + strdupa(). With this we have now replacements for all of
them, that take the limit into account.
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.)
These lines are generally out-of-date, incomplete and unnecessary. With
SPDX and git repository much more accurate and fine grained information
about licensing and authorship is available, hence let's drop the
per-file copyright notice. Of course, removing copyright lines of others
is problematic, hence this commit only removes my own lines and leaves
all others untouched. It might be nicer if sooner or later those could
go away too, making git the only and accurate source of authorship
information.
This part of the copyright blurb stems from the GPL use recommendations:
https://www.gnu.org/licenses/gpl-howto.en.html
The concept appears to originate in times where version control was per
file, instead of per tree, and was a way to glue the files together.
Ultimately, we nowadays don't live in that world anymore, and this
information is entirely useless anyway, as people are very welcome to
copy these files into any projects they like, and they shouldn't have to
change bits that are part of our copyright header for that.
hence, let's just get rid of this old cruft, and shorten our codebase a
bit.
Files which are installed as-is (any .service and other unit files, .conf
files, .policy files, etc), are left as is. My assumption is that SPDX
identifiers are not yet that well known, so it's better to retain the
extended header to avoid any doubt.
I also kept any copyright lines. We can probably remove them, but it'd nice to
obtain explicit acks from all involved authors before doing that.
When we are attempting to create directory somewhere in the bowels of /var/lib
and get an error that it already exists, it can be quite hard to diagnose what
is wrong (especially for a user who is not aware that the directory must have
the specified owner, and permissions not looser than what was requested). Let's
print a warning in most cases. A warning is appropriate, because such state is
usually a sign of borked installation and needs to be resolved by the adminstrator.
$ build/test-fs-util
Path "/tmp/test-readlink_and_make_absolute" already exists and is not a directory, refusing.
(or)
Directory "/tmp/test-readlink_and_make_absolute" already exists, but has mode 0775 that is too permissive (0755 was requested), refusing.
(or)
Directory "/tmp/test-readlink_and_make_absolute" already exists, but is owned by 1001:1000 (1000:1000 was requested), refusing.
Assertion 'mkdir_safe(tempdir, 0755, getuid(), getgid(), MKDIR_WARN_MODE) >= 0' failed at ../src/test/test-fs-util.c:320, function test_readlink_and_make_absolute(). Aborting.
No functional change except for the new log lines.
