This partially reverts ed8e86a7d4.
The change to add the criticals (commit ed8e86a7d4) is correct, but
landed too late in the cycle. Let’s downgrade the criticals to debugs
for now, to stop applications seeing a lot of new criticals in their
output. Those criticals are particularly disruptive for command line
applications and unit tests.
Early in the next cycle, the debugs will be re-upgraded to criticals.
This will give applications a whole additional cycle to fix their
ambiguous use of API.
It turned out that a lot of applications have latent bugs around
calling `g_file_info_get_*()` without checking whether an attribute
is set first, and were hence relying on the ‘unknown’ return value
also being an appropriate default for them.
This was compounded by the fact that several non-local GVFS backends
were not setting `GFileInfo` attributes all the time, which caused the
‘missing attribute’ code path to be hit more frequently. For example,
they would only call `g_file_info_set_is_hidden()` with a true value and
never bother with a false one.
It was further compounded by the fact that, while this change landed for
the 2.75.4 release, there did not seem to be extensive integration
testing of that release, and distributions and downstreams went straight
to 2.76.0. That meant we missed the window between 2.75.4 and 2.76.0 to
change, fix or revert this behaviour. GLib relies on distros and
downstreams doing integration testing of unstable releases. We test with
downstream GNOME as part of gnome-build-meta, but do not have the
resources to do integration testing for everybody.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
See: #2907
See: #2932
See: #2934
See: #2945
See: #2948
This is written in pseudocode C which omits all the callback boilerplate
for the async calls. This should hopefully make the overall structure of
the loop more obvious.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #352
As suggested on #352 by Owen Taylor (commit put together by Philip
Withnall, but in Owen’s name as it’s his wording).
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #352
Before commit ed8e86a7d4, this function would have silently returned a
zero-valued `GTimeVal` if the correct attributes weren’t present.
That partially regressed in commit ed8e86a7d4, which made it return with
a critical warning, but without zeroing the `GTimeVal`. The critical
warning can be ignored by users (it doesn’t abort the process unless
`G_DEBUG=fatal-criticals` is set), but the change in behaviour of
zeroing the `GTimeVal` could cause bugs.
See: #2907
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
While we can’t check for any events on it, this at least tests that
creating a file monitor works. It should cover the fix from the previous
commit.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: !3241
This should catch regressions in the critical warning fixed in the
previous commit.
The launch has to have several conditions:
- Session bus is running (to avoid the launch happening via the spawn
codepath)
- Use a non-existent D-Bus name (to trigger a launch error)
- Use a launch context (to hit the critical warning code path)
- Not have a startup ID specified in the platform data — this implies
having an empty launch context
- Use an async launch, as that provides an error handling path
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
It’s possible for the startup ID to be `NULL` if one wasn’t provided in
the platform data passed to `launch_uris_with_dbus()`.
Passing `NULL` to `g_app_launch_context_launch_failed()` causes a
critical warning.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
The timeout runs for the entire duration of the test, which is a
function that Meson’s test harness already provides for us.
Meson’s timeout can be easily adjusted by a factor to allow for running
tests more slowly under valgrind. The timeout in the code cannot, which
leads to spurious failures like
https://gitlab.gnome.org/GNOME/glib/-/jobs/2645271.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
If the timeout callback was executed, it would remove the timeout
source, leaving the `g_source_remove()` call in the main function with a
dangling source ID.
Fixes commit 73205b8bbd.
Spotted in https://gitlab.gnome.org/GNOME/glib/-/jobs/2645271.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Previously it was only being set on non-Windows platforms. For
consistency, always set it on Windows too.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2934
Following on from #2907, set various boolean attributes if they have
been requested, or are known for sure, and their value is `FALSE`.
Previously the `FALSE` value would have been implicitly returned by the
getter function, but now doing that without the attribute being
explicitly set will trigger a critical warning.
*Don’t* set these attributes if their value is unknown or there was an
error querying it.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2934
The `is-backup` attribute isn't currently set on Windows. It would
be nice to set such basic attributes on all platforms. Let's set
the attribute to `FALSE` there.
Currently, the `G_FILE_ATTRIBUTE_STANDARD_IS_BACKUP` attribute is set
only when its value is `TRUE`. This is wrong with the latest changes as
the `GLib-GIO-CRITICAL **: 00:54:07.260: GFileInfo created without
standard::is-backup` errors are printed now from the
`g_file_info_get_is_backup` function among others. Let's set this
aattribute also when it is `FALSE`.
Related: https://gitlab.gnome.org/GNOME/glib/-/issues/2934
The lost+found dir isn't detected as hidden currently. This is regression
caused by the commit 728ad64b. Let's change the code a bit to be sure that
the lost+found dir is marked as hidden again.
When the `g_file_copy` function is used with files on BTRFS, the
`GLib-GIO-FATAL-CRITICAL: GFileInfo created without standard::size`
error is printed. This is because the `g_file_get_size` function
is used to obtain the file size for the progress callback, but it uses
the wrong `GFileInfo` object that is meant for attributes to be copied
with the file. The file size attribute is missing there obviously. Let's
obtain the file size over the `fstat` call the same way as it is done in
the `splice_stream_with_progress` function to get rid of those errors
and to fix the progress reporting.
This fixes:
| ../glib-2.75.3/gio/tests/cxx.cpp: In function 'int main(int, char**)':
| ../glib-2.75.3/gio/tests/cxx.cpp:61:15: error: missing sentinel in function call [-Werror=format=]
| 61 | g_test_init (&argc, &argv, NULL);
if built with musl libc
Signed-off-by: Markus Volk <f_l_k@t-online.de>
Add a missing steal call in `schedule_method_call()`. This introduces no
functional changes, but documents the ownership transfer more clearly.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2924
This `GDBusMethodInvocation` may be shared across threads, with no
guarantee on the strong ref in one thread outlasting any refs in other
threads — so it needs a ref in this helper struct.
This should fix a use-after-free where the `GDBusMethodInvocation` is
freed from `g_value_unset()` after `g_signal_emit()` returns in
`dispatch_in_thread_func()` in one thread; but then dereferenced again
in `g_source_destroy_internal()` from another thread.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2924
The `GDBusInterfaceSkeleton` is already stored as the source object of
the `GTask` here, with a strong reference.
Storing it again in the task’s data struct is redundant, and makes it
look like the `GDBusInterfaceSkeleton` is being used without holding a
strong reference. (There’s not actually a bug there though: the strong
reference from the `GTask` outlives the data struct, so is sufficient.)
Remove the unnecessary helper struct member to clarify the code a bit.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2924
This introduces no functional changes; just made it while trying to
debug issue #2925.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2925
Otherwise it’s possible for it to hang around in the `GMainContext`
after the “send message” operation has finished. In the best case, this
will cause the `GTask` and `GDBusMessage` to not be freed when the
calling code expects. In the worst case, it could cause use-after-free
problems if it derefs allocations which have since been freed.
I have not seen either of these problems in practice, but it would be
best for the code to eliminate the risk of them altogether by explicitly
destroying the source when the operation is finished.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2925
mkstemp-like family of functions also use g_open () under the hood so
they should pass the O_CLOEXEC flag there for race-free setting of the
close-on-exec flag.
setmntent () call uses the same mode flag set as fopen (), so it should
also include the "e" mode flag for race-free setting of the close-on-exec
flag.
All Unix CRTs examined: glibc, musl, BSDs, Apple libc, Android bionic
ignore unknown fopen () mode flags, so this flag can be added
unconditionally for Unix builds.
Only Windows CRT is intolerant of these, so the single case in
g_dbus_address_connect () where the fopen () call is shared between Unix
and Windows needs appropriate platform-specific handling.
Skipped the call sites in libcharset and xdgmime copylibs.
The `equal_func` closure can already have all required information
available without the item, and passing the item via the closure instead
of an explicit parameter is more natural for languages that have a
concept of closures that can capture variables.
