This test ensures that g_socket_client_connect_to_host_async() fails if
it is cancelled, but it's not cancelled until after 1 millisecond. Our
CI testers are hitting that race window, and Milan is able to reproduce
the crash locally as well. Switching it from 1ms to 0ms is enough for
Milan to avoid the crash, but not enough for our CI, so let's move the
cancellation to a GSocketClientEvent callback where the timing is
completely deterministic.
Hopefully fixes#2221
g_has_typeof macro is wrongly in the public g_ namespace, internaly
symbols are usually in the glib_ namespace. This will also allow to
define glib_typeof differently on non-GNUC compilers (e.g. c++11
decltype).
This introduces no functional changes, but makes the refcount handling a
little easier to follow by no longer splitting a ref/unref pair across
three callbacks. Now, the ref/unref pairs are all within function-local
scopes.
Coverity CID: #1430783
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
By default, when using g_subprocess_launcher_take_fd() to pass an
FD to a child, the GSubprocessLauncher object also takes ownership
of the FD in the parent, and closes it during finalize(). This is
a reasonable assumption in the majority of the cases, but sometimes
it isn't a good idea.
An example is when creating a GSubprocessLauncher in JavaScript:
here, the destruction process is managed by the Garbage Collector,
which means that those sockets will remain opened for some time
after all the references to the object has been droped. This means
that it could be not possible to detect when the child has closed
that same FD, because in order to make that work, both FDs
instances (the one in the parent and the one in the children) must
be closed. This can be a problem in, as an example, a process that
launches a child that communicates with Wayland using an specific
socket (like when using the new API MetaWaylandClient).
Of course, it isn't a valid solution to manually call close() in
the parent process just after the call to spawn(), because the FD
number could be reused in the time between it is manually closed,
and when the object is destroyed and closes again that FD. If that
happens, it will close an incorrect FD.
One solution could be to call run_dispose() from Javascript on the
GSubprocessLauncher object, to force freeing the resources.
Unfortunately, the current code frees them in the finalize()
method, not in dispose() (this is fixed in !1670 (merged) ) but it
isn't a very elegant solution.
This proposal adds a new method, g_subprocess_launcher_close(),
that allows to close the FDs passed to the child. To avoid problems,
after closing an FD with this method, no more spawns are allowed.
Fix: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1677
Originally, GSocketClient returned whatever error occured last. Turns
out this doesn't work well in practice. Consider the following case:
DNS returns an IPv4 and IPv6 address. First we'll connect() to the
IPv4 address, and say that succeeds, but TLS is enabled and the TLS
handshake fails. Then we try the IPv6 address and receive ENETUNREACH
because IPv6 isn't supported. We wind up returning NETWORK_UNREACHABLE
even though the address can be pinged and a TLS error would be more
appropriate. So instead, we now try to return the error corresponding
to the latest attempted GSocketClientEvent in the connection process.
TLS errors take precedence over proxy errors, which take precedence
over connect() errors, which take precedence over DNS errors.
In writing this commit, I made several mistakes that were caught by
proxy-test.c, which tests using GSocketClient to make a proxy
connection. So although adding a new test to ensure we get the
best-possible error would be awkward, at least we have some test
coverage for the code that helped avoid introducing bugs.
Fixes#2211
We should never return unknown errors to the application. This would be
a glib bug.
I don't think it's currently possible to hit these cases, so asserts
should be OK. For this to happen, either (a) a GSocketAddressEnumerator
would have to return NULL on its first enumeration, without returning an
error, or (b) there would have to be a bug in our GSocketClient logic.
Either way, if such a bug were to exist, it would be better to surface
it rather than hide it.
These changes are actually going to be effectively undone in a
subsequent commit, as I'm refactoring the error handling, but the commit
history is a bit nicer with two separate commits, so let's go with two.
GSocketAddressEnumerator encapsulates the details of how DNS happens, so
we don't have to think about it. But we may have taken encapsulation a
bit too far, here. Usually, we resolve a domain name to a list of IPv4
and IPv6 addresses. Then we go through each address in the list and try
to connect to it. Name resolution happens exactly once, at the start.
It doesn't happen each time we enumerate the enumerator. In theory, it
*could*, because we've designed these APIs to be agnostic of underlying
implementation details like DNS and network protocols. But in practice,
we know that's not really what's happening. It's weird to say that we
are RESOLVING what we know to be the same name multiple times. Behind
the scenes, we're not doing that.
This also fixes#1994, where enumeration can end with a RESOLVING event,
even though this is supposed to be the first event rather than the last.
I thought this would be hard to fix, even requiring new public API in
GSocketAddressEnumerator to peek ahead to see if the next enumeration is
going to return NULL. Then I decided we should just fake it: always emit
both RESOLVING and RESOLVED at the same time right after each
enumeration. Finally, I realized we can emit them at the correct time if
we simply assume resolving only happens the first time. This seems like
the most elegant of the possible solutions.
Now, this is a behavior change, and arguably an API break, but it should
align better with reasonable expectations of how GSocketClientEvent
ought to work. I don't expect it to break anything besides tests that
check which order GSocketClientEvent events are emitted in. (Currently,
libsoup has such tests, which will need to be updated.) Ideally we would
have GLib-level tests as well, but in a concession to pragmatism, it's a
lot easier to keep network tests in libsoup.
This isn't an API guarantee, but it's a potentially-surprising
behavior difference between the sync and async functions that is good
to know about, especially because our sync and async functions are
normally identical.
The linux kernel does not know that the socket will be used
for connect or listen and if you bind() to a local address it must
reserve a random port (if port == 0) at bind() time, making very easy
to exhaust the ~32k port range, setting IP_BIND_ADDRESS_NO_PORT tells
the kernel to choose random port at connect() time instead, when the
full 4-tuple is known.
`g_local_file_fstatat()` needs to fall back to returning an error if
`fstatat()` isn’t defined, which is the case on older versions of macOS
(as well as Windows, which was already handled). Callers shouldn’t call
`g_local_file_fstatat()` in these cases. (That’s already the case.)
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2203
Expose a function that prepares an attribute query string to be passed
to g_file_query_info() to get a list of attributes normally copied with
the file. This function is used by the implementation of
g_file_copy_attributes, and it's useful if one needs to split
g_file_copy_attributes into two stages, for example, when nautilus does
a recursive move of a directory. When files are moved from the source
directory, its modification time changes. To preserve the mtime on the
destination directory, it has to be queried before moving files and set
after doing it, hence these two stages.
Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
The GSubprocessLauncher class lacks a dispose() method, and frees
all their resources in the finalize() method.
This is a problem with Javascript because the sockets passed to a
child process using g_subprocess_launcher_take_fd() aren't closed
in the parent space until the object is fully freed. This means
that if the child closes a socket, it won't be detected until the
GSubprocessLauncher object has been freed by the garbage
collector.
Just closing the socket externally is not a valid solution,
because the finalize() method will close it again, and since
another file/pipe/socket could have been opened in the meantime
and use the same FD number, the finalize() method would close
an incorrect FD.
An example is launching a child process that uses its own
socket for Wayland: the parent creates two sockets with
socketpair(), passes one to the Wayland API (wl_client_create()),
and the other is passed to the child process using
g_subprocess_launcher_take_fd(). But now there are two instances
of that second socket: one in the parent, and another one in the
child process. That means that, if the child closes its socket (or
dies), the Wayland server will not detect that until the
GSubprocessLauncher object is fully destroyed. That means that a
GSubprocessLauncher created in Javascript will last for several
seconds after the child dies, and every window or graphical element
will remain in the screen until the Garbage Collector destroys the
GSubprocessLauncher object.
This patch fixes this by moving the resource free code into a
dispose() method, which can be called from Javascript. This allows
to ensure that any socket passed to the child with
g_subprocess_launcher_take_fd() can be closed even from Javascript
just by calling the method run_dispose().
Fix https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1670
This combines a massive code re-folding with functionlity expansion
that allows us to track multiple verbs per handler or per application.
Also fixes a few issues and removes a function that made no sense.
Like G_SOURCE_REMOVE and G_SOURCE_CONTINUE, these make it clearer what
it means to return TRUE or FALSE.
In particular, in GDBus methods that fail, the failure case still needs
to return TRUE (unlike the typical GError pattern), leading to comments
like this:
g_dbus_method_invocation_return_error (invocation, ...);
return TRUE; /* handled */
which can now be replaced by:
g_dbus_method_invocation_return_error (invocation, ...);
return G_DUS_METHOD_INVOCATION_HANDLED;
G_DBUS_METHOD_INVOCATION_UNHANDLED is added for symmetry, but is very
rarely (perhaps never?) useful in practice.
Signed-off-by: Simon McVittie <smcv@collabora.com>
The previous parsing code could read off the end of a URI if it had an
incorrect %-escaped character in.
Fix that, and more closely implement parsing for the syntax defined in
RFC 6874, which is the amendment to RFC 3986 which specifies zone ID
syntax.
This requires reworking some network-address tests, which were
previously treating zone IDs incorrectly.
oss-fuzz#23816
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Emptying trash over `gio trash` is a bit slow in comparison to plain
`rm -r`. On my system, it took about 3 min to empty the trash with a
folder containing 600 000 files, which is not ideal as `rm -r` call
took just a few seconds. I found that `g_file_delete` is implemented
differently for locations provided by the trash backend. The trash
backend prevents modifications of trashed content thus the delete
operation is allowed only for the top-level files and folders. So it
is not necessary to recursive delete all files as the permission
denied error is returned anyway. Let's call `g_file_delete` only for
top-level items, which reduces the time necessary for emptying trash
from minutes to seconds...
See: https://gitlab.gnome.org/GNOME/nautilus/-/issues/1589
Some filesystems don't have meaningful access times under at least some
circumstances (see #2189, #2205). In this situation the traditional stat()
and related kernel interfaces have to put something meaningless in the
st_atime field, and have no way to signal that it is meaningless.
However, statx() does have a way to signal that the atime is meaningless:
if the filesystem doesn't provide a useful access time, it will unset
the STATX_ATIME bit (as well as filling in the same meaningless value
for the stx_atime field that stat() would have used, for compatibility).
We don't actually *need* the atime, so never include it in the required
mask. This was already done for one code path in commit 6fc143bb
"gio: Allow no atime from statx" to fix#2189, but other callers were
left unchanged in that commit, and receive the same change here.
It is not actually guaranteed that *any* of the flags in the
returned stx_mask will be set (the only guarantee is that items in
STATX_BASIC_STATS have at least a harmless compatibility value, even if
their corresponding flag is cleared), so it might be better to follow
this up by removing the concept of the required mask entirely. However,
as of Linux 5.8 it looks as though STATX_ATIME is the only flag in
STATX_BASIC_STATS that might be cleared in practice, so this simpler
change fixes the immediate regression.
Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/2205
Signed-off-by: Simon McVittie <smcv@collabora.com>
It is not allowed to be `NULL` or unset if requested by the file
attribute matcher. Derive it from the basename. This doesn’t handle the
situation of a failed UTF-16 to UTF-8 conversion very well, but will at
least return something.
Note that the `g_filename_display_basename()` function can’t be used as
`GWinHttpFile` provides its URI in UTF-16 rather than in the file system
encoding.
This fixes a crash when using GIMP on Windows. Thanks to lillolollo for
in-depth debugging assistance.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #2194
For interoperability with libdbus, we want to use compatible timeouts.
In particular, this fixes a spurious failure of the `gdbus-server-auth`
test caused by libdbus and gdbus choosing to expire the key (cookie) at
different times, as diagnosed by Thiago Macieira. Previously, the libdbus
client would decline to use keys older than 7 minutes, but the GDBus
server would not generate a new key until the old key was 10 minutes old.
For completeness, also adjust the other arbitrary timeouts in the
DBUS_COOKIE_SHA1 mechanism to be the same as in libdbus. To make it
easier to align with libdbus, create internal macros with the same names
and values used in dbus-keyring.c.
* maximum time a key can be in the future due to clock skew between
systems sharing a home directory
- spec says "a reasonable time in the future"
- was 1 day
- now 5 minutes
- MAX_TIME_TRAVEL_SECONDS
* time to generate a new key if the newest is older
- spec says "If no recent keys remain, the server may generate a new
key", but that isn't practical, because in reality we need a grace
period during which an old key will not be used for new authentication
attempts but old authentication attempts can continue (in practice both
libdbus and GDBus implemented this logic)
- was 10 minutes
- now 5 minutes
- NEW_KEY_TIMEOUT_SECONDS
* time to discard old keys
- spec says "the timeout can be fairly short"
- was 15 minutes
- now 7 minutes
- EXPIRE_KEYS_TIMEOUT_SECONDS
* time allowed for a client using an old key to authenticate, before
that key gets deleted
- was at least 5 minutes
- now at least 2 minutes
- at least (EXPIRE_KEYS_TIMEOUT_SECONDS - NEW_KEY_TIMEOUT_SECONDS)
Based on a merge request by Philip Withnall.
Fixes: #2164
Thanks: Philip Withnall
Thanks: Thiago Macieira
Signed-off-by: Simon McVittie <smcv@collabora.com>
This doesn't trigger the cancellation assertion issue when run locally
(the task didn't return yet, so the error is simply overwritten), but
perhaps it ever does in CI. Anyhow, it's good to have a cancellation
test.
After a splice operation is finished, it attempts to 1) close input/output
streams, as per the given flags, and 2) return the operation result (maybe
an error, too).
However, if the operation gets cancelled early and the streams indirectly
closed, the splice operation will try to close both descriptors and return
on the task when both are already closed. The catch here is that getting the
streams closed under its feet is possible, so the completion callback would
find both streams closed after returning on the first close operation and
return the error, but then the second operation could be able to trigger
a second error which would be returned as well.
What happens here is up to further race conditions, if the task didn't
return yet, the returned error will be simply replaced (but the old one not
freed...), if it did already return, it'll result in:
GLib-GIO-FATAL-CRITICAL: g_task_return_error: assertion '!task->ever_returned' failed
Fix this by flagging the close_async() callbacks, and checking that both
close operations did return, instead of checking that both streams are
closed by who knows.
This error triggers a semi-frequent CI failure in tracker, see the summary at
https://gitlab.gnome.org/GNOME/tracker/-/issues/240
statx does not provide stx_atime when querying a file in a read-only
mounted file system. So call to statx should not expect it to be in
the mask. Otherwise we would fail with ERANGE for querying any file in
a read-only file system.
Fixes#2189.
The `make_pollfd()` call can’t fail because it only does so if
`cancellable == NULL`, and we’ve already checked that. Assert that’s the
case, to shut Coverity up and to catch behavioural changes in future.
Coverity CID: #1159433
Signed-off-by: Philip Withnall <withnall@endlessm.com>
`g_strsplit()` never returns `NULL`, although it can return an empty
strv (i.e. with its first element being `NULL`).
Drop a redundant `NULL` check.
Coverity CID: #1430976
Signed-off-by: Philip Withnall <withnall@endlessm.com>
If `statx()` is supported, query it for the file creation time and use
that if returned.
Incorporating some minor code rearrangement by Philip Withnall
<withnall@endlessm.com>.
Fixes: #1970
This currently just implements the same functionality as the existing
`stat()`/`fstat()`/`fstatat()`/`lstat()` calls, although where a reduced
field set is requested it may return faster.
Helps: #1970
It turns out that our async write operation implementation is broken
on non-O_NONBLOCK pipes, because the default async write
implementation calls write() after poll() said there were some
space. However, the semantics of pipes is that unless O_NONBLOCK is set
then the write *will* block if the passed in write count is larger than
the available space.
This caused a deadlock in https://gitlab.gnome.org/GNOME/glib/-/issues/2182
due to the loop-back of the app stdout to the parent, but even without such
a deadlock it is a problem that we may block the mainloop at all.
In the particular case of g_subprocess_communicate() we have full
control of the pipes after starting the app, so it is safe to enable
O_NONBLOCK (i.e. we can ensure all the code using the fd after this can handle
non-blocking mode).
This fixes https://gitlab.gnome.org/GNOME/glib/-/issues/2182
This has almost the same semantics as WSAECONNRESET and for all
practical purposes is handled the same. The main difference is about
*who* reset the connection: the peer or something in the network.
For UDP sockets this happens when receiving packets and previously sent
packets returned an ICMP "Time(-to-live) expired" message. This is
similar to WSAECONNRESET, which on UDP sockets happens when receiving
packets and previously sent packets returned an ICMP "Port Unreachable"
message.
This is a step towards supporting `statx()`, which allows the set of
fields it returns to be specified by the caller. Currently, the existing
`stat()` and `fstat()` calls continue to be made, and there are no
behavioural changes — but the new wrapper functions will be extended in
future.
Helps: #1970
Don’t call `g_file_query_exists()` followed by `g_file_delete()`. Just
call `g_file_delete()` and check the error.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Make `G_URI_FLAGS_PARSE_RELAXED` available instead, for the
implementations which need to handle user-provided or incorrect URIs.
The default should nudge people towards being compliant with RFC 3986.
This required also adding a new `G_URI_PARAMS_PARSE_RELAXED` flag, as
previously parsing param strings *always* used relaxed mode and there
was no way to control it. Now it defaults to using strict mode, and the
new flag allows for relaxed mode to be enabled if needed.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #2149
Add support for x-gvfs-notrash mount option, which allows to disable
trash functionality for certain mounts. This might be especially useful
e.g. to prevent trash folder creation on enterprise shares, which are
also accessed from Windows...
https://bugzilla.redhat.com/show_bug.cgi?id=1096200
There is already g_unix_mount_at function which allows to find certain
unix mount for given mount path. It would be useful to have similar
function for mount points, which will allow to replace custom codes in
gvfs. Let's add g_unix_mount_point_at.
_g_uri_parse_authority() can be replaced with g_uri_split_network() &
PARSE_STRICT. Keep the original error code, for compatibility reasons.
Notice that GUri uses gint for the port, and value -1 if the port value
is missing. However, GNetworkAddress::port is a guint.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
_g_uri_parse_authority() without argument is actually checking that the
URI is valid, by checking it parses successfully
We keep the existing error domain / code for compatibility reasons,
instead of raising the underlying G_URI_ERROR.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
_g_uri_from_authority() is doing the same work as g_uri_join(): taking
URI components and merging them in a legit URI string, with encoding.
It turns out g_uri_from_authority was unnecessarily complex, since no
caller used the userinfo field.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
It may be defined by the environment (we document that as being allowed)
— if so, individual files should not try to redefine it, as that causes
a preprocessor warning.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Where applicable. Where the current use of `g_file_set_contents()` seems
the most appropriate, leave that in place.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #1302
Give GAppInfo a bunch of readonly properties, and
support them in GDesktopAppInfo. This makes app infos
more convenient to work with in GTK4, and in general.
g_task_set_name() was added in GLib 2.60, so only use it in the
overridden definition of g_task_set_source_tag() if the user has said
that they require GLib ≥ 2.60.
This is a follow up to commit b08bd04abe.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
There is no guarantee that this function would not be called
concurrently. Particularly since flatpak_info_read was set to TRUE
before /.flatpak-info is actually read from disk, there is a potential
race where a second thread would return default values for the various
flags set from that file.
Fixes#2159
Correct an off-by-one error in hex_unescape_string()'s computation of
the output string length.
(Turned into a git-format patch by Philip Withnall. Original patch
submitted on the Debian bug tracker, bug#962912.)
It's safe to assume an escaped string doesn't contain embedded null bytes,
but raw memory buffers (as returned by getxattr()) require more care.
If the length of the data to be escaped is known, use that knowledge instead
of invoking strlen().
(Turned into a git-format patch by Philip Withnall. One minor formatting
tweak. Original patch submitted on the Debian bug tracker, bug#962912.)
Fixes: #422
And improve them externally, where not otherwise set, by setting them
from the function name passed to `g_task_set_source_tag()`, if called by
third party code.
This should make profiling and debug output from GLib more useful.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
`complete_in_idle_cb()` shows up in a lot of sysprof traces, so it’s
quite useful to include the most specific contextual information we can
in it.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
When an app is spawned using g_desktop_app_info_launch_uris_with_spawn
it will expand the various token in the app's commandline with the
URIs of the files to open. The expand_macro() function that is used for
this advances the pointer to the URI list to show up to which entries
it used.
To not loose the pointer to the list head a duplicate of the URI list
was actually passed to expand_macro(). However, it's not necessary to
create a copy of the URI list for that as expand_macro() will only
change which element the pointer will point to.
This behaviour actually caused the duplicated list to be leaked as the
the list pointer is NULL once all URIs are used up by expand_macro()
and thus nothing was freed at the end of the function.
In ostree based systems, such as flatpak and fedora silverblue, the
time of modification of every system file is epoch 0, including
giomodule.cache, which means that every module is loaded and unloaded
every time.
The solution is to use the change time of the file as well. In a typical
system, it is equal to the mtime, and in an ostree based system, since
the directory is mounted as read-only, the user cannot add a module and
we must assume that the cache file corresponds to the modules.
* Add g_tls_connection_get_channel_binding_data API call
* Add g_dtls_connection_get_channel_binding_data API call
* Add get_binding_data method to GTlsConnection class
* Add get_binding_data method to GDtlsConnection interface
* Add GTlsChannelBindingType enum with tls-unique and
tls-server-end-point types
* Add GTlsChannelBindingError enum and G_TLS_CHANNEL_BINDING_ERROR
quark
* Add new API calls to documentation reference gio-sections-common
This speeds up the `cancellable` test a little by stopping waiting for
the threads to start up as soon as they have started, rather than after
an arbitrary timeout.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #1764
This should fix some sporadic test failures in this test, although I
can’t be sure as I was unable to reproduce the original failure.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1764
It seems that allowing the GCancellable to be finalised in either the
main thread or the worker thread sometimes leads to crashes when running
on CI.
I cannot reproduce these crashes locally, and various analyses with
memcheck, drd and helgrind have failed to give any clues.
Fix this for this particular test case by deferring destruction of the
`GCancellable` instances until after the worker thread has joined.
That’s OK because this test is specifically checking a race between
`g_cancellable_cancel()` and disposal of a `GCancellableSource`.
The underlying bug remains unfixed, though, and I can only hope that we
eventually find a reliable way of reproducing it so it can be analysed
and fixed.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
The G_FILE_ATTRIBUTE_FILESYSTEM_REMOTE is set to TRUE only for NFS
filesystem types currently. Let's add also SMB filesystem types. This
also changes g_local_file_is_nfs_home function logic to handle only
NFS filesystems.
The g_local_file_is_remote function is misleading as it works only for
NFS filesystem types and only for locations in home directorly. Let's
rename it to g_local_file_is_nfs_home to make it obvious.
statfs/statvfs is called several times when querying filesystem info.
This is because the G_FILE_ATTRIBUTE_FILESYSTEM_REMOTE attribute is set
over is_remote_fs function, which calls statfs/statvfs again. Let's use
the already known fstype instead of redundant statfs/statvfs calls.
This also changes g_local_file_is_remote implementation to use
g_local_file_query_filesystem_info to obtain fstype, which allows to
remove duplicated code from is_remote_fs function.
The G_FILE_ATTRIBUTE_FILESYSTEM_REMOTE currently works only for locations
in the home directory. Let's make it work also for files outside the home
directory.
There are glocalfile.h and glocalfileprivate.h header files currently.
None of those header files is public, so it doesn't make sense to have
two private headers for glocalfile.c. Let's remove glocalfileprivate.h.
This was mostly machine generated with the following command:
```
codespell \
--builtin clear,rare,usage \
--skip './po/*' --skip './.git/*' --skip './NEWS*' \
--write-changes .
```
using the latest git version of `codespell` as per [these
instructions](https://github.com/codespell-project/codespell#user-content-updating).
Then I manually checked each change using `git add -p`, made a few
manual fixups and dropped a load of incorrect changes.
There are still some outdated or loaded terms used in GLib, mostly to do
with git branch terminology. They will need to be changed later as part
of a wider migration of git terminology.
If I’ve missed anything, please file an issue!
Signed-off-by: Philip Withnall <withnall@endlessm.com>
In glib-networking#127, it was reported that we don't properly implement
the documented behavior of these properties. However, we cannot fix it
because libsoup relies on the implemented behavior, and it's hard to
change that without cascading breakage. The practical solution is to
adjust our documentation to match reality. There should be no downsides
to this, and compat risk of changing the documentation is much smaller
than risk of changing the implementation, so I think this is the best we
can make of an unfortunate situation. See glib-networking#127 for full
discussion and glib-networking#129 for the regression when we attempted
to match the documented behavior.
Some editors automatically remove trailing blank lines, or
automatically add a trailing newline to avoid having a trailing
non-blank line that is not terminated by a newline. To avoid unrelated
whitespace changes when users of such editors contribute to GLib,
let's pre-emptively normalize all files.
Unlike more intrusive whitespace normalization like removing trailing
whitespace from each line, this seems unlikely to cause significant
issues with cherry-picking changes to stable branches.
Implemented by:
find . -name '*.[ch]' -print0 | \
xargs -0 perl -0777 -p -i -e 's/\n+\z//g; s/\z/\n/g'
Signed-off-by: Simon McVittie <smcv@collabora.com>
While these assertions look right at the first glance,
they actually crash the program. That's because GObject
insists on initializing all construct-only properties
to their default values, which results in
g_win32_registry_key_set_property() being called multiple
times with NULL string, once for each unset property.
If "path" is actually set by the caller, a subsequent
call to set "path-utf16" to NULL will fail an assertion,
since absolute_path is already non-NULL.
With assertions moved the set-to-NULL calls bail out before
an assertion is made.
This ensures that we do really export the symbols for Visual
Studio-style builds, by using _GLIB_EXTERN to decorate the generated
prototypes and including config.h so that we are sure the symbols are
actually exported.
This adds three options to gdbus-codegen so that we may be able to
use a self-defined symbol decorator, such as _GLIB_EXTERN, to decorate
the generated prototypes, to be used possibly to export the symbols, if
needed.
The other two options allows including headers that are required for the
specified symbol decorator to be usable and preprocessor macros that are
required for the symbol decorator to be defined appropriately, also when
needed.
Have the generated .c code decorate the prototypes with "G_MODULE_EXPORT"
instead of "extern" when --internal is not being used, so that we also
export the symbols from the generated code on Visual Studio-style
compilers. If --internal is used, we decorate the prototypes with
"G_GNUC_INTERNAL", as we did before.
Note that since the generated .c code does not attempt to include the
generated headers (if one is also generated), the gnerated headers are
still generated as they were before.
Sometimes this test was timing out due to the file monitor notifications
taking longer than the arbitrary 2s delay before ending the test and
checking its results at the end of `iclosed_cb()`.
Avoid that timing-dependence by ending the test when the expected file
monitor notifications are seen, or after a 10s timeout (if so, the test
is failed).
This makes the test run 4× faster in the normal case, as it’s no longer
waiting for a timeout to elapse if the file monitor notifications come
in sooner.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
The test added for #1841 spawned 100000 threads. That was fine on a
desktop machine, but on a heavily loaded CI machine, it could result in
large (and unpredictable) slowdowns, resulting in the test taking over
120s in about 1 in 5 runs, and hence failing that CI pipeline due to a
timeout. When passing normally on CI, the test would take around 90s.
Here’s a histogram of time per iteration on a failing (timed out) test
run. Each iteration is one thread spawn:
Iteration duration (µs) | Frequency
------------------------+----------
≤100 | 0
100–200 | 30257
200–400 | 13696
400–800 | 1046
800–1000 | 123
1000–2000 | 583
2000–4000 | 3779
4000–8000 | 4972
8000–10000 | 1027
10000–20000 | 2610
20000–40000 | 650
40000–80000 | 86
80000–100000 | 10
100000–200000 | 2
>200000 | 0
There’s no actual need for the test to spawn 100000 threads, so rewrite
it to reuse a single thread, and pass new data to that thread.
Reverting the original commit (e4a690f5dd) reproduces the failure on
100 out of 100 test runs with this commit applied, so the test still
works.
The test now takes 3s, rather than 11s, to run on my computer, and has
passed when run with `meson test --repeat 1000 cancellable`.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
An extra argument to g_win32_registry_key_get_value_w() and
g_win32_registry_key_get_value() indicates that RegLoadMUIStringW()
should be used instead of RegQueryValueExW(). It only works on
strings, and automatically resolves resource strings (the ones
that start with "@").
The extra argument is needed to find resource DLLs that are only
specified by their relative name.
It is critical to mention how the identity parameter is expected to be
handled. In particular, if identity is not passed, then the identity of
the server certificate will not be checked at all. This is in contrast
to the connection-level APIs, which are supposed to be fail-safe. The
database and certificate-level APIs are more manual.
There’s no need to call `access()` and then `stat()` on the keyring
directory to check that it exists, is a directory, and has the right
permissions. Just call `stat()`.
This eliminates one potential TOCTTOU race in this code.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #1954
There was a time-of-check-to-time-of-use (TOCTTOU) race in the keyring
lock code, where it would check the existence of the lock file using
`access()`, then proceed to call `open(O_CREAT | O_EXCL)` to try and
create the lock file once `access()` showed that it didn’t exist.
The problem is that, because this is happening in a shared directory
(`~/.dbus-keyrings`), another process could quite legitimately create
the lock file in the meantime.
Instead, unconditionally call `open()` and ignore errors from it (which
will be returned if the lock file already exists) until it succeeds (or
the code times out).
This eliminates the TOCTTOU race, and simplifies the timeout behaviour
so there aren’t two loops (check for existence, try to create)
happening. It brings this code in line with what dbus.git does (see
`_dbus_keyring_lock()`).
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #1954
When multiple tests were run in parallel, this would race on its access
to `~/.dbus-keyrings` to authenticate with the D-Bus server, since the
keyring directory was not appropriately sandboxed to the unit test.
Use `G_TEST_OPTION_ISOLATE_DIRS` to automatically isolate each unit
test’s directory usage.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #1954
Commit 721e385 left one remaining race in the filter test, with a
comment associated with it. Unfortunately, the (seemingly unrelated)
changes in #1841 to `GCancellable` seem to have made this remaining race
a lot more likely to fail on FreeBSD than before.
What’s likely to have happened (although I was unable to reproduce the
failure, due to not having a FreeBSD system; I was only able to
reproduce the problem as a 3/1000 failure on Linux, which is still worth
fixing) is that the atomic write of the `FilterData.serial` to be
expected by the filter function sometimes happened after the filter
function had executed, so the expected message was dropped and didn’t
result in an update to the `FilterData` state.
Rework the test so that instead of setting some expectations (on
`FilterData`) in one thread and then checking them in another thread,
the worker thread just unconditionally returns messages from the filter
function to the main thread, and then the main thread checks whether the
expected one has been filtered.
With this change applied, the `gdbus-connection` test passes 5000 times
in a row for me, on Linux; and doesn’t seem to fail any more on the
FreeBSD CI machines over a few runs. (Previously it failed on 4/5 runs.)
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #2092Fixes: #1957
Mention in the documentation that (presumably for performance reasons)
the search results from `g_desktop_app_info_search()` are not filtered
by executable presence or hidden attribute.
Perhaps they should be in future, but for now we should at least
document it.
Spotted by Will Thompson.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
By default, meson builds glib with -Werror=format=2, which
implies -Werror=format-nonliteral. With these flags, clang errors
out on e.g. the g_message_win32_error function, due to "format
string is not a string literal". This function takes a format
string, and passes the va_list of the arguments onwards to
g_strdup_vprintf, which is annotated with printf attributes.
When passing a string+va_list to another function, GCC doesn't warn
with -Wformat-nonliteral. Clang however does warn, unless the
functions themselves (g_message_win32_error and set_error) are decorated
with similar printf attributes (to force the same checks upon the
caller) - see
https://clang.llvm.org/docs/AttributeReference.html#format
for reference.
Adding these attributes revealed one existing mismatched format string
(fixed in the preceding commit).
The GIO tests memory-monitor-dbus and memory-monitor-portal use a number
of third party Python modules that may not be present when running the
test case.
Instead of failing due to missing imports, catch the ImportError and
mock a test case that skips. This can't use the usual unittest.skip
logic because the test case class itself uses a 3rd party module.
Closes#2083.
There are two memory monitor tests that use Python's unittest module directly,
but GLib tests should be outputting TAP. Use the embedded TAPTestRunner to
ensure that TAP is output for these tests too.
The G_FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE attribute doesn't have to be
always set. See https://gitlab.gnome.org/GNOME/gvfs/-/merge_requests/68
for more details. In that case, the g_file_query_default_handler function
fails with the "No application is registered as handling this file" error.
Let's fallback to the "standard::fast-content-type" attribute instead to
fix issues when opening such files.
https://gitlab.gnome.org/GNOME/nautilus/-/issues/1425
Meson 0.54.0 added a new method meson.override_dependency() that must be
used to ensure dependency consistency. This patch ensures a project that
depends on glib will never link to a mix of system and subproject
libraries. It would happen in such cases:
The system has glib 2.40 installed, and a project does:
dependency('glib-2.0', version: '>=2.60',
fallback: ['glib', 'glib_dep'])
dependency('gobject-2.0')
The first call will configure glib subproject because the system libglib
is too old, but the 2nd call will return system libgobject.
By overriding 'gobject-2.0' dependency while configuring glib subproject
during the first call, meson knows that on the 2nd call it must return
the subproject dependency instead of system dependency.
This also has the nice side effect that with Meson >0.54.0 an
application depending on glib can declare the fallback without knowing
the dependency variable name: dependency('glib-2.0', fallback: 'glib').
Slightly unexpectedly, `g_icon_serialize()` doesn’t produce a floating
`GVariant`, it produces one with full ownership and returns that. That’s
not the convention for `GVariant` return values from functions which
build variants, but there’s nothing we can do to change this now as that
would be an API break.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
D-Bus filter functions run in a worker thread. The `gdbus-connection`
test was sharing a `FilterData` struct between the main thread and the
filter function, which was occasionally (on the order of 0.01% of test
runs) causing spurious test failures due to racing on reads/writes of
`num_handled`.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #480
g_assert() can be compiled out with G_DISABLE_ASSERT, which renders the
test rather useless.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #480
If a username and password are specified by the caller, `GSocks5Proxy`
tells the server that it supports anonymous *and* username/password
authentication, and the server can choose which it prefers.
Otherwise, `GSocks5Proxy` only says that it supports anonymous
authentication. If that’s not acceptable to the server, the code was
previously returning `G_IO_ERROR_PROXY_AUTH_FAILED`. That error code
doesn’t indicate to the caller that authentication might succeed were
they to provide a username and password.
Change the error handling to make that clearer. A fuller solution would
be to expose more of the method negotiation in the `GSocks5Proxy` API,
so that the caller can specify ahead of time which authentication
methods they want to use. That can follow in issue #2059 though.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #1988
