It is cleaner to define glib_typeof() in a header included after
gversionmacros.h so we can use GLIB_VERSION_MIN_REQUIRED directly
instead of doing it everywhere glib_typeof() is used.
Rather than always writing to `out.xbel` in the build directory, which
could cause issues when running tests in parallel, or expecting the
tests to not touch the build directory.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Rewrite bookmark_item_dump to not crash if any of
the timestamps is NULL. Also, avoid some of the gratitious
extra string copying.
Tweaked by Philip Withnall to pass the unit tests.
Include the base URI in the `g_test_bug()` calls instead. This resolves
inconsistencies between the old bug base (bugzilla.gnome.org) and the
new bug base (gitlab.gnome.org). It also has the advantage that the URI
passed to `g_test_bug()` is now clickable in the code editor, rather
than being split across two locations.
See https://gitlab.gnome.org/GNOME/glib/-/merge_requests/275#note_303175
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Currently, `g_atomic_ref_count_dec ()` does two operations.
We try to get this down to one operation.
Replace `g_atomic_int_dec_and_test ()` with `g_atomic_int_add ()`
Passing -1 as value.
Note: changes current behaviour, checks are now done after decrement.
This seems non-obvious to me. Document it.
It also seems important to know, because it means that the data pointer
might already be destroyed, before the source is unreferenced for good.
For example, if the data pointer keeps a reference to the GSource,
and it might seem sensible to call:
g_source_destroy(data->source);
g_source_unref(data->source); /* <<< data is already destroyed? */
This leads to a crash, if the source was attached to a context.
Currently, `g_atomic_ref_count_inc ()` does three operations.
We try to get this down to one operation.
Fixed by replacing `g_atomic_int_inc ()` with `g_atomic_int_add ()`
which increments by 1 and returns old value
Note: changes current behaviour, checks are now done after increment.
Closes: #1583
SHGetSpecialFolderLocation() can cause 200ms delays when getting some
folder locations.
Also, use SHGetKnownFolderPath() normally, without loading it at runtime,
since GLib is Windows 7-or-later now.
Fixes#2397
The actually parsed `@bytes` are not used because the only caller
does not provide an output parameter to request them. So this bug had
no effect in practice.
This allows introspection to properly handle them as GPatternSpec
methods, as per this deprecate g_pattern_match() and
g_pattern_match_string() functions.
Let the compiler figure out the size, rather than hard-coding it the
same as the struct definition.
This makes no functional changes.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
`#define G_GNUC_NORETURN __attribute__((__noreturn__))` was previously under
`#if G_GNUC_CHECK_VERSION(2, 4)`.
But `# define G_NORETURN __attribute__ ((__noreturn__))` was under
`#if G_GNUC_CHECK_VERSION(2, 8) || (0x5110 <= __SUNPRO_C)`.
Take the latter for `g_macro__has_attribute(__noreturn__)` fallback. This will
disable `G_GNUC_NORETURN` for GCC 2.4-2.7 though.
Fall back to compiler version checks only when `__has_attribute()` is not
available.
clang-cl doesn't define `__GNU__`, but still accepts attributes. This change
gets rid of a lot of warnings when building GLib with clang-cl. For GCC and
non-cl Clang nothing should change.
glib/gstring.c: In function ‘g_string_replace’:
glib/gstring.c:998:13: warning: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
998 | if (n == limit)
| ^~
Calling g_atomic_ref_count_dec() or g_ref_count_dec() and the reference
count reaches zero results in different side effects depending on
whether the reference count is atomic or not.
The intended side effect when this happens is undefined, i.e. one should
not rely on the reference count actually reaching zero, or staying 1, or
becoming something else, and one should treat the grefcount /
gatomicrefcount to be unusable until reinitialized.
This wasn't documented, so add a paragraph about this.
This change is proposed by Kamil Dudka. It teaches Coverity to assume
that g_critical() will never return, which is desirable for the same
reasons it is for scan-build: once you've triggered undefined behavior,
the game is already lost, and there's limited benefit from trying to
avoid every possible memory leak on such codepaths. Notably, this
affects g_return_if_fail().
Arguably it might be desirable to fix every such issue, but if we're
already not doing so for scan-build, it doesn't make sense to hold
developers working with Coverity to a higher standard. This allows
focusing on more serious issues found by Coverity.
Thanks Kamil!
glib/gdate.c: In function ‘convert_twodigit_year’:
glib/gdate.c:1217:13: warning: comparison of integer expressions of
different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
1217 | if (y < two)
| ^
When using TAP we want every single line to be one of the following:
- a valid TAP clause
- a comment
- a blank line
Typical explicit test logs are single line comments, but in some cases
we might end up printing debug messages from libraries, and those may
contain multiple lines. When that happens, we break the TAP and fail the
test in conditions entirely outside of our control.
One option to avoid outright failure is to always prepend each line of a
messge with `#`, to ensure that the whole thing is considered a comment.
Resolve "g_date_time_format() does not return UTF-8 if LC_TIME is not UTF8 but other locale settings are UTF-8"
Closes#2055
See merge request GNOME/glib!1777
Use the GCC/Clang macros __has_extension() and __has_feature() on
c_static_assert, which is the documented way for GCC/CLang to check for
_Static_assert if C11 mode is not enabled, as suggested by Aleksandr Mezin.
As a result, add a private macro that is defined to be __has_extension if it
exists, otherwise it is considered to be always false.
It appears that CLang supports _Static_assert() even when not in C11
mode, since at least CLang 3.1, so let's just use that for CLang builds.
Fixes issue #2338.
Functions (_g_get_time_charset and _g_get_ctype_charset) to get LC_TIME and LC_CTYPE charset
by using nl_langinfo with _NL_TIME_CODESET and CODESET).
Another functions (_g_locale_time_to_utf8 and _g_locale_ctype_to_utf8) which uses thel and format
the input string accordingly.
Add new test cases with mixing UTF8 and non UTF8 LC_TIME along with UTF8
and non UTF8 LC_MESSAGES.
Closed#2055
Signed-off-by: Frederic Martinsons <frederic.martinsons@sigfox.com>
If the same `GKeyFile` is reused to load multiple different key files,
any loads after the first which encounter translated keys will crash,
because clearing the data from the first load cleared the cached
language names, but didn’t clear `checked_locales`, so they were never
reloaded.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2361
It should not be possible for `->locales` to be set without
`->checked_locales` being set, so drop the redundant check. This helps
with branch code coverage.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This is basically glnx_steal_fd() from libglnx. We already had two
private implementations of it in GLib.
Signed-off-by: Simon McVittie <smcv@collabora.com>
The (out caller-allocates) and (out callee-allocates) annotations are
meant for structured or pointer types. Plain old data types are just
regular out parameters and don't need the annotation about who
allocates them.
See: https://gitlab.gnome.org/GNOME/gjs/-/issues/386
In the 2.68 cycle we’d added 3 new enumerator elements. Due to the
preceding commit, they can now be annotated with
`GLIB_AVAILABLE_ENUMERATOR_IN_2_68`, which will make it a bit easier for
third party projects to notice when they’re using these symbols without
having bumped their GLib dependency.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2327
The changes in 4273c43902 did not guard
macros in `gatomic.h` which use `glib_typeof`. This meant that when
552b8fd862 was committed, moving the
include of `<type_traits>` under such a guard, these macros were still
trying to use it. This broke the build of at least vte.
Fix this by guarding the API break in `gatomic.h` too.
The doc used different phrasing for the same thing, e.g. "if any thread"
vs "any other thread."
Also make it clear that trying to take a write lock while already having
a lock, or trying to take a read lock while having a write lock, is
undefined.
When included inside an `extern "C"` block, this causes build failures
that look something like:
/usr/include/c++/10/type_traits:2930:3: error: template with C linkage
2930 | template<typename _Fn, typename... _Args>
| ^~~~~~~~
../../disas/arm-a64.cc:20:1: note: ‘extern "C"’ linkage started here
20 | extern "C" {
| ^~~~~~~~~~
Commit 4273c43902 made this opt in for
projects which are defining `GLIB_VERSION_MIN_REQUIRED`, but the include
of `<type_traits>` via `gmacros.h` was not included in this. If we move
the include out to the places where `glib_typeof` is called, we can make
it covered by this macro too, and save a few consumers from FTBFSing.
That also means that, if you don't want to fix your use of the headers,
and as long as this version is sufficient for you, a quick workaround is
to define `GLIB_VERSION_MIN_REQUIRED` to `GLIB_VERSION_2_66` or lower.
Suggested by Simon McVittie.
Alternative to: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1935
Fixes: https://gitlab.gnome.org/GNOME/glib/-/issues/2331
The `source_fds`/`target_fds` functionality is not supported on Windows
at the moment.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2097
This should introduce no functional changes, but condenses the variants
of the internal spawn implementation down to be more like `gspawn.c`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2097
I realise Windows uses handles rather than PIDs, but given that there
are multiple platform-specific implementations of the public
`g_spawn_*()` API, I think it is less confusing for them all to use the
same naming scheme.
This introduces no functional changes.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This is a simple wrapper around the new source/target FD mapping
functionality in `fork_exec()`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2097
If `stdout_fd` was set to (say) 6, and `stderr_fd` was set to 1, the
`set_cloexec()` call for setting up `stderr` would set the new `stdout`
for the forked process to be closed in the pending `exec()`.
This would cause the child process to error when writing to `stdout`.
This situation happens when using `G_SUBPROCESS_FLAGS_STDERR_MERGE`.
Add some conditions to prevent setting `CLOEXEC` in such cases.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2097
It was previously possible to specify the FD number which
`child_err_report_fd` was assigned, as a target FD in the FD mapping set
up using `g_subprocess_launcher_take_fd()`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2097
This effectively moves some of the functionality of `GSubprocess`
(`g_subprocess_launcher_take_fd()`) into `g_spawn*()`, which should make
implementation a little simpler.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2097
This is an internal change which won’t affect the public API. It should
introduce no functional changes, but simplifies the code a little.
The arguments from `fork_exec_with_pipes()` have been added to
`fork_exec_with_fds()`. `child_close_fds` has been dropped since it’s
now an implementation detail within the function.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2097
Since !1715, g_atomic_pointer_get (&x) has usually returned the type of
x, rather than a generic pointer, in C++ code (where x is any pointer,
or any pointer-sized integer such as guintptr). glib/tests/cxx.cpp
asserts that this is the case.
However, this was only implemented for the lock-free fast-path, not
for the slow path used in platforms with an ARMv5 baseline (and
therefore no atomic instructions) such as Debian armel.
Signed-off-by: Simon McVittie <smcv@collabora.com>
GByteArray uses guint for storing the length of the byte array, but it
also has a constructor (g_byte_array_new_take) that takes length as a
gsize. gsize may be larger than guint (64 bits for gsize vs 32 bits
for guint). It is possible to call the function with a value greater
than G_MAXUINT, which will result in silent length truncation. This
may happen as a result of unreffing GBytes into GByteArray, so rather
be loud about it.
(Test case tweaked by Philip Withnall.)
This adds g_string_replace(), a function that replaces instances of one string
with another in a GString. It allows the caller to specify the maximum number
of replacements to perform, and returns the number of replacements performed
to the caller.
Fixes: #225
Unfortunately, `g_memdup()` accepts its size argument as a `guint`,
unlike most other functions which deal with memory sizes — they all use
`gsize`. `gsize` is 64 bits on 64-bit machines, while `guint` is only 32
bits. This can lead to a silent (with default compiler warnings)
truncation of the value provided by the caller. For large values, this
will result in the returned heap allocation being significantly smaller
than the caller expects, which will then lead to buffer overflow
reads/writes.
Any code using `g_memdup()` should immediately port to `g_memdup2()` and
check the pointer arithmetic around their call site to ensure there
aren’t other overflows.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2319
The public API `GIOChannel.line_term_len` is only a `guint`. Ensure that
nul-terminated strings passed to `g_io_channel_set_line_term()` can’t
exceed that length. Use `g_memdup2()` to avoid a warning (`g_memdup()`
is due to be deprecated), but not to avoid a bug, since it’s also
limited to `G_MAXUINT`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2319
Convert all the call sites which use `g_memdup()`’s length argument
trivially (for example, by passing a `sizeof()` or an existing `gsize`
variable), so that they use `g_memdup2()` instead.
In almost all of these cases the use of `g_memdup()` would not have
caused problems, but it will soon be deprecated, so best port away from
it
In particular, this fixes an overflow within `g_bytes_new()`, identified
as GHSL-2021-045 by GHSL team member Kevin Backhouse.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: GHSL-2021-045
Helps: #2319
This will replace the existing `g_memdup()` function, which has an
unavoidable security flaw of taking its `byte_size` argument as a
`guint` rather than as a `gsize`. Most callers will expect it to be a
`gsize`, and may pass in large values which could silently be truncated,
resulting in an undersize allocation compared to what the caller
expects.
This could lead to a classic buffer overflow vulnerability for many
callers of `g_memdup()`.
`g_memdup2()`, in comparison, takes its `byte_size` as a `gsize`.
Spotted by Kevin Backhouse of GHSL.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: GHSL-2021-045
Helps: #2319
Various tests have leaks where it isn't clear whether the data is
intentionally not freed, or leaked due to a bug. If we mark these
tests as TODO, we can skip them under AddressSanitizer and get the
rest to pass, giving us a baseline from which to avoid regressions.
Signed-off-by: Simon McVittie <smcv@collabora.com>
AddressSanitizer detects memory leaks, NULL parameters where only a
non-NULL parameter is expected, and other suspicious behaviour, so if
we try to test that sort of thing we can expect it to fail.
Signed-off-by: Simon McVittie <smcv@collabora.com>
We preallocate buffers that are used after forked. That is because
malloc()/free() are not async-signal-safe and must not be used between
fork() and exec().
However, for the child process that exits without fork, valgrind wrongly
reports these buffers as leaked.
That can be suppressed with "--child-silent-after-fork=yes", but it is
cumbersome.
Work around by trying to allocate the buffers on the stack. At
least in the common cases where the pointers are small enough
so that we can reasonably do that.
If the buffers happen to be large, we still allocate them on the heap
and the problem still happens. Maybe we could have also allocated them
as thread_local, but currently glib doesn't use that.
[smcv: Cosmetic adjustments to address review comments from pwithnall]
For manual test coverage that would reproduce the bug fixed in !1902,
copy /bin/true (or any other harmless executable) to
/usr/bin/spawn-test-helper.
Signed-off-by: Simon McVittie <smcv@collabora.com>
do_exec() and g_execute() rely on being passed a NULL search path
if we intend to avoid searching the PATH, but since the refactoring
in commit 62ce66d4, this was never done. This resulted in some spawn
calls searching the PATH when it was not intended.
Spawn calls that go through the posix_spawn fast-path were unaffected.
The deprecated gtester utility, as used in GTK 3, relies on the
ability to run an executable from the current working directory by
omitting the G_SPAWN_SEARCH_PATH flag. This *mostly* worked, because
our fallback PATH ends with ".". However, if an executable of the
same name existed in /usr/bin or /bin, it would run that instead of the
intended test: in particular, GTK 3's build-time tests failed if
ImageMagick happens to be installed, because gtester would accidentally
run display(1) instead of testsuite/gdk/display.
Fixes: 62ce66d4 "gspawn: Don’t use getenv() in async-signal-safe context"
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=977961
That changes the return type of functions like g_object_ref() that can
break C++ applications like Webkit. Note that it is not an ABI break.
It must thus be opt-in the same way we did when adding this to
g_object_ref() for GNU C compilers in the first place. Unfortunately it
cannot be done directly in gmacros.h because GLIB_VERSION_2_68 is not
defined there, and gversionmacros.h cannot be included there because
there is some strict ordering in which those headers must be included.
This means that applications that does not define
GLIB_VERSION_MIN_REQUIRED will still get an API break, so we encourage
them to declare their minimum requirement to avoir such issues in the
future too.
clang++ checks the 2nd args of __atomic_compare_exchange_n() has the
same type as the first, which fails when 2nd arg is nullptr which is of
type nullptr_t.
Ideally it should do `glib_typeof (*(atomic)) gapcae_oldval = (oldval);`
to ensure oldval and atomic have compatible types but unfortunately that
does not work neither.
Since that function never has been typesafe, and it is not even
attempting to use glib_typeof in case __ATOMIC_SEQ_CST is not defined,
drop it in __atomic_ case too.
Fixes issue #2226.
There are various places glib uses __typeof__ for type safety, but
that's a GNUC extension. C++11 has standard decltype() that does a
similar job, at least for cases we care about.
This avoids C++ code to always have to cast return value of
g_object_ref() which was causing type kind of error:
error: invalid conversion from ‘gpointer’ {aka ‘void*’} to
‘GstElementFactory*’ {aka ‘_GstElementFactory*’} [-fpermissive]
I found myself wanting to know the test that is currently being run,
where e.g. __func__ would be inconvenient to use, because e.g. the place
the string was needed was not in the test case function. Using __func__
also relies on the test function itself containing the whole path, while
loosing the "/" information that is part of the test path.
Since Windows 10 1607, we can make use of SetThreadDescription() API
for setting thread name. Unlike previously used exception based
method, this API will preserve configured thread name on dump file.
This makes it a little easier to link to in the generated documentation,
and separates it from the section above.
Link to the heading from the documentation for
`G_DEFINE_EXTENDED_ERROR`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
On FreeBSD it always crashes due to the platform’s `vasprintf()`
implementation being less forgiving than Linux’s. That’s fine.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This is mostly to cover historic code, but also includes a couple of
additional tests for extended error domains (see #14).
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
