This will become confusing when we start tracking the owner of a
well-known-name sender, and it's redundant anyway. Instead, track the
1 bit of data that we actually need: whether it's a well-known name.
Strictly speaking this too is redundant, because it's syntactically
derivable from the sender, but only via extra string operations.
A subsequent commit will add a data structure to keep track of the
owner of a well-known-name sender, at which point this boolean will
be replaced by the presence or absence of that data structure.
Signed-off-by: Simon McVittie <smcv@collabora.com>
No functional change, just removing some nesting. The check for whether
signal_data->subscribers is empty changes from a conditional that tests
whether it is into an early-return if it isn't.
A subsequent commit will add additional conditions that make us consider
a SignalData to be still in use and therefore not eligible to be removed.
Signed-off-by: Simon McVittie <smcv@collabora.com>
No functional changes, except that the implicit ownership-transfer
for the rule field becomes explicit (the local variable is set to NULL
afterwards).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Subsequent changes will need to access these data structures from
on_worker_message_received(). No functional change here, only moving
code around.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Using these is a bit more clearly correct than repeating them everywhere.
To avoid excessive diffstat in a branch for a bug fix, I'm not
immediately replacing all existing occurrences of the same literals with
these names.
The names of these constants are chosen to be consistent with libdbus,
despite using somewhat outdated terminology (D-Bus now uses the term
"well-known bus name" for what used to be called a service name,
reserving the word "service" to mean specifically the programs that
have .service files and participate in service activation).
Signed-off-by: Simon McVittie <smcv@collabora.com>
On GNOME/glib#3268 there was some concern about whether this would
allow an attacker to send signals and have them be matched to a
GDBusProxy in this situation, but it seems that was a false alarm.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This somewhat duplicates test_connection_signals(), but is easier to
extend to cover different scenarios.
Each scenario is tested three times: once with lower-level
GDBusConnection APIs, once with the higher-level GDBusProxy (which
cannot implement all of the subscription scenarios, so some message
counts are lower), and once with both (to check that delivery of the
same message to multiple destinations is handled appropriately).
Signed-off-by: Simon McVittie <smcv@collabora.com>
If a connection has two signal subscriptions active for the same signal,
one with arg0 matching and one without, a signal which doesn’t contain
an arg0 value (i.e. `g_dbus_message_get_arg0()` returns `NULL`) will
cause `NULL` to be passed to `strcmp()` when checking for a match
against the signal subscription which *has* arg0 matching, causing a
crash.
Fix that by adding the obvious `NULL` check, and add a unit test.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3342
When piecewise validating the offset table for a variable sized array,
it’s possible that the offset table (`offsets.array`) won’t actually
have been set by `gvs_variable_sized_array_get_frame_offsets()` iff the
serialised `GVariant` is not in normal form.
Add an additional check to guard against this. This will result in an
empty child variant being returned, as with other error handling paths
in `gvs_variable_sized_array_get_child()`.
This is a true positive spotted by scan-build. Thanks, scan-build.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
I’m not sure exactly how this code is supposed to work, so this might
not be the right fix. But there’s definitely a problem here, and it was
spotted by scan-build.
If `param_value_array_validate()` is entered with
`value->data[0].v_pointer == NULL && aspec->fixed_n_elements`, that `NULL`
will be stored in `value_array` too. `value->data[0].v_pointer` will
then be set to a new non-`NULL` array.
A few lines down, `value_array_ensure_size()` is called on
`value_array` – which is still `NULL` – and this results in a `NULL`
pointer dereference.
It looks like `value->data[0].v_pointer` and `value_array` are used
interchangeably throughout the whole of the function, so assign the new
value of `value->data[0].v_pointer` to `value_array` too.
My guess is that `value_array` is just a convenience alias for
`value->data[0].v_pointer`, because the latter is a real mouthful to
type or read.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
Spotted by scan-build, an actual true positive result from it, and a
fiendish one too.
If any of the calls to `dupfd_cloexec()` (except the final one) fail,
the remainder of the `duped_source_fds` array would have been left
uninitialised.
The code in `out_close_fds` would have then called `g_clear_fd()` on an
uninitialised FD, with unpredictable results.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
Rather than `strdup()`ing strings when passing them into
`_xdg_glob_list_append()`, `strdup()` them *inside* the function
instead.
This avoids a leak in the case that the list entry (tuple of `data` and
`mime_type`) already exists in the list.
This has been upstreamed as
https://gitlab.freedesktop.org/xdg/xdgmime/-/merge_requests/36.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
There were a couple of functions in `GDBusConnection` which take a
`user_data` argument, but which then leak it if they error out early.
A true positive spotted by scan-build!
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build thinks that the `atypes` array is leaked, but it’s not.
Ownership is transferred into the `ffi_cif` structure, and it’s
eventually freed in `gi_callable_info_destroy_closure()`.
Try and help the static analysis by adding an explicit ownership
transfer annotation. It probably won’t help.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
There were some error paths where it wasn’t set, returning an
uninitialised value to the caller.
Spotted by scan-build.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
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
With the shell in nounset mode, an error is emitted on referencing
`schemadir` as it is not initialized in all code paths.
Initialize to an empty string to fix.
Signed-off-by: Ville Skyttä <ville.skytta@iki.fi>
This fixes an issue with the number getting very big due to
CPU_ISSET not returning exactly 0 or 1.
This also fixes scenarios where there are holes in the CPU
set. E.g. for a simple run like `taskset --cpu-list 1,2,4 ...`
the old code would return 2 instead of 3, due to iterating
until `ncores` (which is 3) and therefore not accounting for
CPUs further in the set.
Ref https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3784
(cherry picked from commit cc25486b23)
This was erroring on recent GCC because `struct heap_dict` is smaller than
the publicly provided size (guintptr[16]) in the header for GVariantDict.
Port to use `g_malloc()` directly, and use a static assertion to ensure
we’re allocating the larger of the two struct sizes.
These consistently fail on scheduled CI runs, which is not helping our
ability to catch Hurd regressions.
For example, https://gitlab.gnome.org/GNOME/glib/-/jobs/3709402
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
See: #3148
Bumping the reference count from 1 to 2 (and back) is more expensive,
due to the check for toggle notifications.
We have a performance test already that hits that code path. Avoid that
for he "property-{get,set}" tests, so we avoid the known overhead and
test more relevant parts.
Despite all the efforts, there still seems to be a lot of noise in the
performance measurement. Especially, the first iterations seem to run
faster. Maybe that is because the kernel didn't yet determine that the
process is CPU bound and is less likely to schedule it out Or maybe it's
because burning the cycles heats up the CPU and it gets throttled after
a while. It's unclear why, and it's even unclear whether this really
happens. But from my observations, it seems to do.
Hence, more warm up.
- the first time we enter the test, ensure that we keep the CPU busy for
at 2 seconds. This additional warm up (WARM_UP_ALWAYS_SEC) is
global, and not per test.
- for each test, ignore the first 5% of the runs. It seems those tend to
run faster, thus skewing the results.
- if the user specifies a "--factor", the warm up operations are the
same and independent from external factors (such as time
measurements).
Note that this matters the most, when you want to run the executable
twice in a row and compare the results.
By default, the test estimates a run factor for each test. This means,
if you run performance under `perf`, the results are not comparable,
as the run time depends on the estimated factor.
Add an option, to set a fixed factor.
Of course, there is only one factor argument for all tests. Quite
possibly, you would want to run each test individually with a factor
appropriate for the test. On the other hand, all tests should be tuned
so that the same factor gives a similar test duration. So this may not
be a concern, or the tests should be adjusted. In any case, the option
is most useful when running only one test explicitly.
You can get a suitable factor by running the test once with "--verbose".
Another use case is if you run the benchmark under valgrind. Valgrind
slows down the run so much, that the estimated factor would be quite
off. As a result, the chosen code paths are different from the real run.
By setting the factor, the timing measurements don't affect the executed
code.
The default output is annoyingly verbose. You see
Running test simple-construction
simple-construction: Millions of constructed objects per second: 33.498
Running test simple-construction1
simple-construction1: Millions of constructed objects per second: 142.493
Running test complex-construction
complex-construction: Millions of constructed objects per second: 14.304
Running test complex-construction1
...
where the "Running test" lines just clutter the output. In fact so much
so, that my terminal fills up and I don't see the output of all tests in
one page. The "Running test" line is not so useful, because I mostly
care about the test result, and that line already contains the test
name.
Add an option to silence this.
