Usually, after g_pointer_bit_lock() we want to read the pointer that we
have. In many cases, when we g_pointer_bit_lock() a pointer, we can
access it afterwards without atomic, as nobody is going to modify the
pointer then.
However, gdataset also supports g_datalist_set_flags(), so the pointer
may change at any time and we must always use atomics to read it. For
that reason, g_datalist_lock_and_get() does an atomic read right after
g_pointer_bit_lock().
g_pointer_bit_lock() can easily access the value that it just set. Add
g_pointer_bit_lock_and_get() which can return the value that gets set
afterwards.
Aside from saving the second atomic-get in certain scenarios, the
returned value is also atomically the one that we just set.
In all cases after taking the lock with g_datalist_lock(), we also
get the pointer. And it hardly makes sense otherwise.
Replace g_datalist_lock() by g_datalist_lock_and_get() which does
both steps in one.
- only use gnewa0() for up to 400 bytes (arbitrarily chosen as
something that is probably small enough to cover most small tasks
while fitting easily on the stack). Otherwise fallback to g_new0().
- don't do intermediate G_DATALIST_SET_POINTER(). Set to NULL
afterwards with g_datalist_unlock_and_set().
- move the g_free(d) after releasing the lock on datalist.
- when setting datalist to NULL, do it at the end with
g_datalist_unlock_and_set() to avoid the additional atomic
operations.
- when freeing the old "d", do that after releasing the bit
lock.
The previous code was in practice correct.
(((guintptr) ptr) | ((gsize) mask))
works even if gsize is smaller than guintptr. And while the C standard
only guarantees that cast between uintptr_t and void* works, on
reasonable compilers it works to directly cast uintptr_t to GData*.
Still, no need for such uncertainty. Just be clear about the involved
types (use guintptr throughout) and cast first to (gpointer).
g_datalist_clear_i() had only one caller, altough the code comment made
it sound as if in the past there were more.
A function that has only one caller, and then needs a code comment to
explain the context in which it is called, makes the code more complicated
than necessary.
Especially since the function expects to be called with a global lock
held, and then unlocks and relocks. Spreading such things to another
function (which is only used once) makes code harder to follow.
Inline the function, so that we can see all the (trivial) code at one place.
This saves an additional atomic set.
Also, changing the pointer before the unlock can awake a blocked thread,
but at this point, the lock is still held (and the thread may need to
sleep again). This can be avoided.
The existing g_pointer_bit_lock() and g_pointer_bit_unlock() API
requires the user to understand/reimplement how bits of the pointer get
mangled. Add helper functions for that.
The useful thing to do with g_pointer_bit_lock() API is to get/set
pointers while having it locked. For example, to set the pointer a user
can do:
g_pointer_bit_lock (&lockptr, lock_bit);
ptr2 = set_bit_pointer_as_if_locked(ptr, lock_bit);
g_atomic_pointer_set (&lockptr, ptr2);
g_pointer_bit_unlock (&lockptr, lock_bit);
That has several problems:
- it requires one extra atomic operations (3 instead of 2, in the
non-contended case).
- the first g_atomic_pointer_set() already wakes blocked threads,
which find themselves still being locked and needs to go back to
sleep.
- the user needs to re-implement how bit-locking mangles the pointer so
that it looks as if it were locked.
- while the user tries to re-implement what glib does to mangle the
pointer for bitlocking, there is no immediate guarantee that they get
it right.
Now we can do instead:
g_pointer_bit_lock(&lockptr, lock_bit);
g_pointer_bit_unlock_and_set(&lockptr, lock_bit, ptr, 0);
This will also emit a critical if @ptr has the locked bit set.
g_pointer_bit_lock() really only works with pointers that have a certain
alignment, and the lowest bits unset. Otherwise, there is no space to
encode both the locking and all pointer values. The new assertion helps
to catch such bugs.
Also, g_pointer_bit_lock_mask_ptr() is here, so we can do:
g_pointer_bit_lock(&lockptr, lock_bit);
/* set a pointer separately, when g_pointer_bit_unlock_and_set() is unsuitable. */
g_atomic_pointer_set(&lockptr, g_pointer_bit_lock_mask_ptr(ptr, lock_bit, TRUE, 0, NULL));
...
g_pointer_bit_unlock(&lockptr, lock_bit);
and:
g_pointer_bit_lock(&lockptr, lock_bit);
/* read the real pointer after getting the lock. */
ptr = g_pointer_bit_lock_mask_ptr(lockptr, lock_bit, FALSE, 0, NULL));
...
g_pointer_bit_unlock(&lockptr, lock_bit);
While glibc is fine with it (and returns a `NULL` pointer), technically
it’s implementation-defined behaviour according to POSIX, so it’s best
avoided.
See
https://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_memalign.html.
In particular, valgrind will warn about it, which is causing failures of
the gdbus-codegen tests when valgrind is enabled. For example,
https://gitlab.gnome.org/GNOME/glib/-/jobs/3460673 gives
```
==15276== posix_memalign() invalid size value: 0
==15276== at 0x484B7BC: posix_memalign (vg_replace_malloc.c:2099)
==15276== by 0x49320B2: g_variant_new_from_bytes (gvariant-core.c:629)
==15276== by 0x4931853: g_variant_new_from_data (gvariant.c:6226)
==15276== by 0x4B9A951: g_dbus_gvalue_to_gvariant (gdbusutils.c:708)
==15276== by 0x41BD15: _foo_igen_bat_skeleton_handle_get_property (gdbus-test-codegen-generated.c:13503)
==15276== by 0x41BFAF: foo_igen_bat_skeleton_dbus_interface_get_properties (gdbus-test-codegen-generated.c:13582)
…
```
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3228
This doesn't appear to work reliably on s390x and ppc64, returning
the results that were expected without %E or intermittently crashing.
It seems that on little-endian 64-bit, the intptr_t returned by the
undocumented _NL_TIME_ERA_NUM_ENTRIES correctly has the number of entries
in its low-order half (at the time of writing, 0x0000'0000'0000'000b for
Japan), but on big-endian 64-bit, it has the number of entries in its
high-order half instead (for example 0x0000'000b'0000'0000 for Japan),
with the low-order half being all-zero or possibly uninitialized.
Making this reliable will require some sort of defined API from glibc.
Helps: https://gitlab.gnome.org/GNOME/glib/-/issues/3225
Bug-Debian: https://bugs.debian.org/1060735
Signed-off-by: Simon McVittie <smcv@collabora.com>
We only care about the introspection data for documentation purposes;
the GStaticMutex and GStaticRecMutex types are long since deprecated,
and they cannot be used from language bindings anyway, so their size is
immaterial.
Fixes: #3222
Previously, the girs and typelibs generated from gobject-introspection
ommited the deprecated apis, however we want them annotated for
documentation purposes.
Skip the deprecated gthead api so they do not make it into the
typelibs which caused problems as not all the symbols exist.
See https://gitlab.gnome.org/GNOME/gjs/-/issues/595
Introduce g_log_writer_syslog() that is suitable for use as a
GLogWriterFunc and sends the log message to the syslog daemon.
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
_Thread_local is also C11, so possibly other compilers would also support
it.
However, since not *all* compilers support it, it can anyway only be
used as optimization and conditional asserts. As such, the current
detection based on __GNUC__ to only support gcc (and clang) is good
enough.
This was done manually, but the changes are very repetitive. There are
some minor rewordings/formatting tweaks included. The bulk of the
changes are to the linking syntax.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3037
There is no reason for key and value to have different annotations.
Both may return NULL as a valid value.
gpointer typed parameters are nullable by convention, so there is
no change here. The (nullable) annotation is just for humans really.
To make it match its definition and documentation comment. This fixes a
g-ir-scanner warning.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3037
Extended error domains can never be unregistered, so these closures
have to be around forever.
Fixes some g-ir-scanner warnings.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3037
`GByteArray` is a bit odd in that it allows call-chaining. All the
instances of that were missing a `(transfer none)` annotation though.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3037
This fixes a warning from g-ir-scanner as the declaration and
documentation comment didn’t match.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3037
