These only show up on macOS. Apparently it’s more sensitive to assigning
`gboolean` (which is secretly `int`) to a `guint` bitfield. 🤷
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
These don’t show up for me on Linux, but are now causing CI failures on
macOS (https://gitlab.gnome.org/GNOME/glib/-/jobs/5006543):
```
../gobject/gclosure.c:923:40: error: implicit conversion changes signedness: 'gboolean' (aka 'int') to 'guint' (aka 'unsigned int') [-Werror,-Wsign-conversion]
ATOMIC_SET (closure, in_marshal, in_marshal);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
```
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
This makes the code a little clearer. In most cases, it’s not a
functional change.
In a few cases, the values are different. I believe the original values
were incorrect (accidentally transposed, perhaps). This never caused an
issue because they were all immediately overwritten during construction
of a `GParamSpec`: these values were defaults in the `instance_init`
vfunc of the `GTypeInstance` for a `GParamSpec`, but the actual min/max
for the `GParamSpec` instance were immediately written over them in the
constructor (such as `g_param_spec_int()`).
Spotted in !4593.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Notably without the (array), the argument of g_byte_array_append appears
as
<type name="guint8" c:type="const guint8*"/>
which will cause the bindings to pass a byte, not a pointer.
Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
The "without first having or creating a strong reference" part is wrong.
While we invoke the dispose() method, we always still hold the last
reference. The calling thread called g_object_unref() with a strong
reference that we are about to give up, but at the point where we call
dispose(), we didn't yet decrement the ref count to zero. Doing so would
be a fatal bug.
As such, during dispose() the object is still healthy and still has a
strong pointer. You can call `g_weak_ref_set()` on that pointer without
taking an additional strong reference. Of course, if you don't actually
take a strong reference (and thus don't resurrect the object), then
right afterwards, the last reference is dropped to zero, and the
GWeakRef gets reset again.
But there is no need to claim that you need to take another strong
reference to set a GWeakRef during dispose(). This was always the case.
Also, reword the previous paragraph. I think this is clearer.
Otherwise, it appears as an "in" parameter with
<type name="gint" c:type="volatile gint*"/>
which is very confusing to the bindings.
An alternative would be to mark it as an "inout" integer, which matches
the semantics somewhat. However, that wouldn't quite capture the concept
of the value being modified having (pointer) identity, which is of
utmost importance for atomic operations. Hence, just say that we accept
a pointer to the value.
Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
The array is allocated by the *caller*, not the callee, and then filled
by the callee during the call.
Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
We don’t want `CONTRIBUTING.md` to become an exhaustive manual of how to
contribute to what is a fairly standard C project using Meson, but it is
a fairly helpful hint to mention how to run the test suite. Then people
can use `meson test --help` and the Meson documentation to learn more.
Pointed out on !4589.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
As with previous commits, we’re enabling `-Wsign-conversion` piecemeal
for all of glib.git.
The previous few commits have fixed all the `-Wsign-conversion` warnings
in libgirepository, so let’s enable the warning by default for that
directory to prevent regressions.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
Due to passing around file lengths variously as `gsize` or `gssize`,
we can’t reliably handle files with length greater than `G_MAXSSIZE`, as
some of the APIs in use need `-1` to indicate that their input is nul
terminated.
Add some checks for this, and gracefully return an error if an input
file is too big, rather than just exploding.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
When parsing a GIR or building a typelib, stop setting the array length
field to `-1` as a default. That field is unsigned, so setting it to
`-1` is actually equivalent to setting it to `G_MAXUINT`. I can’t find
anywhere which treats `G_MAXUINT` or `-1` as a magic value there, so
it’s probably better off left unset.
Given the lack of documentation for the typelib code, though, there is a
fair chance I’m making a mistake and this is actually an integral part
of the format. Let’s see what breaks.
This fixes a `-Wsign-conversion` warning, at least.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
This follows up from the previous two commits to add a unit test.
It doesn’t attempt to cover the multitude of other possible type parsing
conditions; at the moment it’s just a regression test for the previous
two commits, and somewhere to hang new tests on in future.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Reworking the code to add proper `GError` handling for type parsing,
rather than the existing `g_critical()`, turned out to actually be
fairly straightforward.
So now `gi_ir_parser_parse_string()` returns
`G_MARKUP_ERROR_INVALID_CONTENT` on unparseable types, just like it does
with various other bits of invalid GIR.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
If parsing a generic type which has no closing `>`, there was no check
that the `strchr()` call succeeded, which could have resulted in a
negative length being passed to `g_strndup()`, which would result in a
long positive length after implicit type casting.
Fix that by bringing an old error handling path back into use. This
results in a `g_critical()` in the calling function, which is good
enough for now. Potentially all this code could be reworked to use
`GError`, but that’s a much bigger project (a lot more of the
`girparser.c` code would need to change).
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
There are a few `g_strndup()` calls which use a length calculated from
the return value of `strchr()` minus the original string. That’s fine,
as long as `strchr()` doesn’t return `NULL`. Add some asserts to ensure
that.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
As with previous commits, we’re enabling `-Wsign-conversion` piecemeal
for all of glib.git.
It turns out that gthread didn’t have any `-Wsign-conversion` warnings
at all, so this was straightforward.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
There was only one `-Wsign-conversion` warning in the whole
subdirectory, so that was easy.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
These are all fairly straightforward, but I didn’t get them locally;
they only showed up on CI.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
Fixing #3405 is going to take a lot of work, so let’s split it up into
pieces and work on them separately. The `gobject/` and `gobject/tests/`
directories now compile cleanly with `-Wsign-conversion` (see the
previous commits), so let’s enable the warning for those directories to
prevent regressions while we continue to work on the other directories.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
There’s a painful inconsistency in the types of the
`g_{test_rand,random,rand}_int{,_range}()` functions, which vary
arbitrarily between `gint32` and `guint32`.
Unfortunately since those functions mention `int` explicitly in the name
(and then some of them return an `unsigned` integer), I don’t see a way
to make the APIs consistent without significant deprecations or
additions.
So, for the moment, to fix various `-Wsign-conversion` warnings, plaster
the tests with casts.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
This fixes a load of -Wsign-conversion warnings. The dest type setter
function is being used (presumably by design?) so there’s sometimes a
type mismatch (signed/unsigned, or size) with the constant value being
used by the test. This just makes the existing implicit casts explicit.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
Fix all the instances where `-Wsign-conversion` was pointing out that
`g_assert_cmpint()` had been used on unsigned inputs, or vice-versa.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
Not sure why these constants were chosen the way they were, but that’s
not a problem I’m going to investigate right now. This just makes the
implicit cast explicit to shut the compiler warning up.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
This fixes `-Wsign-conversion` warnings, though I’m not sure why the
compiler is emitting them. The signed/unsigned status of flag enum
members is not particularly well defined in the C standard (and even
less well understood by me), so just do what seems necessary to shut the
compiler up.
The benefits of enabling `-Wsign-conversion` across the codebase
hopefully outweighs this noise.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
While we’re at it, rename the variables to make the intent a bit
clearer: these functions return a boolean indicating whether any of the
values were modified to make them valid. `n_changed` is a counter of the
number of modified values.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
Unfortunately the signatures of our atomic functions alternate between
using signed and unsigned integers across different functions, so we
can’t just use one type as input. Add some explicit casts to fix
harmless `-Wsign-conversion` warnings.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
Rather than reinventing it ourselves. The old version in `gboxed.c`
could lose the second half of very long strings, as it truncated the
`size_t` string length to the `ssize_t` accepted by
`g_string_new_len()`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
This does a deep copy on the `GString`.
It means we can eliminate the less-efficient version currently used for
`GBoxed`, which also has `-Wsign-conversion` problems. See the following
commit.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
