| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
|
|
|
|
|
| |
This showed up in https://github.com/projectatomic/rpm-ostree/issues/883
We'll have to audit callers to be sure to avoid double-prefixing.
|
|
|
|
|
|
|
|
|
|
|
|
| |
In a lot of places in ostree, we end up prefixing errors in the *caller*.
Often we only have 1-2 callers, and doing the error prefixing isn't
too duplicative. But there are definitely cases where it's cleaner
to do the prefixing in the callee. We have functions that aren't
ported to new style for this reason (they still do the prefixing
in `out:`).
Introduce a cleanup-oriented version of error prefixing so we can port those
functions too.
|
|
|
|
| |
For consistency.
|
|
|
|
|
| |
There are a number of versions of this in ostree at least, might as well wrap
it.
|
|
|
|
|
| |
There are only two users of this in ostree, and one of them is
fairly bogus; we can just use `fstat()`.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Mostly in ostree/rpm-ostree, we work in either raw `int fd`, or
`G{Input,Output}Stream`. One exception is the rpm-ostree `/etc/passwd`
handling, which uses `FILE*` since that's what glibc exposes.
And in general, there are use cases for `FILE*`; the raw `GUnixOutputStream` for
example isn't buffered, and doing so via e.g. `GBufferedOutputStream` means
allocating *two* GObjects and even worse going through multiple vfuncs for every
write.
`FILE*` is used heavily in systemd, and provides buffering. It is a bit cheaper
than gobjects, but has its own trap; by default every operation locks a mutex.
For more information on that, see `unlocked_stdio(3)`. However, callers can
avoid that by using e.g. `fwrite_unlocked`, which I plan to do for most users of
`FILE*` that aren't writing to one of the standard streams like `stdout` etc.
|
| |
|
|
|
|
| |
Work around an older glibc bug.
|
|
|
|
|
|
|
| |
If the user provides a less than pointer-sized type, we'll clobber other things
on the stack.
See https://github.com/ostreedev/ostree/pull/990/
|
|
|
|
| |
Not sure how I missed this before.
|
|
|
|
| |
This was confusing `g-ir-scanner`.
|
|
|
|
|
| |
`g-ir-scanner` is confused by some of the syntax extensions in `G_IN_SET()`;
none of this is applicable to bindings, so just skip it.
|
|
|
|
| |
There was a user of this in the libostree static delta code.
|
|
|
|
|
|
|
| |
I'm not aware of a problem in practice here, but we should do this on general
principle. Writing this patch now because I hit a fd leak in the ostree static
delta processing that was introduced in the tmpfile prep code, but fixed in the
final port.
|
|
|
|
|
|
|
| |
Looking at converting the ostree codebase, iterating over only the
values of a hash table (while ignoring the key) is actually a more
common pattern than I thought. So let's give it its own macro as well so
users don't have to resort to the _KV variant.
|
|
|
|
|
| |
Besides doing `TEMP_FAILURE_RETRY` and `GError` conversion,
these also prefix the error with arguments.
|
|
|
|
|
|
|
|
| |
These macros make it much easier to iterate over a GHashTable. It takes
care of initializing an iterator and casting keys and values to their
proper types.
See the example usage in the docstring for more info.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
I originally tried to get this into GLib:
https://bugzilla.gnome.org/show_bug.cgi?id=783751
But that looks like it's going to fail due to MSVC. Let's add it here at least
so I can start using it tomorrow and not wait for the MSVC team to catch up.
I renamed `glnx-alloca.h` to `glnx-macros.h` as a more natural collective
home for things from systemd's `macro.h`.
Finally, I used a Coccinelle spatch similar to the one referenced
in the above BZ to patch our uses.
|
|
|
|
|
|
| |
The glibc `posix_fallocate()` implementation has a bad fallback,
and further we need to handle `EOPNOTSUPP` for musl.
https://github.com/flatpak/flatpak/issues/802
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This avoids callers having to use `glnx_steal_fd()` on their own; in general, I
think we should implement move semantics like this at the callee level.
Another reason to do this is there's a subtle problem with doing:
```
somefunction (steal_value (&v), ..., error);
```
in that if `somefunction` throws, it may not have taken ownership of the value.
At least `glnx_dirfd_iterator_init_take_fd()` didn't.
|
|
|
|
| |
Not everything, but a good chunk of the remaining bits.
|
|
|
|
|
|
|
|
|
|
|
| |
Add an `initialized` member which means we work by default
in structs allocated with `g_new0` etc. and don't need
a special initializer. This also fixes a bug where
we need to support `src_dfd == -1` or `AT_FDCWD`.
This fixes flatpak which uses AT_FDCWD.
Modified-by: Colin Walters <walters@verbum.org>
|
|
|
|
| |
Just noticed this while reading the code.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The core problem with the previous tmpfile code
is we don't have an autocleanup that calls `unlinkat`
in the non-`O_TMPFILE` case. And even if we did, it'd
be awkward still since the `glnx_link_tmpfile_at()` call
*consumes* the tmpfile.
Fix this by introducing a struct with a cleanup macro. This simplifies a number
of the callers in libostree - a notable case is where we had two arrays, one of
fds, one of paths. It makes other places in libostree a bit more complex, but
that's because some of the commit code paths want to deal with temporary
*symlinks* too.
Most callers are better though - in libglnx itself, `glnx_file_copy_at()` now
correctly unlinks on failure for example.
|
|
|
|
|
|
| |
For completeness. It just looks much cleaner than doing the `, FALSE`
trick. It also takes care of appending the ': ' for you like its errno
version.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
NOTE: This changes the error handling API of `glnx_loop_write()` to be
"old school POSIX" instead of "systemd".
In ostree in a few places we use `g_output_stream_splice()`. I
thought this would use `splice()`, but actually it doesn't today.
They also, if a cancellable is provided, end up dropping into `poll()` for every
read and write. (In addition to copying data to/from userspace).
My opinion on this is - for *local files* that's dumb. In the big picture, you
really only need cancellation when copying gigabytes. Down the line, we could
perhaps add a `glnx_copy_bytes_cancellable()` that only did that check e.g.
every gigabyte of copied data. And when we do that we should use
`g_cancellable_set_error_if_cancelled()` rather than a `poll()` with the regular
file FD, since regular files are *always* readable and writable.
For my use case with rpm-ostree though, we don't have gigabyte sized files, and
seeing all of the `poll()` calls in strace is annoying. So let's have the
non-cancellable file copying API that's modern and uses both reflink and
`sendfile()` if available, in that order.
My plan at some point once this is tested more is to migrate this code
into GLib.
Note that in order to keep our APIs consistent, I switched the systemd-imported
code to "old school POSIX" error conventions. Otherwise we'd have *3* (POSIX,
systemd, and GError) and particularly given the first two are easily confused,
it'd be a recipe for bugs.
|
|
|
|
|
|
|
|
|
|
| |
There's one function that did `unlinkat()` in the cleanup section,
not doing that yet.
Note I uncovered a few bugs in a few places where we didn't preserve errno
before doing an `unlinkat()` in error paths in a few cases.
I also tried to prefix a few more error cases with the system call name.
|
|
|
|
| |
There's a lot more fdio code, starting with some of the easier ones.
|
|
|
|
|
|
| |
Like tmpfs.
See: https://github.com/flatpak/flatpak/issues/686
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
| |
This is actually the first test case in libglnx 🙌; hopefully the
consumers are prepared for us injecting into `TESTS`.
|
|
|
|
|
| |
This is symmetric with an earlier commit which handled a transition from
`size != 0` -> `size = 0`. Now if xattrs are added we retry.
|
|
|
|
|
|
|
| |
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.
|