GHashTable optimizes for the "set" case, where key and value are the same.
See g_hash_table_add().
A user cannot see from outside, whether a GHashTable internally is a set
and shares the keys and values array. Adding one key/value pair with
differing key and value, will expand the GHashTable.
In all other cases, the GHashTable API hides this implementation detail
correctly. Except with g_hash_table_steal_extended(), when stealing both the
key and the value.
Fix that. This bug fix is obviously a change in behavior. In practice,
it's unlikely that somebody would notice, because GHashTable contains
opaque pointers and the user must know what the keys/values are and
be aware of their ownership semantics when stealing them. That means,
the change in behavior only affects instances that are internally a set,
of what the user most likely is aware and fills the table with
g_hash_table_add(). Such a user would not steal both the key and
values at the same time. Even if they do, then previously stealing the
value was pointless and would not give them what they wanted. It would
not have meaningfully worked, and since nobody reported a bug about this
yet, it's unlikely somebody noticed.
The more problematic case when the user exhibits the bug is when the
dictionary is unexpected a set internally. Imagine a mapping from numbers
to numbers (e.g. a permutation). If "unexpectedly" the dictionary contains
the identity permutation, steal-extended gives always NULL for the target
number.
The example is far fetched. In practice, it's unlikely that somebody is
gonna notice either way. That is not an argument for fixing anything.
The argument for fixing this, is that the bug breaks the illusion that
the set is only an internal optimization. That is ugly and inconsistent.
I answered a question on irc about withdrawing notifications, and
when I handed out a link to the GApplication docs, I noticed we
don't have this function name linkified. Fix that.
This fixes the annoations for g_mapped_file_get_contents() which looks
like it might transfer ownership (due to being a char*) but does not as
we're pointing into the mmap() region.
Now that we've switched to `gi-docgen`, let's make sure our docs are
getting updated. This commit fixes most of the previous gtk-doc
references so that they now follow gi-docgen syntax.
Some exceptions are functions or types that are referenced, but are
generated by a higher level layer like `Gio`, `GObject` or `Gtk`.
Some callers of `g_ascii_strtoull()` and similar functions assume that
they can use this pattern, similar to what they might do for
Standard C `strtoull()`:
errno = 0;
result = g_ascii_strtoull (nptr, endptr, base);
saved_errno = errno;
if (saved_errno != 0)
g_printerr ("error parsing %s\n", nptr);
This is based on the fact that it is non-trivial to tell whether
`strtoull()` and related functions succeeded (in which case the value
of `errno` is unspecified) or failed (in which case `errno` is valid).
For example, POSIX `strtoul(3)` suggests this pattern:
> Since 0, `ULONG_MAX`, and `ULLONG_MAX` are returned on error and are
> also valid returns on success, an application wishing to check for
> error situations should set `errno` to 0, then call `strtoul()` or
> `strtoull()`, then check `errno`.
However, `g_ascii_strtoull()` does not *only* call a function resembling
`strtoull()` (`strtoull_l()` or its reimplementation
`g_parse_long_long()`): it also calls `get_C_locale()`, which wraps
`newlocale()`. Even if `newlocale()` succeeds (which in practice we
expect and assume that it will), it is valid for it to clobber `errno`.
For example, it might attempt to open a file that only conditionally
exists, which would leave `errno` set to `ENOENT`.
This is difficult to reproduce in practice: I encountered what I
believe to be this bug when compiling GLib-based software for i386 in a
Debian 12 derivative via an Open Build Service instance, but I could
not reproduce the bug in a similar chroot environment locally, and I
also could not reproduce the bug when compiling for x86_64 or for a
Debian 10, 11 or 13 derivative on the same Open Build Service instance.
It also cannot be reproduced via the GTest framework, because
`g_test_init()` indirectly calls `g_ascii_strtoull()`, resulting in
the call to `newlocale()` already having happened by the time we enter
test code.
Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/3418
Signed-off-by: Simon McVittie <smcv@collabora.com>
This file doesn’t contain any real implementation, it just call the
`impl` functions from the platform-specific files
`gcontenttype-{fdo,osx,win32}.[cm]`.
It serves as a location for the doc comments, introspection annotations
and API preconditions, and will be built on every platform. In
particular, this means that we get consistent GIR output for the
`g_content_type_*()` APIs regardless of whether GLib was built on Linux or
Windows or macOS.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3399
This reflects its status as actually platform-dependent: it’s only built
on systems using the freedesktop.org content type system.
It makes the file naming match up with other platform-specific
implementations, such as `gcontenttype-win32.c` and
`gcontenttype-osx.m`.
A subsequent commit will introduce a platform-independent high level API
wrapper so that the introspection annotations from this file can be
reused between platforms.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3399
Previously, some of the doc comments for platform-independent APIs were
in `gdesktopappinfo.c`, which is only built on Unix systems. This meant
the introspection annotations for those APIs were not used on non-Unix
systems, which caused platform differences in `Gio-2.0.gir`.
So, move those doc comments to `gappinfo.c` and put them next to some
new platform-independent wrapper functions which provide a consistent
entry point and location for the API preconditions.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3399
Move various doc/introspection comments from `gthread-posix.c` (which is
platform-specific) to `gthread.c` (which is not). Having the
introspection annotations and doc comments in a platform-independent
file means that they are seen by the build process on all platforms, and
we don’t end up with unintrospectable APIs on some platforms, or
platform-specific annotation differences.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3399
On g_cancellable_cancel() we were increasing the GCancellable ref count
before emitting the ::cancelled signal, this is a safe thing to do but
it was happening while the cancellable was locked, and this may have
potentially waken up some toggle notifications.
To prevent this, reference the GCancellable just before locking.
GCancellable is meant to be used in multi-thread operations but all the
cancellable instances were sharing a single mutex to synchronize them
which can be less optimal when many instances are in place.
Especially when we're doing a lock/unlock dances that may leave another
thread to take the control of a critical section in an unexpected way.
This in fact was leading to some races in GCancellableSources causing
leaks because we were assuming that the "cancelled" callback was always
called before our dispose implementation.
As per this, use per-instance mutexes.
The lock is also now used only to protect the calls that may interact
with cancelled state or that depends on that, as per this we can just
reduce it to the cancel and reset case, other than to the connect one to
prevent the race that we could have when connecting to a cancellable
that is reset from another thread.
We don't really need to release the locks during callbacks now as they
are per instance, and there's really no function that we allowed to call
during a ::cancelled signal callback that may require an unlocked state.
This could been done in case with a recursive lock, that is easy enough
to implement but not really needed for this case.
Fixes: #2309, #2313
These rules are not new, they’ve been around for a long time and are
needed to allow introspection machinery to be able to work reliably and
deterministically.
Unfortunately, they have not been documented canonically in one place
before.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Using G_STATIC_ASSERT in headers which are introspected currently
requires guarding them behind `#ifndef __GI_SCANNER__` which is really
annoying. We can just define the macros to be noops in a way that the
scanner doesn't trip over them.
Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
They are guarded for the GI Scanner right now even though they should be
fine to expose and they are used in macros that are not guarded for the
GI Scanner.
Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
