If the first `goto out` is taken, `file` has not yet been initialised,
but `g_clear_object (&file)` is called on it, and things get unhappy.
Fix that by following standard convention and initialising
‘autoptr’-like variables at declaration time.
Spotted by scan-build in
https://gitlab.gnome.org/GNOME/glib/-/jobs/5321883.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Backport !4667 “Incorrect output parameter handling in closure helper of g_settings_bind_with_mapping_closures” to glib-2-84
See merge request GNOME/glib!4668
After commit 34b7992fd6 the overflow check
was only done when expanding the string, but we need to do it before
checking whether to expand the string, otherwise that calculation could
overflow and falsely decide that the string is big enough already.
As a concrete example, consider a `GString` which has:
* `.len = G_MAXSIZE / 2 + 1`
* `.allocated_len = G_MAXSIZE / 2 + 1`
and `g_string_append()` is called on it with an input string of length
`G_MAXSIZE / 2`.
This results in a call `g_string_maybe_expand (string, G_MAXSIZE / 2)`,
which calculates `string->len + len` as `(G_MAXSIZE / 2 + 1) +
(G_MAXSIZE / 2)` which evaluates to `1` as it overflows. This is not
greater than `string->allocated_len` (which is `G_MAXSIZE / 2 + 1`), so
`g_string_expand()` is *not* called, and `g_string_maybe_expand()`
returns successfully. The caller then assumes that there’s enough space
in the buffer, and happily continues to cause a buffer overflow.
It’s unlikely anyone could hit this in practice because it requires
ludicrously big strings and `GString` allocations, which likely would
have been blocked by other code, but if we’re going to have the overflow
checks in `GString` then they should be effective.
Spotted by code inspection.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
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
From `man 3 ctime`:
According to POSIX.1, localtime() is required to behave as though tzset(3)
was called, while localtime_r() does not have this requirement.
For portable code, tzset(3) should be called before localtime_r().
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.
(cherry picked from commit cc647f9e46)
Co-authored-by: Michael Catanzaro <mcatanzaro@redhat.com>
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
Depending on luck, g_closure_ref may access closure->vint and observe
different values between reads. This manifests as a test failure in
signals-refcount{2,4}, properties-refcount1, and closure-refcount depending
on timing and re-runs.
Jakub Jelinek analysed this on GCC bug PR119607 after I'd reported it
over there as a possible GCC regression.
The critical part being g_closure_ref -> ATOMIC_INC_ASSIGN -> ATOMIC_CHANGE_FIELD
where closure->vint gets re-read repeatedly, both outside and inside the retry
loop. To fix that:
1. Atomically fetch it the first time;
2. Use the cached read, not a fresh read, of vint in the loop;
3. Use g_atomic_int_compare_and_exchange_full in the loop so we get a freshly
cached vint if it changed in another thread.
Bug: https://gcc.gnu.org/PR119607
Fixes: 834ddd19 ('turned all modifications to the first 32 integer bits in a closure into')
Co-authored-by: Jakub Jelinek <jakub@redhat.com>
We had code to avoid that we could call a toggle "up" notification
callback in locked state, but this was not covering the case in which
the cancellable second to last reference was removed in its cancellation
callback.
In fact, in such case we end up going from 2 -> 1 references during the
signal callback call and this leads to calling the toggle notify
callback in locked state.
To prevent this, add an even further reference before calling the
callback (in locked state, but there's no risk that a toggle-up
notification happens now), and drop it once unlocked again.
If when calling g_cancellable_connect() the cancellable was already
cancelled we could have ended up in calling the data cleanup
function while the cancellable lock was held.
This is likely not an issue, but it's still better not to do it,
so protect the code against it
We had wrong comments, in particular:
@callback is called exactly once each time @cancellable is cancelled,
either directly at the time of the connect if @cancellable is already
cancelled, or when @cancellable is cancelled in some thread.
In case the cancellable is reset via [method@Gio.Cancellable.reset]
then the callback can be called again if the @cancellable is cancelled
This is not true if `@cancellable` is already cancelled because then the
callback will be thrown away, and will not be called for subsequent
cancellations.
Since GLib 2.40, the lock protecting @cancellable is not held when
@callback is invoked. This lifts a restriction in place for
earlier GLib versions which now makes it easier to write cleanup
code that unconditionally invokes e.g. g_cancellable_cancel().
As per commit 3a07b2abd4 this section needs some amending since
it's not true anymore, as the GCancellable's mutex is held when `@callback` is
invoked. However this happens in a way that it's still safe to call most
of the GCancellable API.
Closes: #3642
When a non-cancelled cancellable ::cancelled signal callback is called
the cancellable has enough references so that it can be unreferenced on
the callback itself. However this doesn't happen if the cancellable has
been already cancelled at the moment we connect to it.
To prevent this, add a temporary reference before calling the signal
callback.
Note that we do this also if the callback has not been already cancelled
to prevent that we may end up calling a toggle-notify callback while we
are locked.
Add tests
Closes: #3643
In function ‘memmem_with_end_pointer’,
inlined from ‘domain_found’ at ../glib/gmessages.c:2695:16,
inlined from ‘should_drop_message’ at ../glib/gmessages.c:2802:35:
../glib/gmessages.c:2674:19: warning: ‘log_domain_length’ may be used
uninitialized [-Wmaybe-uninitialized]
2674 | #define my_memmem memmem
| ^
../glib/gmessages.c:2683:10: note: in expansion of macro ‘my_memmem’
2683 | return my_memmem (haystack, (const char *) haystack_end - (const char *) haystack, needle, needle_len);
| ^~~~~~~~~
../glib/gmessages.c: In function ‘should_drop_message’:
../glib/gmessages.c:2760:13: note: ‘log_domain_length’ was declared here
2760 | gsize log_domain_length;
| ^~~~~~~~~~~~~~~~~
