My GTK testlogs are filled with
GLib-DEBUG: g_unix_open_pipe() called with FD_CLOEXEC; please migrate to using O_CLOEXEC instead
Lets take the hint, and migrate to using O_CLOEXEC.
As with `test-printf`, the `vasprintf()` placeholder checks on BSDs
(including macOS) are less strict than on Linux (glibc), so the expected
critical message is not seen.
Change the test to not expect it on BSDs.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3187
It was previously possible for this to silently fail, which isn’t great
for program correctness. Instead, raise a critical warning so the
programmer knows to either validate their Unicode inputs, or fix their
format placeholders.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3187
It’s possible for the function to fail for the same reasons
`g_vasprintf()` would fail.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3187
As per the previous commit, it’s possible for
`g_printf_string_upper_bound()` to return an error. We need to catch and
handle that error in `g_vasprintf()` to avoid it trying to write to a
`NULL` string allocation and crashing.
So, call `g_vsnprintf()` directly instead of calling
`g_printf_string_upper_bound()`, so that the error case can be handled.
There was already a test for some of this behaviour
(`test_vasprintf_invalid_format_placeholder()`). Because it tested an
invalid format string, the `_g_vsprintf()` call bailed out before
checking whether the buffer it had been passed was `NULL`. The new test
has a valid format string, but an invalid arg.
When running the tests locally, I have disabled the `USE_SYSTEM_PRINTF`
and `HAVE_VASPRINTF` code paths in `g_vasprintf()`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3187
Spotted in https://gitlab.gnome.org/GNOME/glib/-/issues/3187.
In an ideal world, this API would have been designed with an error
return path to begin with — perhaps by returning a `GError` or a
`gssize`. We can’t change the API now, though, which leads to this
slightly awkward “0 indicates an error or success” pattern.
I think that’s justified in this case because:
- This API does not see much use.
- Format strings tend to be literals, and almost always are
non-zero-length, so it tends to be statically possible to determine
that the function won’t return zero on success.
- If callers do need to differentiate the two zero return value cases,
they can just call `g_vsnprintf()` directly instead.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3187
The test needs to call `ftruncate64()` (not `ftruncate()`) to guarantee
it’s using the 64-bit version on Linux, but this doesn’t exist on other
platforms.
Test to see if it exists and, if not, skip the test.
Fixes commit cf5e371c6787747c0440c341407fe5317ea7136b, and fixes CI
failures like https://gitlab.gnome.org/GNOME/glib/-/pipelines/602930.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
The win32-specific APIs in GLib are only defined when building for
Windows. Unfortunately, this means they can not show up in `GLib-2.0.gir`
if it’s build on Linux (which is what mostly happens).
Consequently, that means they’re not present in the API documentation,
which is a bit of a problem.
Make the symbols in the header available on other platforms
(particularly Linux) if building with `g-ir-scanner` (but not
otherwise). This means the win32 symbols show up in `GLib-2.0.gir` and
hence in the documentation.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3037
On certain platforms where file size (off_t) can be truncated when assigning to
gsize, get_contents_regfile() may use the truncated size as the size to read. Add
an explicit check to raise an error in such cases.
G_FILE_ERROR_FAILED will be raised in this case, aligning with behavior for other
cases.
This generally affects 32-bit non-Win32 platforms.
Just as they were ignored under gtk-doc, ignore them when scanning the
sources using `g-ir-scanner`, and make it look at the full function
declarations instead.
This fixes inclusion of functions like `g_creat()` in the documentation.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3037
This allows the methods for `GDir` to be introspected. While we don’t
expect languages which use the introspection bindings to use `GDir`,
full introspection support is necessary for the `GDir` documentation to
be correctly built with gi-docgen.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3037
All GLib tests normally respect `TMPDIR` for writing their temporary
files to. The utils test, however, contained a hack which overwrote
`TMPDIR` so that it could test the behaviour of `g_get_tmp_dir()` for
regressions.
Unfortunately, that hack affected the whole `utils` test suite, not just
the one regression test.
Use the new `g_test_trap_subprocess_with_envp()` API to allow the
regression test to be run as a subprocess with its environment modified,
which allows us to remove the hack affecting the rest of the test suite.
Spotted in
https://gitlab.gnome.org/GNOME/glib/-/issues/3179#note_1925161.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
This is a variant of `g_test_trap_subprocess()` which allows the
environment for the child process to be specified. This is quite useful
when you want to test code which reads environment variables, as it’s
not safe to set those after the start of `main()`.
This will be useful within and outwith GLib for testing such code.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1618
The problem is that resetting the environment of a
proccess is not safe, this is mentioned in the man
pages of the setenv, and also by the concept:
when we get a constant string returned by the getenv,
we need to be guaranteed that this string will exist
during all that time we are using it. While setting
the same env var to another value destroys this string
inside of libc.
Now looking at the should_drop_message(), we can see
that it just gets the environment on every call. Getting
the environment is safe, but however there's no safe way
to switch the logging domains selection: setenv can't be
used because of the reasons listed above.
That is why g_log_writer_default_set_debug_domains() is
needed: it's just a safe replacement for the setenv in this case.
Now should_drop_message() still reads the environment, but
does so only on the first call, after that the domains are
stored internally, and can only be changed by
g_log_writer_default_set_debug_domains().
This also means that now resetting G_MESSAGES_DEBUG env
var in runtime has no effect. But in any case this is not
what the user should do, because resetting the environment
in runtime is not correct.
We presumably want the fallback string used when we cannot determine
the program name to contain balanced angle brackets, as it did before
commit 7098250e.
Fixes: 7098250e "gutils: avoid race setting prgname from g_option_context_parse()/g_application_run()"
Signed-off-by: Simon McVittie <smcv@collabora.com>
g_option_context_parse()/g_application_run()/g_test_init() for
convenience also call g_set_prgname(), when the prgname is unset at this
point. This was racy.
Fix the race by using an atomic compare-and-exchange and only reset the
value, if it is unset still.
g_set_application_name() guards against being reset, but it doesn't
remember whether it was set, it only checks whether g_application_name
was set to non-NULL. When allowing g_set_application_name(NULL) that leads
to odd behaviors, like:
g_set_application_name(NULL);
g_set_application_name("foo");
would not warn.
Disallow that and assert against a NULL application_name.
Note that application_name argument is also not marked as "(nullable)".
Modify all the similar Python test wrappers to set
`G_DEBUG=fatal-warnings` in the environment of the program being tested,
so we can catch unexpected warnings/criticals.
Adding this because I noticed it was missing, not because I noticed a
warning/critical was being ignored.
Signed-off-by: Philip Withnall <philip@tecnocode.co.uk>
Mention that ready time being equal to the current time means the source
will fire immediately.
Related to https://gitlab.gnome.org/GNOME/glib/-/issues/3148
Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
Instead of tracking a "(guint,GSource*)" tuple in the "context->sources"
dictionary, only track pointers to the "source_id".
With this we use the GHashTable as Set (g_hash_table_add()), which is
optimized and avoids storing a separate value array.
It's simple enough to do, because there are literally 5 references to
"context->sources". It's easy to review those usages and reason that the
handling is correct.
While at it, in g_main_context_find_source_by_id() move the check for
SOURCE_DESTROYED() inside the lock. It's not obvious that doing this
without a lock was correct in every case. But doing the check with
a lock should be fast enough to not worry about whether it's absolutely
necessary.
We have g_int_hash()/g_int_equal(), which in practice might also work
with with pointers to unsigned integers. However, according to strict
interpretation of C, I think it is not valid to conflate the two.
Even if it were valid in all cases that we want to support, we should
still have separate g_uint_{hash,equal} functions (e.g. by just #define
them to their underlying g_int_{hash,equal} implementations).
Add instead internal hash/equal functions for guint.
Previously thread-pool-slow ran a single test which encoded a state
machine and polling timer to run 8 different sub-tests and check for
their exit conditions.
This was a bit ugly, and since the timer ran at 1s granularity, several
of the tests completed quite fast and then hang around for most of 1s
before finishing and moving to the next test.
Split the test functions up into separate GTest tests, and split the
state machine up between the test functions. All of the `GMainLoop`
handling is actually only needed for the `test_threadpool_idle_time()`
test.
This reduces the overall test runtime from 36s to 19s on my machine,
with 17s of that being spent in `test_threadpool_idle_time()`.
Signed-off-by: Philip Withnall <philip@tecnocode.co.uk>
Helps: #2810
This makes things a bit more maintainable, as there’s less global state
to worry about.
It introduces no functional changes.
Signed-off-by: Philip Withnall <philip@tecnocode.co.uk>
The race was already acknowledged in the code (via `last_failed`): the
thread pool starts dequeuing jobs as soon as it’s created, so it’s
dequeuing the sorted thread IDs while they’re still being enqueued and
sorted. This can lead to them being dequeued out of the expected order
if new thread IDs are enqueued out of order, which is possible because
they’re randomly generated.
The test tried to handle this by allowing one out-of-order dequeue, but
it looks like the race can race hard enough that multiple out-of-order
dequeues are possible.
Fix that by only starting to dequeue the jobs from the thread pool once
they’ve all been enqueued and put in a total order.
Signed-off-by: Philip Withnall <philip@tecnocode.co.uk>
Fixes: #2810
If the string of one log domain is contained in
another, it was printing both.
For example, if G_MESSAGES_DEBUG is "Gtkspecial",
it would also keep the logs of the "Gtk" domain
The weak symbol resolution doesn’t seem to work (see
https://gitlab.gnome.org/GNOME/glib/-/jobs/3265405):
```
Undefined symbols for architecture x86_64:
"___lsan_enable", referenced from:
_g_leak_sanitizer_is_supported in gutils.c.o
"___lsan_ignore_object", referenced from:
_g_ignore_leak in gquark.c.o
_g_ignore_leak in gthreadpool.c.o
_g_ignore_leak in gutils.c.o
_g_leak_sanitizer_is_supported in gutils.c.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```
See !3672
Signed-off-by: Philip Withnall <philip@tecnocode.co.uk>
