gobject/gobject.c: In function ‘g_object_new_internal’:
gobject/gobject.c:1962:25: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
1962 | for (j = 0; j < n_params; j++)
| ^
gobject/gobject.c:1989:21: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
1989 | for (i = 0; i < n_params; i++)
| ^
gobject/gobject.c: In function ‘g_object_new_with_custom_constructor’:
gobject/gobject.c:1836:21: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
1836 | for (j = 0; j < n_params; j++)
| ^
gobject/gobject.c:1914:17: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
1914 | for (i = 0; i < n_params; i++)
| ^
gobject/gobject.c: In function ‘g_object_class_install_properties’:
gobject/gobject.c:766:17: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
766 | for (i = 1; i < n_pspecs; i++)
| ^
gobject/gtype.c: In function ‘g_type_interface_add_prerequisite’:
gobject/gtype.c:1607:21: error: comparison of integer expressions of different signedness: ‘guint’ {aka ‘unsigned int’} and ‘int’
1607 | for (i = 0; i < prerequisite_node->n_supers + 1; i++)
| ^
The problem occurs because we keep a pointer inside the allocated block,
instead of a pointer to the start of the block. This memory exists for
the lifetime of the application, so let's silence it.
This is probably abuse of VALGRIND_MALLOCLIKE_BLOCK(), which is really
intended for use in memory allocators, but gtype.c already uses it in
two other places, and it's a practical solution. I wrote another larger
fix for this issue that involves keeping an array of extra pointers when
running under valgrind. This is simpler.
Fix suggested by Philip Withnall
```
==180238== 16 bytes in 1 blocks are possibly lost in loss record 3,078 of 16,075
==180238== at 0x483BB1A: calloc (vg_replace_malloc.c:762)
==180238== by 0x5489495: g_malloc0 (gmem.c:132)
==180238== by 0x5489754: g_malloc0_n (gmem.c:364)
==180238== by 0x53FDBEE: type_set_qdata_W (gtype.c:3722)
==180238== by 0x53FDEE8: type_add_flags_W (gtype.c:3787)
==180238== by 0x53FC348: g_type_register_fundamental (gtype.c:2662)
==180238== by 0x53D969B: _g_enum_types_init (genums.c:124)
==180238== by 0x53FF058: gobject_init (gtype.c:4432)
==180238== by 0x53FF082: gobject_init_ctor (gtype.c:4493)
==180238== by 0x4010F29: call_init.part.0 (dl-init.c:72)
==180238== by 0x4011030: call_init (dl-init.c:30)
==180238== by 0x4011030: _dl_init (dl-init.c:119)
==180238== by 0x4002149: ??? (in /usr/lib64/ld-2.30.so)
```
Fixes#2076
The problem occurs because we keep a pointer inside the allocated block,
instead of a pointer to the start of the block:
```
==180238== 16 bytes in 1 blocks are possibly lost in loss record 3,086 of 16,075
==180238== at 0x483980B: malloc (vg_replace_malloc.c:309)
==180238== by 0x548942C: g_malloc (gmem.c:102)
==180238== by 0x54A4748: g_slice_alloc (gslice.c:1025)
==180238== by 0x53D0AAF: freelist_alloc (gatomicarray.c:77)
==180238== by 0x53D0B85: _g_atomic_array_copy (gatomicarray.c:133)
==180238== by 0x53F8E6D: iface_node_set_offset_L (gtype.c:1347)
==180238== by 0x53F91F1: type_node_add_iface_entry_W (gtype.c:1444)
==180238== by 0x53F93DF: type_add_interface_Wm (gtype.c:1477)
==180238== by 0x53FC946: g_type_add_interface_static (gtype.c:2852)
==180238== by 0x4A3D53A: gtk_menu_shell_accessible_get_type_once (gtkmenushellaccessible.c:26)
==180238== by 0x4A3D495: gtk_menu_shell_accessible_get_type (gtkmenushellaccessible.c:26)
==180238== by 0x4C8AC44: gtk_menu_shell_class_init (gtkmenushell.c:424)
```
Note we cannot use VALGRIND_FREELIKE_BLOCK() in freelist_free() because we
have not actually freed the FreeListNode and need to dereference it in
freelist_alloc() to decide whether to reuse the block. That would result
in a use-after-free warning before we would get a chance to call
VALGRIND_MALLOCLIKE_BLOCK() in the reuse path.
Also note that this free list only ever grows: it never shrinks for the
lifetime of the application, so nothing here will ever be truely freed,
although unused elements are eligible for reuse.
Fix suggested by Philip Withnall
Related: #2076
`g_time_zone_new_identifier()` returns NULL in the FreeBSD test setup,
presumably because `TZ` isn’t set.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #553
gobject/gtype.c: In function ‘type_node_add_iface_entry_W’:
gobject/gtype.c:1379:21: error: comparison of integer expressions of different signedness: ‘guint’ {aka ‘unsigned int’} and ‘int’
1379 | for (i = 0; i < num_entries; i++)
| ^
gobject/gtype.c: In function ‘lookup_iface_entry_I’:
gobject/gtype.c:599:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’
599 | if (index < IFACE_ENTRIES_N_ENTRIES (entries))
| ^
gobject/gatomicarray.h:51:8: note: in definition of macro ‘G_ATOMIC_ARRAY_DO_TRANSACTION’
51 | {_C_;} \
| ^~~
Use `g_time_zone_new_identifier()` instead so you can get error
checking.
Adapt the tests to match.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #553
It doesn’t seem to be possible to use `only:changes` from a branch
pipeline extending `.only-default` and have it correctly trigger when
`*.sh` or `*.py` files are modified.
We possibly need to convert all our pipelines to be merge-request-only,
but that’s an avenue I’ve been down before and I couldn’t get it to
work. Using
[`workflow:rules`](https://docs.gitlab.com/ee/ci/yaml/README.html#workflowrules)
might fix that, but I don’t have time to rework the entire CI to use
that now.
So in the meantime, move the SH and Py checks into the existing
style-check job so we’re not always spinning up two additional container
instances on every merge request.
See !1743.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Spotted by shellcheck (warning SC1117): `\e` is not an actual escape
sequence, so it’s interpreted as `\\e`. Best make that explicit.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Half of the references to `init_state` in `gtype.c` already correctly
accessed it atomically, but a couple didn’t. Drop the `volatile`
qualifier from its declaration, as that’s not necessary for atomic
access.
Note that this is the `init_state` in `TypeData`, *not* the `init_state`
in `IFaceEntry`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
In the Windows destructor list, consistently access
`g_private_destructors` using atomic primitives.
`g_atomic_pointer_compare_and_exchange()` should be equivalent to
`InterlockedCompareExchangePointer()`, but is a bit more understandable
in a general GLib context, and pairs with `g_atomic_pointer_get()`. (I
can’t find a Windows API equivalent for that.)
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
`volatile` should not be used to indicate atomic variables, and we
shouldn’t encourage its use. Keep the tests, since they check that we
don’t emit warnings when built against incorrect old code which uses
`volatile`. But add a comment to stop copy/paste use of `volatile`
in the future.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
These variables were already (correctly) accessed atomically. The
`volatile` qualifier doesn’t help with that.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
This is an API break, but no third party code should be touching
`GObject.ref_count`, let alone in a way which would be changed by the
removal of the `volatile` qualifier.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
This is an API break, but it should not affect third party code since
that code should not be interacting with the `data` member in a way that
invokes its `volatile` qualifier (such as copying to an intermediate
variable).
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
This is technically an API break, but since the type is meant to be
opaque (third party code is not meant to treat it like an integer) it
should not cause problems.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
These variables were already (correctly) accessed atomically. The
`volatile` qualifier doesn’t help with that.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
These variables were already (correctly) accessed atomically. The
`volatile` qualifier doesn’t help with that.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
It’s not necessary and provides no thread safety guarantees.
The `volatile` qualifiers on the function arguments have to be kept, as
they are (unfortunately) part of the API.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
