Rather than returning through `G_VARIANT_TYPE`, which scan-build doesn’t
seem to fully understand ownership transfers through, just return `new`
directly, and do the `is_valid()` check separately.
The new code is equivalent to the old code, but squashes a scan-build
false positive around leaking `dest`. (See also: the previous commit.)
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
This introduces no functional changes. Switch from incrementing a
pointer to incrementing a counter and using array indexing.
This squashes a scan-build false positive, where it can’t choose which
of `dest` and `new` ‘own’ the newly allocated memory, so it kind of
assumes both do, and then warns there’s a potential leak of `dest` when
the function returns. In actual fact, ownership of the memory is
returned via `new`.
Partly this might be masked through use of the `G_VARIANT_TYPE` macro,
which the following commit will address.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
Otherwise scan-build thinks there could be `NULL` pointer dereference of
the `tz`. (There can’t be, it’s a false positive. 🤫)
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build thinks there’s a potential `NULL` pointer dereference of some
of the members of `msg->strings`, because it doesn’t know about the
implicit invariant that the length of `msg->strings` is
`msg->n_strings`.
Ideally we want an assertion like `g_assert (g_strv_length
(msg->strings) == msg->n_strings)`, but that’s not very performant, so
just settle for a non-`NULL` assertion on each loop iteration to give
scan-build the hint it needs.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
This helps out scan-build, which otherwise thinks there could be a
`NULL` pointer dereference.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build thinks that `tmp` can be dereferenced before it’s all been
assigned to. I don’t think that’s the case, because the number of
elements in it which have been assigned to is tracked as `i`. But static
analysers find that kind of state tracking hard to reason about, so
let’s just zero-initialise the array to simplify things.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
It’s very obviously a false positive, as `str` has been added to on the
previous line, so can’t be `(void *) 0`. Not sure what scan-build is
thinking.
I’d rather not have this assertion (it doesn’t help the programmer’s
understanding of the code), but I would also rather have scan-build
running with no warnings so that it can helpfully catch newly-introduced
errors in future.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
See the code comment. scan-build can’t handle analysis over the
refcounts, so consistently complains about potential use-after-free
errors in the code, essentially because:
* It understands `name_unref()`, but completely ignores `name_ref()`
* The code often calls `name_unref()` on the ‘wrong’ pointer, in the
sense that it knows that if another struct exists, that struct holds
a ref on a `Name`, but without actually having a pointer to the
`Name`. So the code calls `name_unref (name); name_unref (name)`.
That’s valid, but quite understandably looks like a recipe for a
use-after-free.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build thinks that there can be a `NULL` pointer dereference of
`error` here because it doesn’t understand that the function return
value and `GError` are related: when a valid return value is returned,
the error is `NULL` and vice-versa.
Try and make that clearer to the static analyser by checking whether the
error is `NULL`, rather than the return value.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build thinks there could be a `NULL` pointer dereference of
`t->data` here. It’s wrong, so add an assertion to try and help it
understand the control flow.
The loop is exited as soon as a target is found whose weight is greater
than or equal to a random value between 0 and the sum of all the weights
in the set of remaining targets in the loop. By definition, the last
target in the loop always satisfies this condition, so a target will
always be chosen, and hence `t` will never be `NULL` within the loop.
`t->data` will never be `NULL` by construction of the target list.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build thinks that `data` could be leaked. It’s not, though; it’s
passed as the `user_data` to `g_dbus_connection_register_object()` along
with its free function.
Try and persuade scan-build that there’s no leak by annotating the
transfer.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
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 was complaining that `dest_hostname` and `dest_protocol` were
used after being freed, which could potentially happen if the code is
built with `G_DISABLE_CHECKS`. This is a false positive, because the
state of types in the program should be the same regardless of whether
`G_DISABLE_CHECKS` is used.
However, the code did smell. If we are trying to free things and return
gracefully if the underlying socket address enumerator returns something
of the wrong type, why not free the rest of the function’s state, or
skip the invalid address and move on to the next one? Or if we are trying
to make an assertion, why bother freeing some temporary data at all?
This halfway house doesn’t make sense.
So turn the `g_return_val_if_fail()` into a full assertion.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build was complaining that the `wc_buffer[old_n_wc]` in `cc =
COMBINING_CLASS (wc_buffer[old_n_wc])` could dereference memory off the
end of the initialised `wc_buffer` array. It came to this conclusion by
assuming that the result of `find_decomposition()` for one of the
`gunichar`s was a non-`NULL` empty string, so that iteration of the
decomposition loop didn’t append anything to `wc_buffer`.
I don’t think it’s possible for an iteration of the loop to *not* append
anything to `wc_buffer`. Unicode characters don’t decompose to nothing.
Indeed, the current code coverage for GLib says that the `if (n_wc > 0)`
branch is always taken, and at that point in the control flow, `n_wc <=
0` is never true.
So, add an assertion to check that progress is made (i.e. `n_wc` is
incremented by at least 1), and remove the unnecessary condition.
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
scan-build thinks that `gvs_variable_sized_array_is_normal()` can do a
`NULL` pointer dereference on `value.data` when `value.size == 0`. This
isn’t possible, because `offsets.length == 0` always when `value.size ==
0`, but that’s a bit of a complex relationship which the static analyser
can’t work out.
Give it some help by adding an assertion.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
After a lot of loop unwinding, during which I think it might have lost
its knowledge that `cache->buffer != NULL` (from a prior check on line
765), scan-build seems to think that there can be a `NULL` pointer
dereference of `cache->buffer` within `cache_magic_compare_to_data()`.
There can’t be. Add an assertion to try and help the analyser.
Upstreamed as
https://gitlab.freedesktop.org/xdg/xdgmime/-/merge_requests/38.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build thinks there can be a `NULL` pointer dereference in `while
((i = N_NODES (node->left)) != pos)`, if `node` is `NULL`.
`node` cannot be `NULL`, though, assuming the `n_nodes` member of each
node in the tree is an accurate count of the number of nodes beneath
that point. It controls the tree descent and avoids trying to descend
beneath a leaf.
A static analyser can’t know this though, so let’s add an assertion to
help.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build thinks that it’s possible for `read_netlink_messages()` to
return `FALSE` and an unset error (or `TRUE` and a set error), and this
belief causes it to emit warnings for code which calls
`read_netlink_messages()`.
That’s not possible, but the function is written in such a way that
following the control flow would be hard for a static analyser. It would
have to work out that `retval` and `local_error == NULL` are identical
on all control flow branches.
Avoid the need for such complex analysis by eliminating `retval` and
just using `local_error` throughout.
This introduces no functional changes to the code.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build thinks that `term_arg` could be used uninitialised. I think
there isn’t a bug here because that use is protected by the
`found_terminal == NULL` check and early return. But perhaps that logic
is a bit too complex for static analysis, so add a default value for the
variable.
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>
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>
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>
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>
The previous approach was to return a length as a `gssize`, with
negative values indicating failure. That works fine, but causes a lot of
signed/unsigned comparisons or assignments.
Tidy the code up by splitting success from length, returning success as
a boolean, and length as a `size_t*` out argument. This introduces no
functional changes, but does tidy the code up and fix some compiler
integer warnings.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
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>
Looks like the original author mixed up where the link label and the
link URL goes. :p
Previously the link would point to "https://docs.gtk.org/gio/file
attributes", with a space and no file extension.
There is a meson option (gir_dir_prefix), but without being passed in here
the files would always get installed into the default location (datadir).
Signed-off-by: Alexander Kanavin <alex@linutronix.de>
The recently added libmount-based unix mount monitoring may fail when the
device exceeds inotify limits. Let's fallback to the older implementation
in case of the `mnt_monitor_get_fd` function failure. This among others
fixes tracker-miners failures caused by seccomp rules.
Fixes: https://gitlab.gnome.org/GNOME/tracker-miners/-/issues/315
The define for g_utf8_next_char(p) includes a not needed final cast to
(char *). In fact, this cast has the adverse effect of causing a warning
if p is a (const char *) with gcc/clang compiler option -Wcast-qual.
So lets remove the not needed cast and add option -Werror=cast-qual
to glib/tests/utf8-pointer.c which uses g_utf8_next_char().
Now utf8-pointer.c compiles also with compiler option -Werror=cast-qual
and passes all tests.
This is a partial revert and rework of commit
c79575362e, for the `gsettings` script
only (the other completion scripts are fine).
I blindly added quoting to everything shellcheck told me to, without
testing it properly.
As it turns out, the `$schemadir` argument to `gsettings` invocations
was deliberately not quoted, so that it would expand to zero arguments
if unset, and two arguments (`--schemadir /some/path`) if set earlier in
the command-being-completed.
Quoting it meant that it expanded to one argument (the empty string) if
unset, which caused the `gsettings` subcommands to fail, and hence any
further tab completion to fail.
Fix that as suggested on https://www.shellcheck.net/wiki/SC2086 by
turning `schemadir` into an array, which either has zero members if
unset, or two members if set.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
The copyright entries come from looking at `git log gio/completion/*`
and, in particular, `git log -- gio/gsettings-bash-completion.sh` (etc.)
as the files were moved after being originally written, and haven’t
really changed since.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1415
As suggested by Ville Skyttä in
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4012#note_2084405,
make sure to invoke the copy of the command which is being completed
when asking for completions of a given subcommand.
This avoids accidentally invoking any old `gdbus`/`gresource`/etc.
binary which is hanging around in another part of `$PATH`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
More files now have their copyright and/or licensing tagged explicitly,
so let’s reduce the wiggle room for regressions.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
See: #1415
The copyright from `git log gio/inotify/meson.build` is now included in
the file header. The following commits are too trivial to be
copyrightable:
- d10be6102f
- 03e86d000f
- 1741fc2c6e
- 8733d172a3
The file was contributed while the `COPYING` file for GLib was
LGPL-2.1-or-later, so was previously implicitly licensed as that.
Let’s make that explicit.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1415
