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
Add an internal helper for `g_checksum_update()` to avoid having to take
its length as a `gssize` for internal users who already know the input
length.
Fixes a `-Wsign-conversion` warning.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
The code wasn’t written for it, and we don’t want to diverge from
upstream for this, so disable -Wsign-conversion in case it was set for
the overall GLib build.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
The parser was assuming that all three separators (`_@.`) were in order;
but the input might not contain them in order. In that case, the parser
would have passed negative values to `g_strndup()` which would have been
implicitly cast to large positive values, and potentially exposed a lot
of memory (until the first nul byte, which was probably quite soon).
Expand the existing `g_get_locale_variants()` test to cover some invalid
parsing, and add a fuzzing test too.
Spotted by `-Wsign-conversion`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
It's expected that an interface may have a NULL socket address, so just
skip it and don't crash if so. In practice, this occurs for VPN
interfaces.
Fixes: fe0139ee98
Thankfully this struct is also internal, so we can happily change the
types of any field except the first one (which is in the public `GTuples`
prefix).
This fixes the remaining `-Wsign-conversion` warnings for `grel.c`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
`GTuples.len` is a `guint`.
This doesn’t seem to have caused any problems or warnings, but let’s get
it correct.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
We would otherwise have to have cast and lost width and signedness on
both the arguments to, and return value from, `g_relation_count()`.
Simpler to add an internal helper with the right types, and only do the
casts on the public API wrapper where API stability means it’s
unavoidable.
Fixes some more `-Wsign-conversion` warnings.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
Previously this was passed straight into an array dereference, so could
easily have under- or over-flowed the array.
Add a precondition check to it. This doesn’t change the API contract,
because the code already emitted a critical warning if the index didn’t
find a table:
```
g_return_val_if_fail (table != NULL, NULL)
```
This fixes a load of `-Wsign-conversion` warnings and makes these
functions safer.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
This is what’s documented as supported (and the only value that is
supported), so might as well check for it early.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
The internals of this struct are private, so we have some freedom to fix
the types used to be wider (to allow addressing all of memory) and
correctly unsigned.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
The code wasn’t written for it, and we don’t want to diverge from
upstream for this, so disable -Wsign-conversion in case it was set for
the overall GLib build.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
Wherever we use gssize to allow passing -1, we need to ensure we don't
overflow the value by assigning a gsize to it without checking if the
size exceeds the maximum gssize. The safest way to do this is to just
use normal gsize everywhere instead and use gssize only for the
parameter.
Our computers don't have enough RAM to write tests for this. I tried
forcing string->len to high values for test purposes, but this isn't
valid and will just cause out of bounds reads/writes due to
string->allocated_len being unexpectedly small, so I don't think we can
test this easily.
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 seems very unlikely that you really could achieve higher parallelism
by taking more locks (and minimizing the time to hold the GData lock).
On the contrary, taking one lock less and doing all the work there is
beneficial.
