g_variant_n_children() returns a size_t (or gsize, which is equivalent)
so in principle it could overflow an int, although in practice we are
not going to have that many extended attributes. This silences a
warning from gcc -Wsign-compare.
Signed-off-by: Simon McVittie <smcv@collabora.com>
fallocate() is only visible in fcntl.h if _GNU_SOURCE is defined.
Most users of libglnx will want to do that anyway, but it seems nicer
to avoid "implicit declaration of function ‘fallocate’" warnings from
simply including <libglnx.h> into naive code.
Signed-off-by: Simon McVittie <smcv@collabora.com>
What isn't tested usually doesn't work, and the only way to use libglnx
is as a subproject, so test it like that.
Signed-off-by: Simon McVittie <smcv@collabora.com>
If we're building libglnx as a subproject in a larger project, we won't
necessarily want to run these.
Signed-off-by: Simon McVittie <smcv@collabora.com>
One of Flatpak's unit tests uses this, for _GLNX_TEST_SCOPED_TEMP_DIR.
Its API is not really stable, but libglnx is a "copylib" anyway, so
none of it has (or needs) a strictly stable API.
Signed-off-by: Simon McVittie <smcv@collabora.com>
It seems that on SELinux systems, files in /proc have extended
attributes that cannot be copied by an unprivileged user.
Signed-off-by: Simon McVittie <smcv@debian.org>
To fill in some gaps, I've had to make some assumptions:
* trivial changes (such as checking for an additional function or
header file in libglnx.m4) are assumed to not be copyrightable
* Will Thompson and Matthew Leeds are assumed to be contributing on
behalf of Endless Mobile Inc.
* files with no explicit licensing information are assumed to be
under the license found in COPYING
Reference: https://reuse.software/
Signed-off-by: Simon McVittie <smcv@debian.org>
Some pseudo-files in /proc and /sys list a size of 0 bytes in stat()
results, but do in fact have content. For these pseudo-files, the only
way to find out the true file size is to try read() until EOF, so leave
the max_bytes set to -1.
copy_file_range() and sendfile() don't work for such files in some
kernel versions (see <https://lwn.net/Articles/846403/>), so skip the
fast-paths and use a read() and write() loop. For pseudo-files, we
expect to be able to copy the whole content in one read() in any case.
Signed-off-by: Simon McVittie <smcv@debian.org>
This avoids colliding with a config.h generated by a parent Meson
project.
In the Meson build system, we generate libglnx-config.h by doing our
own checks, so we want to avoid it colliding.
In the Autotools build system, we assume that the parent project will
generate its own config.h that contains the results of LIBGLNX_CONFIGURE,
and create a forwarding header libglnx-config.h in the $(top_builddir)
(so that it is next to config.h).
Note that after updating libglnx in an Autotools non-recursive-Make
project (libostree, flatpak, flatpak-builder) it will be necessary to
re-run autogen.sh.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This doesn't really matter, since it only happens when our process is
about to exit anyway, but it makes it easier to use AddressSanitizer
and similar tools.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Otherwise, clang diagnoses it as unused. It is - deliberately - only
allocated and cleaned up, with no other use.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Often, the caller doesn't actually care about the details of the stat
struct itself, but just whether the entry exists or not. It does work
to just pass `NULL` directly to glibc in a quick test, but given that
the argument is tagged as `__nonnull` and that the documentation does
not explicitly specify this is supported, let's do this safely.
Particularly if `AT_FDCWD` is used, we need to open
in the target dir, otherwise we can get `EXDEV` when trying
to do the final link.
(Theoretically we can cross a mountpoint even with fd-relative
though this is a lot less likely)
Having our tests forced into a `goto out` style is seriously annoying
since we can't write tests like we write production code. Add
a macro that checks for the error being NULL.
This doesn't fully solve the problem since the test functions are
still forced into `void` returns; at some point I may extend
GLib to have `g_test_add_err_func()`.
I was working on rpm-ostree unified core, and hit the fact that
`glnx_file_copy_at()` had the same bug with `fsetxattr()` and files whose mode
is <= `0400` (e.g. `000` in the case of `/etc/shadow`) that libostree did a
while ago. Basically, Linux currently allows `write()` on non-writable open files
but not `fsetxattr()`. This situation is masked for privileged (i.e.
`CAP_DAC_OVERRIDE`) code.
Looking at this, I think it's cleaner to convert to `O_TMPFILE` here,
since that code already handles setting the tmpfile to mode `0600`. Now,
this *is* a behavior change in the corner case of existing files which
are symbolic links. Previously we'd do an `open(O_TRUNC)` which would follow
the link.
But in the big picture, I think the use cases for `open(O_TRUNC)` are really
rare - I audited all callers of this in ostree/rpm-ostree/flatpak, and all of
them will be fine with this behavior change. For example, the ostree `/etc`
merge code already explicitly unlinks the target beforehand. Other cases like
supporting `repo/pubring.gpg` in an ostree repo being a symlink...eh, just no.
Making this change allows us to convert to new style, and brings all of the
general benefits of using `O_TMPFILE` too.
While reading a strace I noticed a double close in the tests; this was because
we were missing an assignment to `-1` in the tests. However, let's make
supporting this clearer by explicitly supporting the fd being `-1` while still
setting the `initialized` variable to `FALSE`. We also add the `EBADF` assertion
checking.
This is a very common pattern in both ostree/rpm-ostree. Make a better API for
this. I thought a lot about simply zeroing out `struct stat` but that feels
dangerous; none of the values have seem obviously `cannot be zero`.
Basically all of the ostree/rpm-ostree callers want to both create and open, so
let's merge `glnx_mkdtempat()` and `glnx_mkdtempat_open()`.
Second, all of them want to do `glnx_shutil_rm_rf_at()` on cleanup, so we do the
same thing we did with `GLnxTmpfile` and create `GLnxTmpDir` that has a cleanup
attribute.
The cleanup this results in for rpm-ostree is pretty substantial.
This is kind of long overdue. Reasons are the same as the other wrappers. I
debated adding `O_NOFOLLOW` support but the use cases for that are pretty
obscure, callers who want that can just use the syscall directly for now.
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.
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.
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.
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.
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
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`.
