g_strdup() is often used to duplicate static strings, in these cases the
compiler could use a faster path because it knows the length of the
string at compile time, but this cannot happen because our g_strdup()
implementation is hidden.
To improve this case, we add a simple implementation of g_strdup() when
it is used with static or NULL strings that explicitly uses strlen,
g_malloc and memcpy to give hints to the compiler how to behave better.
This has definitely some benefits in terms of performances, causing an
iteration of 1000000 string duplication to drop from 2.7002s to 1.9428s
for a static string and from ~0.6584s to ~0.4408 for a NULL one.
Since compiler can optimize these cases quite a bit, the generated code
[2] is not increasing a lot, given that it can now avoid generating some
code or do it in few simpler steps.
Update tests to cover both inlined and non inlined cases.
[1] https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3209#note_1644383
[2] https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3209#note_1646662
We have actual definitions for these functions so we should ignore their
inline versions, not to potentially break doc parsers due to the different
argument names.
For some reasons we can't merge the check together with the gnu C check
if, otherwise the check gets ignored by doc parser.
We could have unguarded crashes when calling strlen (NULL) or when passing
invalid GString's.
Also ensure that we are not using the macro `val` argument multiple times as
it may lead to an unwanted behavior when passing to it a variable value such
as `str[++i]`, as the value may be called multiple times.
C++ tests and Coverity were both underlining this.
Fixes: #2890
Now that the implementation of GSlice has been dropped, these tests for
the internals of the implementation are unnecessary.
We can keep `glib/tests/slice.c` as it tests the API rather than the
implementation.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1079
Keep the API for ABI compatibility.
See
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/2935#note_1650099
for a summary of the reasoning for this change:
- The performance of system-provided allocators has improved since
GSlice was written, and they are now similarly as performant, or more
performant, than GSlice.
- The code is unmaintained and nobody understands it.
- It doesn’t integrate with tooling and system security features which
have been written for the system `malloc()` implementation (such as
sanitisers, valgrind, etc.).
- It’s confusing for developers: should they use `g_slice_new()` or
`g_new()`?
- GSlice is faster than the libc allocator for allocating and
(particularly) freeing linked lists, but since these are a rubbish
data structure, that’s not a great thing to optimise for.
For the cases where application performance is negatively impacted by
the implementation of GSlice being dropped (and we don’t think there’ll
be many), applications can use a drop-in `malloc()` replacement which is
more suited to their particular workload. Choosing an allocator in GLib
to suit all application workloads is not possible.
Including documentation updates and cleanups by Philip Withnall.
Fixes: #1079
Fix the tests, by allocating the structure.
==121338==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7ffe44018610 at pc 0x00000040ff71 bp 0x7ffe440178f0 sp 0x7ffe440178e8
READ of size 8 at 0x7ffe44018610 thread T0
#0 0x40ff70 in test_launch_uris_with_terminal ../gio/tests/desktop-app-info.c:1393
#1 0x7efd97b831e8 in test_case_run ../glib/gtestutils.c:2947
#2 0x7efd97b831e8 in g_test_run_suite_internal ../glib/gtestutils.c:3037
#3 0x7efd97b82d23 in g_test_run_suite_internal ../glib/gtestutils.c:3056
#4 0x7efd97b82d23 in g_test_run_suite_internal ../glib/gtestutils.c:3056
#5 0x7efd97b82d23 in g_test_run_suite_internal ../glib/gtestutils.c:3056
#6 0x7efd97b84189 in g_test_run_suite ../glib/gtestutils.c:3136
#7 0x7efd97b842c5 in g_test_run ../glib/gtestutils.c:2248
#8 0x4055bc in main ../gio/tests/desktop-app-info.c:1901
#9 0x7efd9564a50f in __libc_start_call_main (/lib64/libc.so.6+0x2750f)
#10 0x7efd9564a5c8 in __libc_start_main_alias_1 (/lib64/libc.so.6+0x275c8)
#11 0x4059f4 in _start (/home/elmarco/src/gnome/glib/build/gio/tests/desktop-app-info+0x4059f4)
Address 0x7ffe44018610 is located in stack of thread T0 at offset 128 in frame
#0 0x404d1f in main ../gio/tests/desktop-app-info.c:1823
This frame has 6 object(s):
[48, 52) 'argc' (line 1821)
[64, 72) 'path' (line 1870)
[96, 104) 'argv' (line 1822)
[128, 144) '<unknown>' <== Memory access at offset 128 is inside this variable
[160, 176) '<unknown>'
[192, 288) 'supported_terminals' (line 1825)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
There’s a kernel bug on the CI machines which is causing this test to
fail all the time and it’s getting my goat.
The test can be re-enabled later (by reverting this commit) when the
kernel on the CI VM host is fixed. I don’t know when that’s going to
happen.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2879
Although unlikely, apparently `message` may be `NULL` (the rest of the
code checks for it), so the code to convert newlines to spaces should
probably also check for `NULL`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Coverity CID: #1504054
Otherwise if, for whatever reason, the `app` loses its D-Bus name,
`g_application_quit()` is called from `name_was_lost()` before it’s
called from `quit_already()`, and then `quit_already()` does an invalid
read on `app`.
If the name was not meant to be lost at this point in the test, the
subsequent `g_assert_false (name_lost)` will catch that, so this change
shouldn’t cause the test to pass unnecessarily.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This should stop the following valgrind error:
```
==14506== 80 bytes in 1 blocks are definitely lost in loss record 1,098 of 1,463
==14506== at 0x484086F: malloc (vg_replace_malloc.c:381)
==14506== by 0x48C5D83: g_malloc (gmem.c:130)
==14506== by 0x48E7911: g_slice_alloc (gslice.c:1074)
==14506== by 0x48E7951: g_slice_alloc0 (gslice.c:1100)
==14506== by 0x493617D: g_system_thread_new (gthread-posix.c:1188)
==14506== by 0x48F9EAA: g_thread_new_internal (gthread.c:935)
==14506== by 0x48F9E29: g_thread_try_new (gthread.c:919)
==14506== by 0x48FA351: g_thread_pool_spawn_thread (gthreadpool.c:312)
==14506== by 0x48F9D2C: g_thread_proxy (gthread.c:831)
==14506== by 0x4EFA2A4: start_thread (in /usr/lib64/libpthread-2.33.so)
==14506== by 0x4D89322: clone (in /usr/lib64/libc-2.33.so)
```
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Now that we're using TAP parsing, this will show subtest failures in
details but without showing any logging error, that we'd still need to parse
from actual logs.
When we've a failure our TAP reporting just bails out without that is clear
what is the test that has failed.
So in this case, expose a "not ok" message if the test name is known, and
use it to report the error message too if available.
Otherwise just use the same Bail out! strategy.
This is something that a base test should decide, so if a subtest fails it's
up to the caller to decide whether to bail out the whole suite or just the
subtest.
