Even if we get warnings from the first lint check, we probably want to
see the warnings from later lint checks too, to reduce the number of
round-trips.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This will make it easier and more obvious for developers to run them
locally: I'm sure I'm not the only developer who had assumed that
`.gitlab-ci/` is private to the CI environment and inappropriate (or
perhaps even destructive) to run on a developer/user system.
The lint checks are automatically skipped (with TAP SKIP syntax) if we
are not in a git checkout, or if git or the lint tool is missing. They
can also be disabled explicitly with `meson test --no-suite=lint`,
which downstream distributions will probably want to do.
By default, most lint checks are reported as an "expected failure"
(with TAP TODO syntax) rather than a hard failure, because they do not
indicate a functional problem with GLib and there is a tendency for
lint tools to introduce additional checks or become more strict over
time. Developers can override this by configuring with `-Dwerror=true`
(which also makes compiler warnings into fatal errors), or by running
the test suite like `LINT_WARNINGS_ARE_ERRORS=1 meson test --suite=lint`.
One exception to this is tests/check-missing-install-tag.py, which is
checking a functionally significant feature of our build system, and
seems like it is unlikely to have false positives: if that one fails,
it is reported as a hard failure.
run-style-check-diff.sh and run-check-todos.sh are not currently given
this treatment, because they require search-common-ancestor.sh, which
uses Gitlab-CI-specific information to find out which commits are in-scope
for checking.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Since they are copylibs, we don’t want to diverge from upstream by
adding SPDX lines to all the files, so add them to the dep5 file
instead.
gnulib discussed adding them upstream a couple of years ago but the
discussion seemed to peter out:
https://lists.gnu.org/archive/html/bug-gnulib/2021-06/msg00004.html.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Otherwise we would generate a multiple-inclusion guard of the
form `#ifndef __STDOUT__ ...`, which can only work for one D-Bus
interface per translation unit.
Signed-off-by: Simon McVittie <smcv@collabora.com>
If we're writing the body to standard output, we cannot know what the
filename of the corresponding header is going to be, but it seems
vanishingly unlikely that it will be either `stdout.h` (which we would
traditionally have generated) or `-.h` (which we would have generated
since !3886).
This makes some of the output snippets sufficiently short that black(1)
requires that they are folded into a single line.
Signed-off-by: Simon McVittie <smcv@collabora.com>
In command-line tools, ordinary filenames normally do not have
special-cased meanings, so commit 3ef742eb "Don't skip dbus-codegen tests
on Win32" was a command-line API break: in the unlikely event that a
user wanted to write to a file named exactly `stdout`, this would have
been an incompatible change.
There is a conventional pseudo-filename to represent standard output,
which is `-` (for example `cat -` is a no-op filter). Adding support
for this is technically also a command-line API break (in the very
unlikely event that a user wants to write to a file named exactly `-`,
they would now have to write it as `./-`), but filenames starting with
a dash often require special treatment anyway, so this probably will not
come as a surprise to anyone.
When the output filename is `-` we don't want to use `#ifdef _____` as
a header guard, so special-case it as `__STDOUT__` as before.
Signed-off-by: Simon McVittie <smcv@collabora.com>
It’s reasonable for the `main()` function in a test suite to pass
ownership of some test data to `g_test_add_data_func_full()`, along with
a `GDestroyNotify`, and rely on GTest to free the data after all tests
have been run.
Unfortunately that only worked if the test was run, and not skipped
before its test function was called. This could happen if, for example,
it had `/subprocess` in its path.
Fix that by always freeing the test data. This required reworking how
tests are skipped, slightly, to bring all the logic for that within
`test_case_run()`, so that it could always handle the memory management.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3248
It’s more helpful to always register the test, even if it’s normally
skipped, since then the skip is recorded in the test logs so people can
see what’s ‘missing’ from them.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
The function arguments index_ and length could lead to a sum which is
larger than G_MAXUINT, possibly leading to out of boundary accesses
in array_remove_range functions.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Fixes: #3240
It's not obvious why we wouldn't use g_quark_try_string(). Add a code
comment that this is intentional and a reference for how to find out
more.
Also, fix typo in another code comment.
So that it matches `gi_arg_info_get_type_info()`. We can’t use
`gi_arg_info_get_type()` because that collides with the `GType` getter
for the type.
Spotted by Philip Chimento.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3243
The amount of used memory should stay in relation to the number of
entries we have. If we delete most (75%) of the entries, let's also
reallocate the buffer down to 50% of its size.
datalist_append() now starts with 2 elements. This works together with
the shrinking. If we only have one entry left, we will shrink the buffer
back to size 2. In general, d->alloc is always a power of two (unless it
overflows after G_MAXUINT32/2, which we assume will never happen).
The previous buffer growth strategy of never shrinking is not
necessarily bad. It has the advantage to not require any checks for
shrinking, and it works well in cases where the amount of data actually
does not shrink (as we'd often expect).
Also, it's questionable what a realloc() to a smaller size really
brings. Is that really gonna help and will the allocator do something
useful?
Anyway. This patch introduces shrinking. The check for whether to shrink
changes from `if (d->len == 0)` to `if (d->len <= d->alloc / 4u)`, which
is probably cheap even if most of the time we don't need to shrink. For
most cases, that's the only change that this patch brings. However, once
we find out that 75% of the buffer are empty, calling realloc() seems a
sensible thing to do.
GI_IS_REGISTERED_TYPE_INFO() wasn't working because it was actually
defined to be the same as GI_IS_OBJECT_INFO().
Add some desultory type-checking assertions to the repository tests.
gi_repository_enumerate_versions() was missing a type check of the
instance parameter. This helps catch mistakes when porting from
girepository 1.x where the parameter was allowed to be null.
Memory was leaking when allocating it inside libelf and losing the pointer to it (it was an automatic variable) when returning NULL from the get_elf function in some cases
Closes#3242
Signed-off-by: Maxim Moskalets <Maxim.Moskalets@kaspersky.com>
The main point here is to reuse datalist_remove() and datalist_shrink().
Especially, datalist_shrink() will become more interesting next, when it
actually shrinks the buffer.
Also, I find the previous implementation with "data_end" confusing.
Instead, only use index "i_data" to iterate over the data.
Extract helper functions datalist_remove() and datalist_shrink(). This
is to reduce duplicate code, but also to have a default way how to do
this.
In particular, later datalist_shrink() might do more aggressive
shrinking. We need to have that code in one place.
g_datalist_unlock() is probably faster than g_datalist_unlock_and_set().
Move the "if (data)" check (that we anyway had) earlier, so we can
call g_datalist_unlock() and return early.
If too many keys are requested, they temporary buffer is allocated
on the heap. There is no problem in principle, to remove more than
16 keys.
Well, the problem is that GData tracks entries in a linear list, so
performance will degrade when it grows too much. That is a problem,
and users should be careful to not add unreasonably many keys. But it's
not the task of g_datalist_id_remove_multiple() to decide what is
reasonable.
This limitation was present from the beginning, in commit 0415bf9412
('Add g_datalist_id_remove_multiple'). It's no longer necessary since
commit eada6be364 ('gdataset: cleanup g_data_remove_internal()').
GSList doesn't seem the best choice here. It's benefits are that it's
relatively convenient to use (albeit not very efficient) and that an
empty list requires only the pointer to the list's head.
But for non-empty list, we need to allocate GSList elements. We can do
better, by writing more code.
I think it's worth optimizing GObject, at the expense of a bit(?) more
complicated code. The complicated code is still entirely self-contained,
so unless you review WeakRefData usage, it doesn't need to bother you.
Note that this can be easily measure to be a bit faster. But I think the
more important part is to safe some allocations. Often objects are
long-lived, and the GWeakRef will be tracked for a long time. It is
interesting, to optimize the memory usage of that.
- if the list only contains one weak reference, it's interned/embedded in
WeakRefData.list.one. Otherwise, an array is allocated and tracked
at WeakRefData.list.many.
- when the buffer grows, we double the size. When the buffer shrinks,
we reallocate to 50% when 75% are empty. When the buffer shrinks to
length 1, we free it (so that "list.one" is always used with a length
of 1).
That means, at worst case we waste 75% of the allocated buffer,
which is a choice in the hope that future weak references will be
registered, and that this is a suitable strategy.
- on architectures like x86_68, does this not increase the size of
WeakRefData.
Also, the number of weak-refs is now limited to 65535, and now an
assertion fails when you try to register more than that. But note that
the internal tracking just uses a linear search, so you really don't
want to register thousands of weak references on an object. If you do
that, the current implementation is not suitable anyway and you must
rethink your approach. Nor does it make sense to optimize the
implementation for such a use case. Instead, the implementation is
optimized for a few (one!) weak reference per object.
We can safely combine this, and use bit 30 of the ref-count for locking.
This leaves still 2^30-1 for the ref-count, which is more than enough,
because these references are only taken for a short time in
g_weak_ref_get() and g_weak_ref_set(). Note that one thread can at most
take one reference at a time, so the ref-count will always a smaller
number.
Also note, that obviously we will only take a bit lock while also
holding a reference. That means, when weak_ref_data_unref() decreases
the ref-count to zero, the bit will be unlocked as well.
The reason to do this is to free up some space in WeakRefData. Note that
(on x86_64) this doesn't actually make the struct smaller. It's
probably not reasonably possible to make WeakRefData smaller than it
already is (on x86_64). However, by combining the fields we have some
space for reuse without increasing the struct size. That space will be
used next.
The implementation of GWeakRef tracks weak references in a way, that
requires linear search. That is probably best, for an expected low
number of entries (e.g. compared to the overhead of having a hash
table). However, it means, if you create thousands of weak references,
performance start to degrade.
Add a test that creates 64k weak references. Just to see how it goes.
