Convert libglnx to subtree #690
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This drops a lot of duplicate code.
Some systems have bugs with it, so let's allow downstreams to easily disable it. https://bugzilla.gnome.org/show_bug.cgi?id=769453 ostreedev/ostree#421
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.
Signed-off-by: Simon McVittie <[email protected]>
I'm porting rpm-ostree and need this. Of course all this libcontainer stuff will be nuked in favor of bubblewrap when everything comes together.
No longer used by anything; see coreos/rpm-ostree#429
This is needed by ostree when creating a tarball with make dist.
See https://mail.gnome.org/archives/ostree-list/2016-October/msg00003.html Basically https://github.com/wrpseudo/pseudo doesn't implement newer APIs like renameat2() and O_TMPFILE, so on the host side (as potentially opposed to the target system) we want to be able to disable them.
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.
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 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.
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.
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.
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.
This is symmetric with an earlier commit which handled a transition from `size != 0` -> `size = 0`. Now if xattrs are added we retry.
This is actually the first test case in libglnx 🙌; hopefully the consumers are prepared for us injecting into `TESTS`.
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 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`.
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: GNOME/libglnx#36
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);
```
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
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
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.
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
There's a lot more fdio code, starting with some of the easier ones.
backport-testutils: Add g_assert_true(), g_assert_false() See merge request GNOME/libglnx!44
g_memdup2() replaces g_memdup(), which is prone to integer overflow on 64-bit systems if copying a very large object with an attacker-controlled size. The original version in GLib is extern, but it seems simple enough to inline a backport. Related: https://gitlab.gnome.org/GNOME/glib/-/issues/2319 Signed-off-by: Simon McVittie <[email protected]>
Signed-off-by: Simon McVittie <[email protected]>
backports: Add a backport of g_memdup2() See merge request GNOME/libglnx!46
This is essentially the same as glnx_steal_fd, so make glnx_steal_fd an alias for it. The unit test is taken from GLib, slightly modified to avoid g_close() and also test the old name glnx_steal_fd(). Signed-off-by: Simon McVittie <[email protected]>
backports: Add g_steal_fd, from GLib >= 2.70 See merge request GNOME/libglnx!47
This is a very useful utility function that allows you to compare two strings arrays. Signed-off-by: Ludovico de Nittis <[email protected]>
When testing something that is expected to fail or crash, it's useful to disable core dump reporting. This is a slightly simplified version of GNOME/glib!3510: because libglnx isn't portable to non-Linux, we can assume that <sys/prctl.h>, prctl() and <sys/resource.h> are always available, so we don't need to check for them in the build system. Signed-off-by: Simon McVittie <[email protected]>
backports: Add g_strv_equal from GLib >= 2.60 See merge request GNOME/libglnx!48
testutils: Add a backport of g_test_disable_crash_reporting() See merge request GNOME/libglnx!50
This allows doing:
meson setup _build
meson test -C _build
without an intervening `meson compile` step. Previously, this would have
failed in `tests/testing` because `testing-helper` was never compiled.
For a project this small, there's no real need to distinguish precisely
which tests need the helper: we can have a simpler build by just
assuming that they all do, like Autotools would.
Signed-off-by: Simon McVittie <[email protected]>
build: Make tests depend on the helper required by one of them See merge request GNOME/libglnx!51
We weren't passing the right path argument here. This bug would've been quickly noticed if the function were actually used but I still did at least a global GitHub search which didn't return any users.
glnx-xattrs: Fix invalid argument to lsetxattr() See merge request GNOME/libglnx!52
These will be new API in GLib 2.79.2. The only code change in the implementation, other than the _glnx wrappers, was to use `close()` instead of `g_close (fd, NULL)` in a context where it must be async-signal safe. The test-case needed some more adjustments to be backwards-compatible with GLib from the distant past. Signed-off-by: Simon McVittie <[email protected]>
backports: Add a backport of g_closefrom(), g_fdwalk_set_cloexec() See merge request GNOME/libglnx!53
Signed-off-by: Simon McVittie <[email protected]>
Taken from systemd, but replacing assert_cc with G_STATIC_ASSERT. Signed-off-by: Simon McVittie <[email protected]>
This is not the same as systemd's, because systemd's disagrees with glibc on the signedness of the arguments (systemd/systemd#31270). Signed-off-by: Simon McVittie <[email protected]>
Signed-off-by: Simon McVittie <[email protected]>
Signed-off-by: Simon McVittie <[email protected]>
Fixes for g_closefrom() backport See merge request GNOME/libglnx!54
…39841d1210b' git-subtree-dir: subprojects/libglnx git-subtree-mainline: 549d4a3 git-subtree-split: 202b294 Signed-off-by: bbhtt <[email protected]>
Merged
|
Prior subtree art is at flatpak/flatpak#5800 The full history being here is a bit icky but meh, we can also squash add the subtree with which will create a merge commit and another commit to add the subtree. |
Not entirely sure why flatpak decided to keep the history; but having a single commit would be my preference as well. |
|
Having the merge commit (and hence the history) is how (I think that means this merge request can't be merged correctly using the web-UI - someone would have to push it to |
|
We can update but the commits will squashed, here it is with squashed adding #692 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
17 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.