Because of the generic nature of `GError`, `g_set_error()` has to take
an `int`, but `g_file_error_from_errno()` returns a `GFileError`. The
macOS CI runner decides that’s a good reason to emit
`-Wsign-conversion`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
This struct is only ever heap allocated, and enums are always the same
size as an int (or unsigned int), so it won’t change size.
The struct doesn’t correspond to any mmapped structure from a
typelib file.
This should fix some `-Wsign-conversion` warnings (curiously only seen
on the macOS CI runner) by using the most specific type.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
Parsing scripts using g_shell_parse_argv() line by line, it's useful to
have empty comments, so one can write pragraphs etc, e.g.
# This is a comment with multiple paragraphs.
#
# It's useful to split things up like this at times.
#
# The empty comment characters makes it clear the paragraphs form a
# single comment.
If g_file_set_contents{_full,} is replacing an existing file, require
that the tmpfile have the same mode as the existing file.
This prevents the umask from taking effect for consistent writes to
existing files.
ClosesGNOME/dconf#76
We can tighten up the types which are being used, to prevent the
warnings. Not everything in the world has to be a `guint`.
These warnings only showed up on the macOS CI runner.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3405
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
