Now all accesses to quark_notify_queue are guarded by the GData lock.
Several non-trivial operations are implemented via
g_datalist_id_update_atomic().
The OPTIONAL_BIT_LOCK_NOTIFY lock is thus unnecessary and can be dropped.
Note that with the move to g_datalist_id_update_atomic(), we now
potentially do more work while holding the GData lock (e.g. some code
paths allocation additional memory). But note that
g_datalist_id_set_data() already has code paths where it must allocate
memory to track the GDataElt. Also, most objects are not used in
parallel, so holding the per-object (per-GData) lock longer does not
affect them. Also, many operations also require a object_bit_lock(), so
it's unlikely that you really could achieve higher parallelism by taking
more locks (and minimizing the time to hold the GData lock).
A common pattern is to look whether a GData entry exists, and if it
doesn't, add it.
For that, we currently always must take a OPTIONAL_BIT_LOCK_NOTIFY lock.
This can be avoided, because GData already uses an internal mutex. By
using g_datalist_id_update_atomic(), we can perform all relevant
operations while holding that mutex.
Move functionality from g_object_notify_queue_freeze() inside
g_datalist_id_update_atomic().
The goal will be to drop the OPTIONAL_BIT_LOCK_NOTIFY lock in a later
commit.
Cache the function pointer for g_datalist_id_update_atomic() in a static
variable to avoid looking it up repeatedly.
g_datalist_id_update_atomic() is anyway internal API. And just like
GData is not a very useful data structure in general, it's mainly used
by GObject. And since GObject is the at the code, optimizing this makes
sense.
We can easily see that we never call g_datalist_id_update_atomic()
without having a GObject at hand (because we call it on
`&object->qdata`. Thus initializing the pointer in
g_object_do_class_init() (under lock) works perfectly fine. And if this
invariant ever changes, you will get a crash immediately and notice the
problem.
None of the users actually care about this parameter. And it's unlikely
that they ever will. Also, the passed "key_id" is the argument from
g_datalist_id_update_atomic(). If the caller really cared to know the
"key_id" in the callback, they could pass it as additional user data.
We were reusing the same logic everywhere, while we can just reuse an
unique class to base our tests on that avoids having to copy-and-paste
code for no good reason
Add some basic support for having glib-tests-only python libraries that
can be shared across the various projects, so that we don't have to
maintain multiple copies of them.
Move the shebang line from the full Python path of the build
machine to the first Python on the path during runtime. Set
the shebang in the main meson.build file so that it can be
overridden in one place if needed.
Fixes: #3331
We don't allow unloading types, both static and dynamic, since 2013. The
code that deals with reference counting is mostly dead code, and makes
reasoning about the type system more complicated than necessary.
Since type classes and interfaces can only be instantiated, we introduce
explicit getter functions that create a GTypeClass or a GTypeInterface
vtable; the ref() and unref() API gets a "soft" deprecation (explicitly
not using `GOBJECT_DEPRECATED_IN_*` yet), to allow people to
progressively port their code.
`@filename@` expands to the (absolute or relative) path from the
build directory to the source directory, which can be rather verbose.
In practice Meson usually (always?) generates a relative path, but
even so, the resulting installed header is not necessarily reproducible
if using different build directories outside the source directory.
We don't really need a full path here anyway: the basename is enough
of a hint to point a reader towards the file where the underlying
enum was defined.
Signed-off-by: Simon McVittie <smcv@collabora.com>
The docs for the constructed vfunc make it clear that when called
constructed properties are already set. However, the first place where a
user would look for is the flag's documentation.
It’s a false positive, but points to a slightly unnecessary use of a
global variable to store something which could be computed per-test.
scan-build thought that arrays of size `n_handlers` could have
uninitialised values accessed in them if `n_handlers` were to change
value part-way through a test (which it didn’t).
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
It’s deprecated, but I was modifying it anyway and it didn’t have any
coverage, so let’s add a simple test (as suggested by Michael
Catanzaro).
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
The latter only accepts a `gint` as the number of elements in the array,
which means that its use in `GArray` (and related array implementations)
truncates at least half the potential array size.
So, introduce a replacement for it which uses `size_t` for the number of
elements. This is inline with what `qsort()` (or `qsort_r()`) actually
does. Unfortunately we can’t directly use `qsort_r()` because it’s not
guaranteed to be a stable sort.
This fixes some `-Wsign-conversion` warnings (when building GLib with
that enabled).
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
If we enable `-Wfloat-conversion`, these warn about a possible loss of
precision due to an implicit conversion from `double` to some other
numeric type.
The warning is correct: there is a possible loss of precision here. In
these instances, we don’t care, as the floating point arithmetic is
being done to do some imprecise scaling or imprecise timing. A loss of
precision is not a problem.
So, add an explicit cast to squash the warning.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
The tests were using a lot of signed `int`s when actually the values
being handled were always non-negative. Use `unsigned int` consistently
throughout.
Take the opportunity to move declarations of loop iterator variables
into the loops.
This introduces no functional changes.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
There were various places where (signed or unsigned) integer assertions
were being used to compare `double` or `float` values, resulting in an
implicit integer conversion.
This causes a warning when building with `-Wfloat-conversion`.
Improve the specificity of the tests by using the most-specific numeric
assertions through all `param` tests.
For the conversion tests, this means using the assertion function
associated with the target type, not the source type.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
Compiling with `-Wfloat-conversion` warns about a few implicit
conversions from `double`/`float` to other numeric types in the `GValue`
transform functions.
These warnings are correct: value transformations can result in loss of
precision. That loss of precision is understood and expected, so add
some explicit casts to squash the warnings.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
Rather than defining it as a double constant. This introduces no
functional changes, but does squash some `-Wfloat-conversion` warnings.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
As spotted by `-Wfloat-conversion`. Doubles which could not be
accurately represented as floats may have erroneously failed bounds
validation.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
The default values for construct properties always have to be set, even
if those properties are deprecated. The code to do that is in GLib, and
not under the control of the user (unless they completely override the
`constructor` vfunc, which is not recommended). So don’t emit a warning
for that if `G_ENABLE_DIAGNOSTICS` is enabled.
In particular, this fixes deprecation warnings being emitted for
properties of a parent class when chaining up with a custom constructor,
even when none of the child class code mentions the deprecated property.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3254
This just makes it a bit clearer that they’re atomic/for thread safety,
and not just NIHed bit operations with shouty names.
This introduces no functional changes.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
This avoids the need to ref/unref the closure while invalidating it in
the `closure->ref_count == 1` path in `g_closure_unref()`.
scan-build gets very confused about the ref count here, and ends up
assuming it’s possible for the `g_closure_unref()` call in
`g_closure_invalidate()` to finalise the closure when the latter is
called from `g_closure_unref()`. There was an existing assertion in
`g_closure_invalidate()` which hinted that this wasn’t possible, but
scan-build doesn’t seem to be able to propagate assumptions about
refcounts between function contexts.
So, introduce an internal variant of `g_closure_invalidate()` which can
skip modifying the closure’s refcount. It’s safe to invalidate the
closure without adding a ref when doing so from `g_closure_unref()` with
`closure->ref_count == 1` because at that point `g_closure_unref()`
holds the only remaining ref to the closure. So none of the invalidation
callbacks are allowed to unref it further.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build is worried that `node->data->common.value_table->value_init`
will be a `NULL` pointer dereference in the assignment to
`node->mutatable_check_cache`.
There’s already an assertion immediately below to check against this, so
let’s move it up a line to help the static analyser out.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
Avoid scan-build thinking that `new_wrdata` could be `NULL` on this
control path. It can’t be `NULL` if `new_object` is set.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
Basically various trivial instances of the following MSVC compiler
warning:
```
../gio/gio-tool-set.c(50): warning C4267: '=': conversion from 'size_t' to 'int', possible loss of data
```
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
I’m not sure exactly how this code is supposed to work, so this might
not be the right fix. But there’s definitely a problem here, and it was
spotted by scan-build.
If `param_value_array_validate()` is entered with
`value->data[0].v_pointer == NULL && aspec->fixed_n_elements`, that `NULL`
will be stored in `value_array` too. `value->data[0].v_pointer` will
then be set to a new non-`NULL` array.
A few lines down, `value_array_ensure_size()` is called on
`value_array` – which is still `NULL` – and this results in a `NULL`
pointer dereference.
It looks like `value->data[0].v_pointer` and `value_array` are used
interchangeably throughout the whole of the function, so assign the new
value of `value->data[0].v_pointer` to `value_array` too.
My guess is that `value_array` is just a convenience alias for
`value->data[0].v_pointer`, because the latter is a real mouthful to
type or read.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
The python interpreter found by `/usr/bin/env python3` is not
necessarily the same installation as the one that's found by meson's
`pymod.find_installation('python')`. This means that even though meson
is checking that the python installation it found includes the
'packaging' module, the scripts might not have access to that module
when run.
For distribution packaging, it's usually desirable to have python script
interpreters be fully specified paths, rather than use `/usr/bin/env`,
to ensure the scripts run using the expected python installation (i.e.
the one where the python 'packaging' dependency is installed).
The easiest way to fix this is to set the script interpreter to the
`full_path()` of the python interpreter found by meson. The specific
python interpreter that will be used can be selected through the use of
a meson machine file by overriding the "python" program. Many
distributions already have this set up using meson packaging helpers.
These consistently fail on scheduled CI runs, which is not helping our
ability to catch Hurd regressions.
For example, https://gitlab.gnome.org/GNOME/glib/-/jobs/3709402
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
See: #3148
