In the typical `while (g_file_enumerator_next_file ())` patterns,
there is nothing much checking whether the operation was cancelled
on the GIO side. Unless the user checks for the case, this means
local enumerators always run to completion even if cancelled.
Fix this by checking the cancellable state explicitly for local
enumerators, so there are oportunities for bailing out early if
the enumerator is going through a very large directory.
It's redundant, which leads to impossible code like:
if (child_watch_source->using_pidfd)
{
if (child_watch_source->poll.fd >= 0)
close (child_watch_source->poll.fd);
GChildWatchSource uses waitpid(), among pidfd and GetExitCodeProcess().
It thus only works for child processes which the user must ensure to
exist and not being reaped yet. Also, the user must not kill() the PID
after the child process is reaped and must not race kill() against
waitpid(). Also, the user must not call waitpid()/kill() after the child
process is reaped.
Previously, GChildWatchSource would call waitpid() already when adding
the source (g_child_watch_source_new()) and from the worker thread
(dispatch_unix_signals_unlocked()). That is racy:
- if a child watcher is attached and did not yet fire, you cannot call
kill() on the PID without racing against the PID being reaped on the
worker thread. That would then lead to ESRCH or even worse, killing
the wrong process.
- if you g_source_destroy() the source that didn't fire yet, the user
doesn't know whether the PID was reaped in the background. Any
subsequent kill()/waitpid() may fail with ESRCH/ECHILD or even address
the wrong process.
The race is most visible on Unix without pidfd support, because then the
process gets reaped on the worker thread or during g_child_watch_source_new().
But it's also with Windows and pidfd, because we would have waited for
the process in g_child_watch_check(), where other callbacks could fire
between reaping the process status and emitting the source's callback.
Fix all that by calling waitpid() right before dispatching the callback.
Note that the prepare callback only has one caller, which pre-initializes
the timeout argument to -1. That may be an implementation detail and not
publicly promised, but it wouldn't make sense to do it any other way in
the caller.
Also, note that g_unix_signal_watch_prepare() and the UNIX branch of
g_child_watch_prepare() already relied on that.
Note that the variable source_timeout is already initialized upon
definition, at the beginning of the block.
It's easy to see, that no code changes the variable between the variable
definition, and the place where it was initialized. It was thus
unnecessary.
It's not about dropping the unnecessary code (the compiler could do that
just fine too). It's that there is the other branch of the "if/else", where
the variable is also not initialized. But the other branch also requires
that the variable is in fact initialized to -1, because prepare()
callbacks are free not to explicitly set the output value. So both
branches require the variable to be initialized to -1, but only one of
them did. This poses unnecessary questions about whether anything is
wrong. Avoid that by dropping the redundant code.
- if a child watch source has "using_pidfd", it is never linked in the
unix_child_watches list. Drop that check.
- replace the deep nested if, with an early "continue" in the loop,
if we detect there is nothing to do. It makes the code easier to
read.
Let's move the difference between the win/unix implementations closer to
where the difference is. Thereby, we easier see the two implementations
side by side. Splitting it at a higher layer makes the code harder to
read.
This is just a preparation for what comes next.
MRs are already tested in CI before merge, so it’s redundant and a waste
of resources to test them again after merge.
In the rare case where something breaks post-merge (perhaps because
several MRs have been tested individually and merged, but interact with
each other badly), that’ll be caught in the weekly scheduled CI run.
YAML inspiration from https://stackoverflow.com/questions/63893431/gitlab-run-a-pipeline-job-when-a-merge-request-is-merged
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
commit bfbe7127d5 which did a code refactor in
gobject_gdb.py introduced a bug by failing to
return the signal name when a signal had no
'detail', this was preventing pretty printing
name for signals with no 'detail'.
When `copy_file_range()` support was added, I used the definition of
`copy_file_range()` from Linux, which uses `loff_t` to abstract the
different `off*_t` types.
`loff_t` doesn’t exist on FreeBSD, so this doesn’t compile, and was
caught in subsequent asynchronous CI.
Define `loff_t` with a fallback value if it’s not defined, which should
fix this and other uses of `loff_t` in `gfile.c` (for example, if
FreeBSD ever starts declaring `splice()`).
Fixes this CI failure: https://gitlab.gnome.org/GNOME/glib/-/jobs/2812302
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
GTK lost it's '+' suffix back in 2019, according to
<https://mail.gnome.org/archives/gtk-devel-list/2019-February/msg00000.html>
This commit can be re-generated with:
git grep -l GTK+ \
| grep -v -e ^NEWS -e ^glib/tests/collate.c \
| xargs sed -i 's/GTK+/GTK/g'
Most of the changes are in comments and documentation.
Use more modern styling to the code added in the previous patch:
- split 'label: stmt; stmt;' into multiple lines
- add default: label with g_assert_not_reached() [yes, it's a bit
weird adding an assertion inside code that handles assertions, but
we should be okay since g_assertion_message_* are not public
functions and should only be used by our macros]
- use <inttypes.h> for shorter format strings
Note, however, that using uint64_t in gtestutils.h is not feasible,
since it would require adding an '#include <stdint.h>' with potential
unintended namespace pollution to older clients.
Signed-off-by: Eric Blake <eblake@redhat.com>
While x86_64 has enough precision in long double to do a round trip
from guint64 to long double and back, this is platform-specific, and
is a disservice to users trying to debug failing unit tests on other
architectures where it loses precision for g_assert_cmp{int,uint,hex}.
See also https://bugzilla.gnome.org/show_bug.cgi?id=788385 which
mentions having to add casts to specifically silence the compiler on
platforms where the precision loss occurs.
Meanwhile, g_assert_cmpuint() does an unsigned comparison, but outputs
signed values if the comparison fails, which is confusing.
Fix both issues by introducing a new g_assertion_message_cmpint()
function with a new 'u' numtype. For backwards compatibility, the
macros still call into the older g_assertion_message_cmpnum() when not
targetting 2.78, and that function still works when passed 'i' and 'x'
types even though code compiled for 2.78 and later will never invoke
it with numtype anything other than 'f'. Note that g_assert_cmpmem
can also take advantage of the new code, even though in practice,
comparison between two size_t values representing array lengths that
can actually be compiled is unlikely to have ever hit the precision
loss. The macros in signals.c test code does not have to worry about
versioning, since it is not part of the glib library proper.
Closes#2997
Signed-off-by: Eric Blake <eblake@redhat.com>
While it can’t be used in all situations, it is a little bit faster than
`splice()` in some situations, basically if the file system supports
copy on write. In other situations it’s no slower than `splice()`.
See `man copy_file_range` for the situations where it doesn’t work. In
all of these situations, it will return an error, and the GLib code will
fall through and try the existing `splice()` copy code instead.
From my testing of `time gio copy A B` with a 9GB file, the `splice()`
code path takes 22s, and the `copy_file_range()` code path takes 20s.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2863
The start of the `g_file_copy()` implementation stats the source file to
find all the attributes to copy onto the destination file, so it makes
sense to get it to store the source file size at the same time.
This saves a subsequent `stat()` call on the source FD in the btrfs
reflink or splice code. Every little helps.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
When cross-building with a non-Linux target, linux/netlink.h will not
actually be buildable, even if distributions typically put it in
/usr/include and thus exposed to the cross-compiler.
Something has changed recently which causes this error to now be emitted
when building on Windows msys2-mingw32:
```
../gio/gwin32networkmonitor.c: In function 'win_network_monitor_get_ip_info':
../gio/gwin32networkmonitor.c:92:15: error: storing the address of local variable 'prefix' in '*dest' [-Werror=dangling-pointer=]
92 | *dest = (guint8 *) &prefix.Prefix.Ipv4.sin_addr;
| ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
If `IP_ADDRESS_PREFIX` is defined as a scalar rather than a pointer,
that could explain the problem.
Change the function to always operate on a pointer to avoid any
potential such issues.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
