Any ASCII character above 0x7F should be treated as UTF-8 in
ghostutils functions because GLib expects host names to be either
punycode encoded or in valid UTF-8 format.
The checks in gutf8.c already treat 0x80 as non-ASCII, but two checks
in ghostutils.c erroneously check for "great than" not "greater than or
equal to".
Clarify this by adding a new macro which is reused by PUNICODE_IS_BASIC
for better documentation in code.
This commit has the potential to close a thousand Coverity issues.
On generated code from gdbus-codegen, Coverity typically warns on
skeleton->priv->changed_properties_idle_source happening outside
the skeleketon->priv->lock during finalize(), while it's protected
by this mutex in other parts.
Presumably if the object is in finalization, there should be no
other threads poking at it, so the code is actually safe for the
intended use and the warning moot.
To address this, change gdbus-codegen to prefer g_clear_pointer()
with glib >= 2.38 (should be most often the case nowadays) so this
access is reserved to a single line of code, and mark this line of
code with a Coverity code annotation in order to suppress the
warning.
This test specifically checks whether the documented behaviour of
deliberately leaking old special dirs strings (which might still be
pointed to in user code) works.
I haven’t gone back and used this new unit test with an older version of
GLib, but I suspect the ‘deliberate leak’ code hasn’t worked for a
while. See the changes in the previous few commits, which were necessary
to get this unit test to pass.
The previous `test_user_special_dirs()` test has been deleted, as what
it was testing has been entirely subsumed into the new test.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
This introduces no functional changes, it just factors out a helper to
set a mock `user-dirs.dirs` file, so that we can do the same in multiple
tests.
It does add a little more error checking to the helper code though; in
particular it checks that the test is running with
`G_TEST_OPTION_ISOLATE_DIRS`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Otherwise it isn’t set on every code path, and it’s possible for a
caller to receive `NULL` when they call `g_get_user_special_dir
(G_USER_DIRECTORY_DESKTOP)`, i.e. after calling
`g_reload_user_special_dirs_cache()`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
It seems it wasn’t behaving as advertised: it was freeing all the old
string values unless they were strcmp-equal to the new ones, in which
case the new ones were discarded.
What’s actually documented is that the old values are always leaked,
unless they’re strcmp-equal to the new ones.
Adjust the code to match the documentation. A unit test will be added in
a following commit.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Found by the oss-fuzz test for this parser,
`fuzzing/fuzz_special_dirs.c`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
oss-fuzz#445870121
oss-fuzz#445848222
Found with `meson test --setup valgrind`, although I also had to change
relevant lines in `glib/tests/meson.build` temporarily to avoid the
other tests in the file taking forever:
```
'utils' : {
'c_standards': c_standards.keys(),
'args': ['-p', '/utils/user-special-dirs'],
},
```
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
`utils-isolated.c` is meant for testing the behaviour of
`G_TEST_OPTION_ISOLATE_DIRS`, not testing the general behaviour of
`gutils.c`. My mistake for not noticing this at code review time when
the test was added.
This introduces no functional changes, just moves a test around.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Another test isolated.
This does mean moving one of the test cases from the suite out into a
separate suite, as it explicitly relies on running without
`G_TEST_OPTION_ISOLATE_DIRS`, and I think that makes sense. The other
test cases run fine when isolated.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
If the build/test machine is slow, heavily-loaded or otherwise
inconvenienced, it might take a few seconds for the signal to be sent
by the subprocess, received by the message bus, re-broadcasted by the
message bus and received by the test code. Wait a few more seconds
before giving up.
If this test is successful, increasing this timeout will not slow it
down: we stop waiting for the signal as soon as we receive it. This will
only make any difference if the test would have failed.
Signed-off-by: Simon McVittie <smcv@debian.org>
We have two things happening in parallel:
1. The GDBus worker thread writes out an AddMatch call to the socket,
the message bus reads that method call from the other end of the
socket, and the message bus responds by adding the match rule
for the signal subscription
2. The main thread forks, and the child execs
gdbus-connection-flush-helper, which sends the signal that we are
expecting; the message bus receives that signal, and broadcasts it
to subscribers, if any
Normally (1.) wins the race because exec'ing a child process is more
time-consuming than IPC, and the test passes.
However, it is possible for (2.) to win the race. If so, we will never
receive the expected signal (because it was received by the message bus
before the AddMatch() method call, so at the time it was received, the
test was not yet a subscriber); the test waits until the timeout and
then gives up, and the test fails.
For whatever reason, Debian's s390x buildd seems to be reliably failing
this test since this week, having previously passed. I don't know what
changed. I can (very rarely) reproduce the race condition described
above on a developer-accessible s390x machine by repeatedly running the
/gdbus/connection/flush test in a loop.
Bug-Debian: https://bugs.debian.org/1115617
Signed-off-by: Simon McVittie <smcv@debian.org>
The functions g_strconcat, g_strjoinv and g_strjoin perform the
concatination of strings in two phases. The first phase figures out the
required amount of memory to hold the resulting string. The second
phase actually copies the strings into the allocated memory.
If the sum of the lengths of all strings to be joined exceeds G_SIZEMAX,
then phase two triggers an out of boundary write due to insufficient
amount of memory allocated.
While this sounds impossible to do at first, actually it becomes a
possibility on 32 bit systems with merely 20 MB of heap. The overflow
can actually happen if the same string is joined multiple times. See
attached unit test. While the same can be done with 64 bit systems, it
takes much more memory and a lot of time.
Fortunately the protection is rather cheap, although it adds two or
three machine instructions and branches due to testing.
The load_user_special_dirs function performs no internal locking, which
means that callers must already hold the g_utils_global lock. Since we
mark some getters as unlocked by now, do the same with
load_user_special_dirs to highlight this additional requirement.
Suggested by Michael Catanzaro
The list pointer is allowed to be null while still creating a new valid list.
The missing "nullable" flagging can cause issues in gir generated wrappers.
See https://github.com/gircore/gir.core/issues/1318
List of functions with changed comments:
- g_list_append
- g_list_prepend
- g_list_insert
- g_list_insert_before
- g_slist_append
- g_slist_prepend
- g_slist_insert
- g_slist_insert_before
...C, C++, ObjC, and ObjC++ too. It's not a problem to add
arguments to unused languages.
This means, for example, that all the defines are in place when writing
WinRT C++ code.
