The gutils-user-database.c is broken in two ways and currently doesn't test anything:
* It only overrides getpwuid, where the implementation used getpwnam_r if it exist, which should be every system for at least 20 years.
* It only partly cargo-culted setting the environment for the local and installed tests, but failed to actually set the environment for either.
Before commit 71b7efd08a1feadc8ddca31e164034b1f5a6bd74, `GKeyFile`
incorrectly allowed invalid escape sequences: it would treat the
sequence as a literal, set a `GError`, but not return failure from the
function. So if a caller was explicitly checking for returned `GError`s,
they could detect the invalid escape; but if they were just checking the
function’s return value, they’d miss it.
This is not correct use of `GError`, and the [Desktop Entry
Spec](https://specifications.freedesktop.org/desktop-entry-spec/latest/ar01s04.html)
doesn’t allow for invalid escape sequences to be accepted. So it’s wrong
in both ways.
However, the commit above changed this behaviour without realising it,
quite close to the 2.78 stable release deadline. There are numerous key
files in the wild which use invalid escape sequences, and it’s too late
in the cycle to ‘break’ parsing of all of them.
So, for now, revert to the old behaviour for invalid escape sequences,
and give people another cycle to adapt to the changes. This will likely
mean they end up calling `g_key_file_get_value()` rather than
`g_key_file_get_string()`. See
https://gitlab.gnome.org/GNOME/glib/-/issues/3098 for tracking
re-enabling the error handling for invalid escape sequences.
Signed-off-by: Philip Withnall <philip@tecnocode.co.uk>
Fixes: #3095
See: #3098
pthread_exit() is also marked noreturn, and the function does not in
fact return.
For win32, I can't find a copy of the header file to check if
_endthreadex() is marked with __declspec(noreturn), but its
documentation does say that it never returns.
When parsing a line of "key3=foo\i\" in a keyfile,
g_key_file_parse_value_as_string currently first sets the error to
'contains invalid escape' and later to 'contains escape character at end
of line'.
This leaks the first GError and causes the following warning message:
Error set over the top of a previous GError or uninitialized memory.
This indicates a bug in someone's code. You must ensure an error is
NULL before it's set. The overwriting error message was: Key file
contains escape character at end of line
Fix this by returning when an error is detected. As we may have
collected data in pieces, we instead collect to a tmp_pieces GSList and
free it on error.
On 32 bit systems 'long' value will overflow in 2038 and become negative.
As it is used to index into letters array, and % operation preserves signs,
data corruption will then occur.
Signed-off-by: Alexander Kanavin <alex@linutronix.de>
The point of to be able to call localtime()/localtime_r() from another
place inside glib (without reimplementing the #ifdef).
- also handles failures from localtime_r(). It is documented that it
might fail, so detect the failure.
- in case of failures of localtime(), still initialize the GDate.
Previously, we may error out with a g_critical() assertion before.
However, now that failures from localtime_r() are also caught, I think
we should make an effort to initialize the GDate to something. It
either way it not supposed to happen.
Print the PID, the errno and the pidfd in case of an unexpected failure
in g_child_watch_dispatch().
This is always(?) caused by a bug in the user application. Also hint to
g_child_watch_source_new() documentation for possible causes.
Also use G_PID_FORMAT for printing GPid values.
Prompted by #3071, this clarifies that `g_test_trap_subprocess()` uses
`g_child_watch_source_new()` internally, so it will not work if any of
the preconditions for using that API are not met. In particular, if
`SIGCHLD` is ignored, things will break.
This documentation is not meant to be an API guarantee which constrains
the implementation of `g_test_trap_subprocess()` in future, just a tip
to people currently using the API.
Signed-off-by: Philip Withnall <philip@tecnocode.co.uk>
Helps: #3071
This is one step towards rectifying the mistake of using `FD_CLOEXEC` in
the first place. Eventually we may deprecate support for `FD_CLOEXEC`,
as the `O_*` flags better match the underlying `pipe()` API.
See discussion on
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3459#note_1779264
Signed-off-by: Philip Withnall <philip@tecnocode.co.uk>
Since 5c65437d "glib-unix: Add O_NONBLOCK support to g_unix_open_pipe()"
we have been using O_NONBLOCK unconditionally, so we might as well drop
the fallback here as well. This commit should be reverted if someone
reports a significant/supported platform that genuinely doesn't have
O_NONBLOCK.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Since 5c65437d "glib-unix: Add O_NONBLOCK support to g_unix_open_pipe()"
we have effectively been assuming that these two flags are
distinguishable. If that's an assumption we want to make, we should make
it a static assertion, so that GLib will fail to compile on platforms
where it isn't true.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Previous default used was 32KiB (the library default) which caused some
complex patterns to fail, see #2824. The memory will not be allocated
unless used.
There is no point to enable jit in g_regex_new, since JIT will be only
used when we do a first match, and at that point
enable_jit_with_match_options will be called again already and will
update the options set in g_regex_new. Instead just run it at first
match for the first time, to the same end result.
We might repeatedly get si_pid == 0 for a child that hasn't exited,
meaning we won't get a correct exit status. This seems to happen when
the glib application tracks a ptrace():ed child process; the correct
exit status of the process using e.g. a BPF program, where one can
observe that glib appears to get it wrong.
Fixes: #3071
While it's only called one time, this is something that can only happen
during the function call, so it's more correct to mark it as call scope,
so that bindings don't have to wait for the callback invocation to
cleanup the data.
As per commit 5d738ddc some callbacks are not nullable anymore even
though they should be.
This breaks the introspection as some arguments won't be considered
nullable anymore.
You can notice this in https://gitlab.gnome.org/3v1n0/gjs/-/pipelines/558839
where some some tests are running against stable g-i version and others
(the failing ones) against the devel one.
Currently, when adding new elements to GTree we blindly increment the node
counter, which is only of guint size (so 32-bit even on 64-bit Unix
platforms).
This is even more problematic because the only way to check whether
particular GTree is empty is to check whether its node count is zero.
This will obviously give wrong answer if this counter overflows.
Let's fix this by adding an appropriate check when adding a new node.
For the recently added g_tree_{insert,replace}_node () API we can simply
return NULL in such case.
However, the older g_tree_{insert,replace} () API doesn't have any ability
to return an error so for them we follow the example of
g_ptr_array_extend () and g_ptr_array_set_size () by calling g_error ()
when this happens.
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
g_tree_nnodes () is returning a signed integer type (gint), however the
tree node counter value type is really an unsigned integer (guint).
This means that the returned size will be negative if the container holds
more than G_MAXINT elements.
Add a note to this function that its return value can be cast back to
guint in order to support its full range of values.
This will also make sure that we take this into account in future Glib
versions.
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
Fix typo in gio/gappinfo (duplicated word)
Fix typo in gio/gtlsdatabase.c (duplicated word)
Fix typo in gio/gapplication.c (duplicated word, previous words rearranged to improve readability)
Fix typo in glib/tests/gvariant.c (duplicated word)
Fix typo in glib/win_iconv.c (duplicated word)
Fix typo in gio/gschema.dtd (meaning, missing letter)
Fix typo in gio/gdbusintrospection.c (duplicated word)
Fix typo in gio/gdbusintrospection.c (duplicated word).
As commit 44616ebafdca ('gmain: More explicitly document
g_main_context_release() prereqs') correctly notes, you need to have the
context acquired before releasing it (just like a ref must match an
unref).
Commit 3926af723a74 ('gmain: Add precondition assertions to
g_main_context_release()') then goes one step further, and requires that
the calling thread is also the owner (the thread, that acquired the
context).
This is something which has been documented by g_main_context_release()
for years:
> Releases ownership of a context previously acquired **by this thread**
With acquire/release and g_main_context_is_owner() we track the thread
that acquired the context. That is mainly useful for asserting
correctness to not accessing the context from an unexpected thread.
Note that g_main_context_acquire() returns FALSE and does nothing when
the context is already acquired from another thread. Methods like
g_main_context_{prepare,query,dispatch}() require that the calling
thread is the owner (although, they don't assert for that, which they
maybe should).
With the assertion, it means you cannot pass an acquired context to
another thread for release. Obviously, if you pass on an acquired
context to another thread, the only next thing you can do is
g_main_context_release() (no acquire,prepare,query,dispatch). But it's
still useful to be able to release it, and to be able to keep it
acquired for a prolonged time.
libnm needs that, as it integrates a GMainContext into another
GMainContext. For that, it needs to acquire the inner context and keep
it acquired for as long as the context is integrated. Otherwise, when a
source gets attached to the inner context, the inner context is not
woken up (see g_source_attach_unlocked()). In commit e26a8a59813c ('Add
G_MAIN_CONTEXT_FLAGS_OWNERLESS_POLLING'), a flag was introduced to solve
that same problem, without keeping the inner context acquired all the
time. Note that G_MAIN_CONTEXT_FLAGS_OWNERLESS_POLLING is a flag of the
GMainContext, so it only works if the user who integrates the inner
context also controls how the context was created. For libnm, having
the inner context acquired at all times is no problem, because it's
understood that the user gives up agency on the inner context while it's
integrated. The only thing to consider is that the outer context might
be iterated on another thread. When calling prepare,query,dispatch on
the inner context, the code will notice it, release the inner context
and re-acquire it on the new thread ([1]). This works just fine, but it
requires that g_main_context_release() works from any thread.
So, in order to not break NetworkManager, let’s drop the ownership
assertion. However, NetworkManager is strictly breaking the API contract
here, and GLib reserves the right to re-add this assertion in future.
Still keep the assertion for the owner_count.
(Commit and commit message significantly updated by Philip Withnall; all
errors are his.)
[1] 9f01cff04f/src/libnm-glib-aux/nm-shared-utils.c (L4944)
Related: 3926af723a74 ('gmain: Add precondition assertions to g_main_context_release()')
Related: c67dd9d3fe91 ('gmain: Add a missing return on error path in g_main_context_release()')
Closes#3054
This reverts commit 252bbcd2078aadc67a49bacd5b2cd4fd348a8dd7.
After further discussion in !3511, we’ve decided that there are risks
associated with this change, and it’s not the best way of addressing the
original problem.
The original motivation for the change turned out to be that
`-mms-bitfields` was not handled by `windres`, which was receiving it
from `pkg-config --cflags glib-2.0` in some projects. However, if
`windres` is claiming to accept CFLAGS then it should accept (and
ignore) `-mms-bitfields`, since the `-m` family of options are defined
in `man gcc`, just like `-I`, `-D`, etc.
There is some question that there might still be third party projects
which are built with an old enough compiler that `-mms-bitfields` is not
the compiler default. For that reason, we should either still continue
to specify `-mms-bitfields` in the `.pc` file, or add a test to assert
that third party projects are always compiled with `-mms-bitfields` set.
But adding a new test for all third-party compilations is risky (if we
get it wrong, things will break; and it’s a test which may behave
differently on different platforms), so it seems safer to just keep
`-mms-bitfields` in `.pc` for now.
Once all compilers which we require specify `-mms-bitfields` by default,
we can finally drop this flag (without adding a test for third-party
compilations).
See: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3511
The old name was not quite correct: the part of a URI which is just past
the scheme might be the hostname or the path. It isn’t necessarily just
a path.
This introduces no functional changes.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Otherwise, crashing tests like assert-msg-test will still report to
pipe-based crash reporting frameworks like systemd-coredump, even though
the RLIMIT_CORE limit is zero.
Signed-off-by: Simon McVittie <smcv@collabora.com>
