Add two inline wrappers around fstat() and fstatat() which handle
retrying on EINTR and return other errors using GError, to be consistent
with other glnx functions.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
At the moment, it’s not possible for them to do this race-free (since
openat(O_DIRECTORY | O_CREAT | O_EXCL) doesn’t work), but in future this
could be possible. In any case, it’s a useful thing to want to do.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
This is a variant of glnx_shutil_mkdir_p_at() which opens the given
directory and returns a dirfd to it. Currently, the implementation
cannot be race-free (due to a kernel bug), but it could eventually be
made race-free.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
In general, all FDs < 0 are invalid (and should not have close() called
on them), so check that. This could have caused problems if a function
returned an error value < -1.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
We were missing the previous automatic `: ` addition; noticed in
a failing ostree test.
Fix this by just calling the new API as the non-prefix case does too.
These are equivalent to the non-null throw, except that the returned
value is a NULL pointer. They can be used in functions where one wants
to return a pointer. E.g.:
GKeyFile *foo(GError **error) {
return glnx_null_throw (error, "foobar");
}
The function call redirections are wrapped around a compound statement
expression[1] so that they represent a single top-level expression. This
allows us to avoid -Wunused-value warnings vs using a comma operator if
the return value isn't used.
I made the 'args...' absorb the fmt argument as well so that callers can
still use it without always having to specify at least one additional
variadic argument. I had to check to be sure that the expansion is all
done by the preprocessor, so we don't need to worry about stack
intricacies.
[1] https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html
Following up to the previous commit, also shorten our use of
`g_set_error (..., G_IO_ERROR_FAILED, ...)`. There's a lot of
this in libostree at least.
See also https://bugzilla.gnome.org/show_bug.cgi?id=774061
We have a *lot* of code of the form:
```
if (unlinkat (fd, pathname) < 0)
{
glnx_set_error_from_errno (error);
goto out;
}
```
After conversion to `return FALSE style` which is in progress, it's way shorter,
and clearer like this:
```
if (unlinkat (fd, pathname) < 0)
return glnx_throw_errno (error);
```
I want the `RENAME_EXCHANGE` version for rpm-ostree, to atomically
swap `/usr/share/rpm` (a directory) with a new verison. While
we're here we might as well expose `RENAME_NOREPLACE` in case
something else wants it.
These both have fallbacks to the non-atomic version.
Closes: https://github.com/GNOME/libglnx/pull/36
This showed up in the ostree runs with `-fsanitize=undefined` - if we happened
to get `0` then `g_malloc` would return `NULL`. However, what's interesting is
it seemed to happen *consistently*. I think what's going on is GCC proved that
the value *could* be zero, and hence it *could* return NULL, and hence it was
undefined behavior. Hooray for `-fsanitize=undefined`.
We originally inherited LGPL 2.0 from glib I think. But
I didn't notice when importing systemd code it's LGPL 2.1.
While individual file licenses still apply; I'm not going
to bother bumping all of them to 2.1, the complete module
should be viewed as under 2.1.
Bump the master COPYING file accordingly.
By taking both fd and path into one copy of the reader func, exactly like we do
in `read_xattr_name_array`, we can abstract over the difference.
Preparatory cleanup for more work here.
We should be robust in the face of this and return a snapshot of the current
value we saw, not transiently fail. This is the semantics we expect with ostree
upgrades for `/etc` for example.
To get the right sized buffer to pass to `flistattr` and `llistattr` we
first call them with a zero byte buffer. They then return the number of
bytes they'll actually need to operate. We would `malloc` and then call
again assuming that the size we got originally was correct.
On my computer at least this isn't always the case. I've seen instances
where the first call returns 23B, but then on the second one returns no
data at all. Getting these non-existant xattrs would then cause ostree
to fail.
I'm not sure why it's behaving this way on my machine. I suspect its some
interaction with overlayfs but I haven't proven this.
I was looking at ostree performance, and a surprising amount of
time was spent in `glnx_gen_temp_name()`. We end up calling it
from the main loop, and the iteration here shows up in my perf
profiles.
The glibc algorithm here that we adopted is *very* dated; let's
switch to use `GRand`, which gives us a better algorithm.
It'd be even better of course to use `getrandom()`, but we should do that in
glib at some point.
While I had the patient open, I extended the charset with lowercase, to better
avoid collisions.
And use it when deinitializing, to avoid calling `closedir(NULL)`.
In practice, this doesn't matter, because `closedir` *does* handle `NULL`
in glibc.
However, I'm playing with the GCC `-fsanitize=undefined`, and it
aborts because `closedir` is tagged as requiring a non-`NULL` pointer.
I wanted to add a new one, and realized it was wrong. Luckily,
I think we were safe until now, since the set of bits for `(0, 1, 2)`
is actually distinct.
Although, hm, callers specifying `GLNX_FILE_COPY_OVERWRITE` may
have not actually been getting that.
In some cases we want to replace with zero size, and `posix_fallocate()`
is documented to return `EINVAL` in this case.
Making this change since I noticed it elsewhere.
A wild sordid tale of substractions and unsigned integers leads this
team of variables down a loonng path...
Reported-by: Gatis Paeglis <gatis.paeglis@qt.io>
This is kind of an ABI change but it's for the better I think; on
error we consistently clean up the temp file.
This is obviously necessary without `O_TMPFILE`. With it, we still
need an error cleanup in the case where we're trying to replace an
existing file. I noticed this in ostree's `tests/test-refs.sh` which
intentionally tries to rename a file over a directory path.
While auditing this code to figure out why ostree's
`tests/test-refs.sh` was failing, while the bug turned out to be
different, I noticed that in the case where `dfd != target_dfd`, we
failed to do the right `renameat()`. (No code I'm aware of does this
now).
We had a bug previously where we failed to clean up a temporary file
in an error path. This is a classic case where the new `O_TMPFILE`
API in Linux is nicer.
To implement this, as usual we start with some original bits from
systemd. But in this case I ended up having to heavily modify it
because systemd doesn't support "link into place and overwrite". They
don't actually use their tempfile code much at all in fact - as far as
I can tell, just in the coredump code.
Whereas in many apps, ostree included, a very common use case is
atomically updating an existing file, which is
`glnx_file_replace_contents_at()`, including subtleties like doing an
`fdatasync()` if the file already existed.
Implementing this then is slightly weird since we need to link() the
file into place, then rename() after.
It's still better though because if we e.g. hit `ENOSPC` halfway
through, we'll clean up the file automatically.
We still do keep the mode where we error out if the file exists.
Finally, the ostree core though does have a more unusual case where we
want to ignore EEXIST (allow concurrent object writers), so add
support for that now.
Note: One really confusing bug I had here was that `O_TMPFILE` ignores
the provided mode, and this caused ostree to write refs that weren't
world readable.
Rework things so we always call `fchmod()`, but as a consequence we're
no longer honoring umask in the default case. I doubt anyone will
care, and if they do we should probably fix ostree to consistently use
a mode inherited from the repo or something.
We noticed the temp files being left over in ostree when (mistakenly)
trying to replace the contents of a subpath that wasn't a directory.
In the future we should look at the systemd code using `O_TMPFILE`
here.
Padding in the percentage case was useless (and actually didn't work
properly) since all the real estate is taken up by the text and the bar.
We only need padding in the text case, in case the new string is
shorter.
For rpm-ostree's use we always run in a new root, so we don't want to
inherit the host system's PATH. For example, NixOS uses PATH for its
software namespacing, but one could be using rpm-ostree to build
CentOS commits.
We're ignoring the result from the close, but it can still affect
errno, which is bad if you use this in functions that sets
errno, because errno can unexpectedly change after you've set it.
