It worked when I first wrote it, but I broke it during the late stages
of code review. str is already freed here, so this is a use-after-free
vulnerability for starters. It also causes the file saved to be always
empty.
[23/236] Compiling C object 'lib/76b5a...isc@sha/contrib_gvdb_gvdb-reader.c.o'.
../../../../Projects/epiphany/lib/contrib/gvdb/gvdb-reader.c: In function ‘gvdb_table_get_names’:
../../../../Projects/epiphany/lib/contrib/gvdb/gvdb-reader.c:428:27: warning: comparison of integer expressions of different signedness: ‘guint32’ {aka ‘unsigned int’} and ‘gint’ {aka ‘int’} [-Wsign-compare]
428 | else if (parent < n_names && names[parent] != NULL)
| ^
To fix this, we have to change n_names to guint, and then also change
the types of everything it's compared against. This seems to be safe
since none of these should ever be negative.
It's confused both myself and Jan-Michael, when reviewing my changes to
this code. It's weird for the serialize function to take ownership of
the passed FileBuilder. Don't do that.
We can also add a convenience free function.
I'm not auditing all the public functions in this file for precondition
checks, but since I'm adding an async version of this function, it
makes sense to ensure there are matching checks for the sync version.
This is just an async version of gvdb_table_write_contents().
Future work: someone could write an async version of gvdb_table_new(),
then sync I/O would no longer be required to construct a GvdbTable.
Commit 084e1d868 added a preallocation to an array to avoid
reallocations later on, but neglected the fact that after N insertions
into the array, there’s always a NULL terminator added to the end. Fix
the preallocation to include that NULL terminator.
This doesn’t change the correctness of the code, but should eliminate
one reallocation.
Spotted by Sebastian Dröge. See
https://gitlab.gnome.org/GNOME/glib/merge_requests/674.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Since tests/dconf-mock-gvdb.c has functions conflicting with the real
gvdb and it is intended for the former to override the latter in tests,
we have to make functions in gvdb library have weak bindings instead of
the default strong bindings to avoid duplicate symbol errors.
Fixes https://gitlab.gnome.org/GNOME/dconf/issues/47
It should not be unsigned. The type in the on-disk format is gint32, so
we need to return something at least as wide as that. However, we
should not expose the implementation detail that the on-disk format is
specifically gint32. Use a gsize, since that’s the normal type for array
lengths — but check that we’re not on a platform where (somehow) gsize
is smaller than gint32.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
glibc string.h declares memcpy() with attribute(nonnull(1,2)), causing
calls with NULL arguments to be treated as undefined behaviour.
This is consistent with ISO C99 and C11, which state that passing 0
to string functions as an array length does not remove the requirement
that the pointer to the array is a valid pointer.
gcc -fsanitize=undefined catches this while running OSTree's test suite.
Similarly, running the GLib test suite reports similar issues for
qsort(), memmove(), memcmp().
(This is a partial cherry-pick of commit e5ed410c8c from GLib.)
Signed-off-by: Simon McVittie <smcv@debian.org>
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=775510
Reviewed-by: Colin Walters
GVDB is essentially part of GLib, so should have the same maintainer
list.
At least this way, it’s not just maintained by one absentee maintainer.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Drop gvdb_table_new_from_data() and add gvdb_table_new_from_bytes().
Since the underlying backingstore of a GvdbTable is now always
refcounted, drop the refcounting on GvdbTable itself.
The attempt at the simple method for preventing unbounded recursion
proved to be insufficient due to the existence of dconf databases in the
wild that violated the rule (leading to the entire content of the
database being scrapped). It also still had the ugly assert for less
than 64 levels of recursion that could have been hit by a determined
advisary.
gvdb_table_get_names() allows the dconf-service to do everything it
needs without the troubles associated with the walk approach.
Improve the robustness of gvdb-reader in two ways.
First: ensure that the result of gvdb_table_has_value() always agrees
with gvdb_table_get_value(). Those two could disagree in the case that
the value was recorded as existing but pointed to an out-of-bounds
region.
Second: prevent gvdb_table_walk() from getting stuck in finite loops due
to self-referential directories.
Our hashing of non-ASCII strings was undefined due to the fact that
'char' is signed on some platforms, unsigned on others. Always use a
signed char.
Discovered by Alexander Larsson.
https://bugzilla.gnome.org/show_bug.cgi?id=658806
