From 7683c2b7968126c5d9c1a0163b69d7acaa7f0a53 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Mar 2012 11:26:49 -0500 Subject: [PATCH 01/19] builder: Note don't actually implement bloom filters --- gvdb-builder.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gvdb-builder.c b/gvdb-builder.c index 91adec673..c63d1171e 100644 --- a/gvdb-builder.c +++ b/gvdb-builder.c @@ -340,6 +340,13 @@ file_builder_allocate_for_hash (FileBuilder *fb, #undef chunk memset (*bloom_filter, 0, n_bloom_words * sizeof (guint32_le)); + + /* NOTE - the code to actually fill in the bloom filter here is missing. + * Patches welcome! + * + * http://en.wikipedia.org/wiki/Bloom_filter + * http://0pointer.de/blog/projects/bloom.html + */ } static void From 4e77b52ad8fd5342978fd8be6aff19f13beda0ef Mon Sep 17 00:00:00 2001 From: Ryan Lortie Date: Thu, 5 Jul 2012 19:13:58 -0400 Subject: [PATCH 02/19] gvdb-reader: correct whitespace damage --- gvdb-reader.c | 66 +++++++++++++++++++++++++-------------------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/gvdb-reader.c b/gvdb-reader.c index dfdb4479f..81b8e6b86 100644 --- a/gvdb-reader.c +++ b/gvdb-reader.c @@ -128,13 +128,13 @@ gvdb_table_setup_root (GvdbTable *file, static GvdbTable * new_from_data (const void *data, - gsize data_len, - gboolean trusted, - gpointer user_data, - GvdbRefFunc ref, - GDestroyNotify unref, - const char *filename, - GError **error) + gsize data_len, + gboolean trusted, + gpointer user_data, + GvdbRefFunc ref, + GDestroyNotify unref, + const char *filename, + GError **error) { GvdbTable *file; @@ -163,15 +163,15 @@ new_from_data (const void *data, else { - if (filename) - g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_INVAL, - "%s: invalid header", filename); - else - g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_INVAL, - "invalid gvdb header"); + if (filename) + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_INVAL, + "%s: invalid header", filename); + else + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_INVAL, + "invalid gvdb header"); g_slice_free (GvdbTable, file); - if (unref) - unref (user_data); + if (unref) + unref (user_data); return NULL; } @@ -213,13 +213,13 @@ gvdb_table_new (const gchar *filename, return NULL; return new_from_data (g_mapped_file_get_contents (mapped), - g_mapped_file_get_length (mapped), - trusted, - mapped, - (GvdbRefFunc)g_mapped_file_ref, - (GDestroyNotify)g_mapped_file_unref, - filename, - error); + g_mapped_file_get_length (mapped), + trusted, + mapped, + (GvdbRefFunc)g_mapped_file_ref, + (GDestroyNotify)g_mapped_file_unref, + filename, + error); } /** @@ -242,18 +242,18 @@ gvdb_table_new (const gchar *filename, **/ GvdbTable * gvdb_table_new_from_data (const void *data, - gsize data_len, - gboolean trusted, - gpointer user_data, - GvdbRefFunc ref, - GDestroyNotify unref, - GError **error) + gsize data_len, + gboolean trusted, + gpointer user_data, + GvdbRefFunc ref, + GDestroyNotify unref, + GError **error) { return new_from_data (data, data_len, - trusted, - user_data, ref, unref, - NULL, - error); + trusted, + user_data, ref, unref, + NULL, + error); } static gboolean @@ -618,7 +618,7 @@ gvdb_table_unref (GvdbTable *file) if (g_atomic_int_dec_and_test (&file->ref_count)) { if (file->unref_user_data) - file->unref_user_data (file->user_data); + file->unref_user_data (file->user_data); g_slice_free (GvdbTable, file); } } From ae3d42c60fe0f0f2be06b68b67a1a6bd795fec5d Mon Sep 17 00:00:00 2001 From: Ryan Lortie Date: Thu, 5 Jul 2012 19:17:44 -0400 Subject: [PATCH 03/19] gvdb-reader: refuse to open file with small header Clean up the logic for dealing with invalid headers and include the case where the file is too small to contain a fully-formed header. --- gvdb-reader.c | 54 ++++++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/gvdb-reader.c b/gvdb-reader.c index 81b8e6b86..c04630d9e 100644 --- a/gvdb-reader.c +++ b/gvdb-reader.c @@ -136,6 +136,7 @@ new_from_data (const void *data, const char *filename, GError **error) { + const struct gvdb_header *header; GvdbTable *file; file = g_slice_new0 (GvdbTable); @@ -147,39 +148,40 @@ new_from_data (const void *data, file->unref_user_data = unref; file->user_data = user_data; - if (sizeof (struct gvdb_header) <= file->size) - { - const struct gvdb_header *header = (gpointer) file->data; + if (file->size < sizeof (struct gvdb_header)) + goto invalid; - if (header->signature[0] == GVDB_SIGNATURE0 && - header->signature[1] == GVDB_SIGNATURE1 && - guint32_from_le (header->version) == 0) - file->byteswapped = FALSE; + header = (gpointer) file->data; - else if (header->signature[0] == GVDB_SWAPPED_SIGNATURE0 && - header->signature[1] == GVDB_SWAPPED_SIGNATURE1 && - guint32_from_le (header->version) == 0) - file->byteswapped = TRUE; + if (header->signature[0] == GVDB_SIGNATURE0 && + header->signature[1] == GVDB_SIGNATURE1 && + guint32_from_le (header->version) == 0) + file->byteswapped = FALSE; - else - { - if (filename) - g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_INVAL, - "%s: invalid header", filename); - else - g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_INVAL, - "invalid gvdb header"); - g_slice_free (GvdbTable, file); - if (unref) - unref (user_data); + else if (header->signature[0] == GVDB_SWAPPED_SIGNATURE0 && + header->signature[1] == GVDB_SWAPPED_SIGNATURE1 && + guint32_from_le (header->version) == 0) + file->byteswapped = TRUE; - return NULL; - } + else + goto invalid; - gvdb_table_setup_root (file, &header->root); - } + gvdb_table_setup_root (file, &header->root); return file; + +invalid: + if (filename) + g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_INVAL, "%s: invalid header", filename); + else + g_set_error_literal (error, G_FILE_ERROR, G_FILE_ERROR_INVAL, "invalid gvdb header"); + + g_slice_free (GvdbTable, file); + + if (unref) + unref (user_data); + + return NULL; } /** From a9551ccf92fb046a92573fb4a5b42a4d43e2abec Mon Sep 17 00:00:00 2001 From: Ryan Lortie Date: Thu, 5 Jul 2012 23:14:55 -0400 Subject: [PATCH 04/19] more whitespace fixes --- gvdb-reader.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gvdb-reader.h b/gvdb-reader.h index e6878c3f0..2a50c10fb 100644 --- a/gvdb-reader.h +++ b/gvdb-reader.h @@ -36,12 +36,12 @@ GvdbTable * gvdb_table_new (const g GError **error); G_GNUC_INTERNAL GvdbTable * gvdb_table_new_from_data (const void *data, - gsize data_len, + gsize data_len, gboolean trusted, - gpointer user_data, - GvdbRefFunc ref, - GDestroyNotify unref, - GError **error); + gpointer user_data, + GvdbRefFunc ref, + GDestroyNotify unref, + GError **error); G_GNUC_INTERNAL GvdbTable * gvdb_table_ref (GvdbTable *table); G_GNUC_INTERNAL From 374cb1bc87a64efe48f3aac2b0c71c395792d42d Mon Sep 17 00:00:00 2001 From: Ryan Lortie Date: Fri, 6 Jul 2012 21:42:04 -0400 Subject: [PATCH 05/19] gvdb-reader: robustness improvements 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. --- gvdb-reader.c | 47 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/gvdb-reader.c b/gvdb-reader.c index c04630d9e..b41fb5d15 100644 --- a/gvdb-reader.c +++ b/gvdb-reader.c @@ -457,7 +457,15 @@ gboolean gvdb_table_has_value (GvdbTable *file, const gchar *key) { - return gvdb_table_lookup (file, key, 'v') != NULL; + static const struct gvdb_hash_item *item; + gsize size; + + item = gvdb_table_lookup (file, key, 'v'); + + if (item == NULL) + return FALSE; + + return gvdb_table_dereference (file, &item->value.pointer, 8, &size) != NULL; } static GVariant * @@ -699,23 +707,38 @@ gvdb_table_walk (GvdbTable *table, item = gvdb_table_get_item (table, *pointers[index]++); start_here: - if (item != NULL && - (name = gvdb_table_item_get_key (table, item, &name_len))) + if (item != NULL && (name = gvdb_table_item_get_key (table, item, &name_len))) { if (item->type == 'L') { - if (open_func (name, name_len, user_data)) + const guint32_le *dir; + guint length; + + if (gvdb_table_list_from_item (table, item, &dir, &length)) { - guint length; + gint i; - index++; - g_assert (index < 64); + /* In order to avoid files with recursive contents + * we impose the rule that a directory's data must + * follow the data of any directory pointing to + * it. + * + * If we discover that our newly-discovered + * directory follows the one we're traversing now + * then bail out. + */ + if (dir <= pointers[index]) + continue; - gvdb_table_list_from_item (table, item, - &pointers[index], - &length); - enders[index] = pointers[index] + length; - name_lengths[index] = name_len; + if (open_func (name, name_len, user_data)) + { + index++; + g_assert (index < 64); + + name_lengths[index] = name_len; + pointers[index] = dir; + enders[index] = pointers[index] + length; + } } } else if (item->type == 'v') From 82cbc59297f08865e38c5527954ae4c4e27cc0e1 Mon Sep 17 00:00:00 2001 From: Ryan Lortie Date: Sun, 8 Jul 2012 10:44:05 -0400 Subject: [PATCH 06/19] remove a stray variable This was used to implement a much more complicated check which was dismissed in favour of the simple 'must follow in file' approach. --- gvdb-reader.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/gvdb-reader.c b/gvdb-reader.c index b41fb5d15..b81aa56af 100644 --- a/gvdb-reader.c +++ b/gvdb-reader.c @@ -716,8 +716,6 @@ gvdb_table_walk (GvdbTable *table, if (gvdb_table_list_from_item (table, item, &dir, &length)) { - gint i; - /* In order to avoid files with recursive contents * we impose the rule that a directory's data must * follow the data of any directory pointing to From d9577f100bf44f7d3e38b6800a470c46d96a9f50 Mon Sep 17 00:00:00 2001 From: Ryan Lortie Date: Mon, 9 Jul 2012 14:32:22 -0400 Subject: [PATCH 07/19] gvdb-reader.c: add gvdb_table_get_names() This function lists off all names that appear within a particular hash. --- gvdb-reader.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++ gvdb-reader.h | 4 +- 2 files changed, 152 insertions(+), 1 deletion(-) diff --git a/gvdb-reader.c b/gvdb-reader.c index b81aa56af..a8531af2d 100644 --- a/gvdb-reader.c +++ b/gvdb-reader.c @@ -378,6 +378,155 @@ gvdb_table_list_from_item (GvdbTable *table, return TRUE; } +/** + * gvdb_table_get_names: + * @table: a #GvdbTable + * @length: the number of items returned, or %NULL + * + * Gets a list of all names contained in @table. + * + * No call to gvdb_table_get_table(), gvdb_table_list() or + * gvdb_table_get_value() will succeed unless it is for one of the + * names returned by this function. + * + * Note that some names that are returned may still fail for all of the + * above calls in the case of the corrupted file. Note also that the + * returned strings may not be utf8. + * + * Returns: a %NULL-terminated list of strings, of length @length + **/ +gchar ** +gvdb_table_get_names (GvdbTable *table, + gint *length) +{ + gchar **names; + gint n_names; + gint filled; + gint total; + gint i; + + /* We generally proceed by iterating over the list of items in the + * hash table (in order of appearance) recording them into an array. + * + * Each item has a parent item (except root items). The parent item + * forms part of the name of the item. We could go fetching the + * parent item chain at the point that we encounter each item but then + * we would need to implement some sort of recursion along with checks + * for self-referential items. + * + * Instead, we do a number of passes. Each pass will build up one + * level of names (starting from the root). We continue to do passes + * until no more items are left. The first pass will only add root + * items and each further pass will only add items whose direct parent + * is an item added in the immediately previous pass. It's also + * possible that items get filled if they follow their parent within a + * particular pass. + * + * At most we will have a number of passes equal to the depth of the + * tree. Self-referential items will never be filled in (since their + * parent will have never been filled in). We continue until we have + * a pass that fills in no additional items. + * + * This takes an O(n) algorithm and turns it into O(n*m) where m is + * the depth of the tree, but in all sane cases the tree won't be very + * deep and the constant factor of this algorithm is lower (and the + * complexity of coding it, as well). + */ + + n_names = table->n_hash_items; + names = g_new0 (gchar *, n_names + 1); + + /* 'names' starts out all-NULL. On each pass we record the number + * of items changed from NULL to non-NULL in 'filled' so we know if we + * should repeat the loop. 'total' counts the total number of items + * filled. If 'total' ends up equal to 'n_names' then we know that + * 'names' has been completely filled. + */ + + total = 0; + do + { + /* Loop until we have filled no more entries */ + filled = 0; + + for (i = 0; i < n_names; i++) + { + const struct gvdb_hash_item *item = &table->hash_items[i]; + const gchar *name; + gsize name_length; + guint32 parent; + + /* already got it on a previous pass */ + if (names[i] != NULL) + continue; + + parent = guint32_from_le (item->parent); + + if (parent == 0xffffffffu) + { + /* it's a root item */ + name = gvdb_table_item_get_key (table, item, &name_length); + + if (name != NULL) + { + names[i] = g_strndup (name, name_length); + filled++; + } + } + + else if (parent < n_names && names[parent] != NULL) + { + /* It's a non-root item whose parent was filled in already. + * + * Calculate the name of this item by combining it with + * its parent name. + */ + name = gvdb_table_item_get_key (table, item, &name_length); + + if (name != NULL) + { + const gchar *parent_name = names[parent]; + gsize parent_length; + gchar *fullname; + + parent_length = strlen (parent_name); + fullname = g_malloc (parent_length + name_length + 1); + memcpy (fullname, parent_name, parent_length); + memcpy (fullname + parent_length, name, name_length); + fullname[parent_length + name_length] = '\0'; + names[i] = fullname; + filled++; + } + } + } + + total += filled; + } + while (filled && total < n_names); + + /* If the table was corrupted then 'names' may have holes in it. + * Collapse those. + */ + if G_UNLIKELY (total != n_names) + { + GPtrArray *fixed_names; + + fixed_names = g_ptr_array_new (); + for (i = 0; i < n_names; i++) + if (names[i] != NULL) + g_ptr_array_add (fixed_names, names[i]); + + g_free (names); + n_names = fixed_names->len; + g_ptr_array_add (fixed_names, NULL); + names = (gchar **) g_ptr_array_free (fixed_names, FALSE); + } + + if (length) + *length = n_names; + + return names; +} /** * gvdb_table_list: diff --git a/gvdb-reader.h b/gvdb-reader.h index 2a50c10fb..5c4631a62 100644 --- a/gvdb-reader.h +++ b/gvdb-reader.h @@ -46,7 +46,9 @@ G_GNUC_INTERNAL GvdbTable * gvdb_table_ref (GvdbTable *table); G_GNUC_INTERNAL void gvdb_table_unref (GvdbTable *table); - +G_GNUC_INTERNAL +gchar ** gvdb_table_get_names (GvdbTable *table, + gint *length); G_GNUC_INTERNAL gchar ** gvdb_table_list (GvdbTable *table, const gchar *key); From fc37611a97605938949d50374f61e2a81a9d1ae4 Mon Sep 17 00:00:00 2001 From: Ryan Lortie Date: Mon, 9 Jul 2012 15:23:39 -0400 Subject: [PATCH 08/19] gvdb-reader: drop gvdb_table_walk() 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. --- gvdb-reader.c | 127 -------------------------------------------------- gvdb-reader.h | 19 -------- 2 files changed, 146 deletions(-) diff --git a/gvdb-reader.c b/gvdb-reader.c index a8531af2d..062c427a3 100644 --- a/gvdb-reader.c +++ b/gvdb-reader.c @@ -348,18 +348,6 @@ gvdb_table_lookup (GvdbTable *file, return NULL; } -static const struct gvdb_hash_item * -gvdb_table_get_item (GvdbTable *table, - guint32_le item_no) -{ - guint32 item_no_native = guint32_from_le (item_no); - - if G_LIKELY (item_no_native < table->n_hash_items) - return table->hash_items + item_no_native; - - return NULL; -} - static gboolean gvdb_table_list_from_item (GvdbTable *table, const struct gvdb_hash_item *item, @@ -798,118 +786,3 @@ gvdb_table_is_valid (GvdbTable *table) { return !!*table->data; } - -/** - * gvdb_table_walk: - * @table: a #GvdbTable - * @key: a key corresponding to a list - * @open_func: the #GvdbWalkOpenFunc - * @value_func: the #GvdbWalkValueFunc - * @close_func: the #GvdbWalkCloseFunc - * @user_data: data to pass to the callbacks - * - * Looks up the list at @key and iterate over the items in it. - * - * First, @open_func is called to signal that we are starting to iterate over - * the list. Then the list is iterated. When all items in the list have been - * iterated over, the @close_func is called. - * - * When iterating, if a given item in the list is a value then @value_func is - * called. - * - * If a given item in the list is itself a list then @open_func is called. If - * that function returns %TRUE then the walk begins iterating the items in the - * sublist, until there are no more items, at which point a matching - * @close_func call is made. If @open_func returns %FALSE then no iteration of - * the sublist occurs and no corresponding @close_func call is made. - **/ -void -gvdb_table_walk (GvdbTable *table, - const gchar *key, - GvdbWalkOpenFunc open_func, - GvdbWalkValueFunc value_func, - GvdbWalkCloseFunc close_func, - gpointer user_data) -{ - const struct gvdb_hash_item *item; - const guint32_le *pointers[64]; - const guint32_le *enders[64]; - gsize name_lengths[64]; - gint index = 0; - - item = gvdb_table_lookup (table, key, 'L'); - name_lengths[0] = 0; - pointers[0] = NULL; - enders[0] = NULL; - goto start_here; - - while (index) - { - close_func (name_lengths[index], user_data); - index--; - - while (pointers[index] < enders[index]) - { - const gchar *name; - gsize name_len; - - item = gvdb_table_get_item (table, *pointers[index]++); - start_here: - - if (item != NULL && (name = gvdb_table_item_get_key (table, item, &name_len))) - { - if (item->type == 'L') - { - const guint32_le *dir; - guint length; - - if (gvdb_table_list_from_item (table, item, &dir, &length)) - { - /* In order to avoid files with recursive contents - * we impose the rule that a directory's data must - * follow the data of any directory pointing to - * it. - * - * If we discover that our newly-discovered - * directory follows the one we're traversing now - * then bail out. - */ - if (dir <= pointers[index]) - continue; - - if (open_func (name, name_len, user_data)) - { - index++; - g_assert (index < 64); - - name_lengths[index] = name_len; - pointers[index] = dir; - enders[index] = pointers[index] + length; - } - } - } - else if (item->type == 'v') - { - GVariant *value; - - value = gvdb_table_value_from_item (table, item); - - if (value != NULL) - { - if (table->byteswapped) - { - GVariant *tmp; - - tmp = g_variant_byteswap (value); - g_variant_unref (value); - value = tmp; - } - - value_func (name, name_len, value, user_data); - g_variant_unref (value); - } - } - } - } - } -} diff --git a/gvdb-reader.h b/gvdb-reader.h index 5c4631a62..d6984acef 100644 --- a/gvdb-reader.h +++ b/gvdb-reader.h @@ -65,28 +65,9 @@ GVariant * gvdb_table_get_value (GvdbTab G_GNUC_INTERNAL gboolean gvdb_table_has_value (GvdbTable *table, const gchar *key); - G_GNUC_INTERNAL gboolean gvdb_table_is_valid (GvdbTable *table); -typedef void (*GvdbWalkValueFunc) (const gchar *name, - gsize name_len, - GVariant *value, - gpointer user_data); -typedef gboolean (*GvdbWalkOpenFunc) (const gchar *name, - gsize name_len, - gpointer user_data); -typedef void (*GvdbWalkCloseFunc) (gsize name_len, - gpointer user_data); - -G_GNUC_INTERNAL -void gvdb_table_walk (GvdbTable *table, - const gchar *key, - GvdbWalkOpenFunc open_func, - GvdbWalkValueFunc value_func, - GvdbWalkCloseFunc close_func, - gpointer user_data); - G_END_DECLS #endif /* __gvdb_reader_h__ */ From 26000821510c66713feacb8b40c1ed5253195a53 Mon Sep 17 00:00:00 2001 From: Ryan Lortie Date: Tue, 23 Oct 2012 17:34:13 +0200 Subject: [PATCH 09/19] reader: embrace GBytes 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. --- gvdb-reader.c | 168 +++++++++++++++----------------------------------- gvdb-reader.h | 18 ++---- 2 files changed, 55 insertions(+), 131 deletions(-) diff --git a/gvdb-reader.c b/gvdb-reader.c index 062c427a3..8ccfc8f5f 100644 --- a/gvdb-reader.c +++ b/gvdb-reader.c @@ -25,15 +25,11 @@ #include struct _GvdbTable { - gint ref_count; + GBytes *bytes; const gchar *data; gsize size; - gpointer user_data; - GvdbRefFunc ref_user_data; - GDestroyNotify unref_user_data; - gboolean byteswapped; gboolean trusted; @@ -126,27 +122,32 @@ gvdb_table_setup_root (GvdbTable *file, file->n_hash_items = size / sizeof (struct gvdb_hash_item); } -static GvdbTable * -new_from_data (const void *data, - gsize data_len, - gboolean trusted, - gpointer user_data, - GvdbRefFunc ref, - GDestroyNotify unref, - const char *filename, - GError **error) +/** + * gvdb_table_new_from_bytes: + * @bytes: the #GBytes with the data + * @trusted: if the contents of @bytes are trusted + * @error: %NULL, or a pointer to a %NULL #GError + * @returns: a new #GvdbTable + * + * Creates a new #GvdbTable from the contents of @bytes. + * + * This call can fail if the header contained in @bytes is invalid. + * + * You should call gvdb_table_free() on the return result when you no + * longer require it. + **/ +GvdbTable * +gvdb_table_new_from_bytes (GBytes *bytes, + gboolean trusted, + GError **error) { const struct gvdb_header *header; GvdbTable *file; file = g_slice_new0 (GvdbTable); - file->data = data; - file->size = data_len; + file->bytes = g_bytes_ref (bytes); + file->data = g_bytes_get_data (bytes, &file->size); file->trusted = trusted; - file->ref_count = 1; - file->ref_user_data = ref; - file->unref_user_data = unref; - file->user_data = user_data; if (file->size < sizeof (struct gvdb_header)) goto invalid; @@ -171,38 +172,24 @@ new_from_data (const void *data, return file; invalid: - if (filename) - g_set_error (error, G_FILE_ERROR, G_FILE_ERROR_INVAL, "%s: invalid header", filename); - else - g_set_error_literal (error, G_FILE_ERROR, G_FILE_ERROR_INVAL, "invalid gvdb header"); + g_set_error_literal (error, G_FILE_ERROR, G_FILE_ERROR_INVAL, "invalid gvdb header"); + + g_bytes_unref (file->bytes); g_slice_free (GvdbTable, file); - if (unref) - unref (user_data); - return NULL; } /** * gvdb_table_new: - * @filename: the path to the hash file - * @trusted: if the contents of @filename are trusted + * @filename: a filename + * @trusted: if the contents of @bytes are trusted * @error: %NULL, or a pointer to a %NULL #GError * @returns: a new #GvdbTable * - * Creates a new #GvdbTable from the contents of the file found at - * @filename. - * - * The only time this function fails is if the file cannot be opened. - * In that case, the #GError that is returned will be an error from - * g_mapped_file_new(). - * - * An empty or otherwise corrupted file is considered to be a valid - * #GvdbTable with no entries. - * - * You should call gvdb_table_unref() on the return result when you no - * longer require it. + * Creates a new #GvdbTable using the #GMappedFile for @filename as the + * #GBytes. **/ GvdbTable * gvdb_table_new (const gchar *filename, @@ -210,52 +197,21 @@ gvdb_table_new (const gchar *filename, GError **error) { GMappedFile *mapped; + GvdbTable *table; + GBytes *bytes; - if ((mapped = g_mapped_file_new (filename, FALSE, error)) == NULL) + mapped = g_mapped_file_new (filename, FALSE, error); + if (!mapped) return NULL; - return new_from_data (g_mapped_file_get_contents (mapped), - g_mapped_file_get_length (mapped), - trusted, - mapped, - (GvdbRefFunc)g_mapped_file_ref, - (GDestroyNotify)g_mapped_file_unref, - filename, - error); -} + bytes = g_mapped_file_get_bytes (mapped); + table = gvdb_table_new_from_bytes (bytes, trusted, error); + g_mapped_file_unref (mapped); + g_bytes_unref (bytes); -/** - * gvdb_table_new_from_data: - * @data: the data - * @data_len: the length of @data in bytes - * @trusted: if the contents of @data are trusted - * @user_data: User supplied data that owns @data - * @ref: Ref function for @user_data - * @unref: Unref function for @user_data - * @returns: a new #GvdbTable - * - * Creates a new #GvdbTable from the data in @data. - * - * An empty or otherwise corrupted data is considered to be a valid - * #GvdbTable with no entries. - * - * You should call gvdb_table_unref() on the return result when you no - * longer require it. - **/ -GvdbTable * -gvdb_table_new_from_data (const void *data, - gsize data_len, - gboolean trusted, - gpointer user_data, - GvdbRefFunc ref, - GDestroyNotify unref, - GError **error) -{ - return new_from_data (data, data_len, - trusted, - user_data, ref, unref, - NULL, - error); + g_prefix_error (error, "%s: ", filename); + + return table; } static gboolean @@ -611,6 +567,7 @@ gvdb_table_value_from_item (GvdbTable *table, { GVariant *variant, *value; gconstpointer data; + GBytes *bytes; gsize size; data = gvdb_table_dereference (table, &item->value.pointer, 8, &size); @@ -618,12 +575,11 @@ gvdb_table_value_from_item (GvdbTable *table, if G_UNLIKELY (data == NULL) return NULL; - variant = g_variant_new_from_data (G_VARIANT_TYPE_VARIANT, - data, size, table->trusted, - table->unref_user_data, - table->ref_user_data ? table->ref_user_data (table->user_data) : table->user_data); + bytes = g_bytes_new_from_bytes (table->bytes, ((gchar *) data) - table->data, size); + variant = g_variant_new_from_bytes (G_VARIANT_TYPE_VARIANT, bytes, table->trusted); value = g_variant_get_variant (variant); g_variant_unref (variant); + g_bytes_unref (bytes); return value; } @@ -706,7 +662,7 @@ gvdb_table_get_raw_value (GvdbTable *table, * contained in the file. This newly-created #GvdbTable does not depend * on the continued existence of @file. * - * You should call gvdb_table_unref() on the return result when you no + * You should call gvdb_table_free() on the return result when you no * longer require it. **/ GvdbTable * @@ -722,14 +678,11 @@ gvdb_table_get_table (GvdbTable *file, return NULL; new = g_slice_new0 (GvdbTable); - new->user_data = file->ref_user_data ? file->ref_user_data (file->user_data) : file->user_data; - new->ref_user_data = file->ref_user_data; - new->unref_user_data = file->unref_user_data; + new->bytes = g_bytes_ref (file->bytes); new->byteswapped = file->byteswapped; new->trusted = file->trusted; new->data = file->data; new->size = file->size; - new->ref_count = 1; gvdb_table_setup_root (new, &item->value.pointer); @@ -737,37 +690,16 @@ gvdb_table_get_table (GvdbTable *file, } /** - * gvdb_table_ref: - * @file: a #GvdbTable - * @returns: a new reference on @file - * - * Increases the reference count on @file. - **/ -GvdbTable * -gvdb_table_ref (GvdbTable *file) -{ - g_atomic_int_inc (&file->ref_count); - - return file; -} - -/** - * gvdb_table_unref: + * gvdb_table_free: * @file: a #GvdbTable * - * Decreases the reference count on @file, possibly freeing it. - * - * Since: 2.26 + * Frees @file. **/ void -gvdb_table_unref (GvdbTable *file) +gvdb_table_free (GvdbTable *file) { - if (g_atomic_int_dec_and_test (&file->ref_count)) - { - if (file->unref_user_data) - file->unref_user_data (file->user_data); - g_slice_free (GvdbTable, file); - } + g_bytes_unref (file->bytes); + g_slice_free (GvdbTable, file); } /** diff --git a/gvdb-reader.h b/gvdb-reader.h index d6984acef..5980925ec 100644 --- a/gvdb-reader.h +++ b/gvdb-reader.h @@ -26,26 +26,18 @@ typedef struct _GvdbTable GvdbTable; -typedef gpointer (*GvdbRefFunc) (gpointer data); - G_BEGIN_DECLS +G_GNUC_INTERNAL +GvdbTable * gvdb_table_new_from_bytes (GBytes *bytes, + gboolean trusted, + GError **error); G_GNUC_INTERNAL GvdbTable * gvdb_table_new (const gchar *filename, gboolean trusted, GError **error); G_GNUC_INTERNAL -GvdbTable * gvdb_table_new_from_data (const void *data, - gsize data_len, - gboolean trusted, - gpointer user_data, - GvdbRefFunc ref, - GDestroyNotify unref, - GError **error); -G_GNUC_INTERNAL -GvdbTable * gvdb_table_ref (GvdbTable *table); -G_GNUC_INTERNAL -void gvdb_table_unref (GvdbTable *table); +void gvdb_table_free (GvdbTable *table); G_GNUC_INTERNAL gchar ** gvdb_table_get_names (GvdbTable *table, gint *length); From 355228121e7656b1c48fd96fa4bcccc0d14161ae Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 13 Aug 2018 14:29:51 +0100 Subject: [PATCH 10/19] doap: Update maintainers list to synchronise with GLib MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- gvdb.doap | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/gvdb.doap b/gvdb.doap index b4ae60c8c..8c5f3e802 100644 --- a/gvdb.doap +++ b/gvdb.doap @@ -23,9 +23,34 @@ - Ryan Lortie - - ryanl + Matthias Clasen + + matthiasc + + + + + + Allison Ryan Lortie + + desrt + + + + + + Philip Withnall + + + pwithnall + + + + + + Emmanuele Bassi + + ebassi From 7fd9f61dbdbe4b0a05c7c66267f06119a16e869a Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 13 Aug 2018 14:13:28 +0100 Subject: [PATCH 11/19] Avoid calling Standard C string/array functions with NULL arguments 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 e5ed410c8c0fe823883 from GLib.) Signed-off-by: Simon McVittie Bug: https://bugzilla.gnome.org/show_bug.cgi?id=775510 Reviewed-by: Colin Walters --- gvdb-builder.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gvdb-builder.c b/gvdb-builder.c index c63d1171e..061876845 100644 --- a/gvdb-builder.c +++ b/gvdb-builder.c @@ -293,7 +293,8 @@ file_builder_add_string (FileBuilder *fb, chunk->offset = fb->offset; chunk->size = length; chunk->data = g_malloc (length); - memcpy (chunk->data, string, length); + if (length != 0) + memcpy (chunk->data, string, length); *start = guint32_to_le (fb->offset); *size = guint16_to_le (length); From 4763db8a4c4a4f8e4fcfc9aefda082b273978a92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Wilmet?= Date: Fri, 26 May 2017 16:08:19 +0200 Subject: [PATCH 12/19] gio/gvdb/: LGPLv2+ -> LGPLv2.1+ https://bugzilla.gnome.org/show_bug.cgi?id=776504 --- gvdb-builder.c | 2 +- gvdb-builder.h | 2 +- gvdb-format.h | 2 +- gvdb-reader.c | 2 +- gvdb-reader.h | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gvdb-builder.c b/gvdb-builder.c index 061876845..ea1c76fee 100644 --- a/gvdb-builder.c +++ b/gvdb-builder.c @@ -4,7 +4,7 @@ * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either - * version 2 of the licence, or (at your option) any later version. + * version 2.1 of the License, or (at your option) any later version. * * This library is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of diff --git a/gvdb-builder.h b/gvdb-builder.h index 797626ef6..edbaac288 100644 --- a/gvdb-builder.h +++ b/gvdb-builder.h @@ -4,7 +4,7 @@ * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either - * version 2 of the licence, or (at your option) any later version. + * version 2.1 of the License, or (at your option) any later version. * * This library is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of diff --git a/gvdb-format.h b/gvdb-format.h index 886aa5697..d02a2eba9 100644 --- a/gvdb-format.h +++ b/gvdb-format.h @@ -4,7 +4,7 @@ * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either - * version 2 of the licence, or (at your option) any later version. + * version 2.1 of the License, or (at your option) any later version. * * This library is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of diff --git a/gvdb-reader.c b/gvdb-reader.c index 8ccfc8f5f..eb7a11c83 100644 --- a/gvdb-reader.c +++ b/gvdb-reader.c @@ -4,7 +4,7 @@ * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either - * version 2 of the licence, or (at your option) any later version. + * version 2.1 of the License, or (at your option) any later version. * * This library is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of diff --git a/gvdb-reader.h b/gvdb-reader.h index 5980925ec..7113eb3be 100644 --- a/gvdb-reader.h +++ b/gvdb-reader.h @@ -4,7 +4,7 @@ * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either - * version 2 of the licence, or (at your option) any later version. + * version 2.1 of the License, or (at your option) any later version. * * This library is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of From c78664e101cd46e10d59be83703ade46b5d4c8e5 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 13 Aug 2018 14:22:53 +0100 Subject: [PATCH 13/19] =?UTF-8?q?Update=20FSF=E2=80=99s=20address?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (This is based on commit 892fc2e4 from dconf.) Signed-off-by: Philip Withnall --- gvdb-builder.c | 4 +--- gvdb-builder.h | 4 +--- gvdb-format.h | 4 +--- gvdb-reader.c | 4 +--- gvdb-reader.h | 4 +--- 5 files changed, 5 insertions(+), 15 deletions(-) diff --git a/gvdb-builder.c b/gvdb-builder.c index ea1c76fee..2383e6002 100644 --- a/gvdb-builder.c +++ b/gvdb-builder.c @@ -12,9 +12,7 @@ * Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the - * Free Software Foundation, Inc., 59 Temple Place - Suite 330, - * Boston, MA 02111-1307, USA. + * License along with this library; if not, see . * * Author: Ryan Lortie */ diff --git a/gvdb-builder.h b/gvdb-builder.h index edbaac288..b4815f0d0 100644 --- a/gvdb-builder.h +++ b/gvdb-builder.h @@ -12,9 +12,7 @@ * Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the - * Free Software Foundation, Inc., 59 Temple Place - Suite 330, - * Boston, MA 02111-1307, USA. + * License along with this library; if not, see . * * Author: Ryan Lortie */ diff --git a/gvdb-format.h b/gvdb-format.h index d02a2eba9..ed6adabfa 100644 --- a/gvdb-format.h +++ b/gvdb-format.h @@ -12,9 +12,7 @@ * Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the - * Free Software Foundation, Inc., 59 Temple Place - Suite 330, - * Boston, MA 02111-1307, USA. + * License along with this library; if not, see . * * Author: Ryan Lortie */ diff --git a/gvdb-reader.c b/gvdb-reader.c index eb7a11c83..bc4de095d 100644 --- a/gvdb-reader.c +++ b/gvdb-reader.c @@ -12,9 +12,7 @@ * Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the - * Free Software Foundation, Inc., 59 Temple Place - Suite 330, - * Boston, MA 02111-1307, USA. + * License along with this library; if not, see . * * Author: Ryan Lortie */ diff --git a/gvdb-reader.h b/gvdb-reader.h index 7113eb3be..39827737d 100644 --- a/gvdb-reader.h +++ b/gvdb-reader.h @@ -12,9 +12,7 @@ * Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the - * Free Software Foundation, Inc., 59 Temple Place - Suite 330, - * Boston, MA 02111-1307, USA. + * License along with this library; if not, see . * * Author: Ryan Lortie */ From d2f0461ec0e548ab659d247599846d16741a1e5d Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 13 Aug 2018 14:27:07 +0100 Subject: [PATCH 14/19] =?UTF-8?q?docs:=20Use=20=E2=80=98Returns:=E2=80=99?= =?UTF-8?q?=20instead=20of=20the=20invalid=20=E2=80=98@returns=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is based on the commit 59a24ab5a3 in GLib. Signed-off-by: Philip Withnall --- gvdb-reader.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/gvdb-reader.c b/gvdb-reader.c index bc4de095d..1ec66332a 100644 --- a/gvdb-reader.c +++ b/gvdb-reader.c @@ -125,7 +125,6 @@ gvdb_table_setup_root (GvdbTable *file, * @bytes: the #GBytes with the data * @trusted: if the contents of @bytes are trusted * @error: %NULL, or a pointer to a %NULL #GError - * @returns: a new #GvdbTable * * Creates a new #GvdbTable from the contents of @bytes. * @@ -133,6 +132,8 @@ gvdb_table_setup_root (GvdbTable *file, * * You should call gvdb_table_free() on the return result when you no * longer require it. + * + * Returns: a new #GvdbTable **/ GvdbTable * gvdb_table_new_from_bytes (GBytes *bytes, @@ -184,10 +185,11 @@ invalid: * @filename: a filename * @trusted: if the contents of @bytes are trusted * @error: %NULL, or a pointer to a %NULL #GError - * @returns: a new #GvdbTable * * Creates a new #GvdbTable using the #GMappedFile for @filename as the * #GBytes. + * + * Returns: a new #GvdbTable **/ GvdbTable * gvdb_table_new (const gchar *filename, @@ -474,7 +476,6 @@ gvdb_table_get_names (GvdbTable *table, * gvdb_table_list: * @file: a #GvdbTable * @key: a string - * @returns: a %NULL-terminated string array * * List all of the keys that appear below @key. The nesting of keys * within the hash file is defined by the program that created the hash @@ -487,6 +488,8 @@ gvdb_table_get_names (GvdbTable *table, * * You should call g_strfreev() on the return result when you no longer * require it. + * + * Returns: a %NULL-terminated string array **/ gchar ** gvdb_table_list (GvdbTable *file, @@ -537,12 +540,13 @@ gvdb_table_list (GvdbTable *file, * gvdb_table_has_value: * @file: a #GvdbTable * @key: a string - * @returns: %TRUE if @key is in the table * * Checks for a value named @key in @file. * * Note: this function does not consider non-value nodes (other hash * tables, for example). + * + * Returns: %TRUE if @key is in the table **/ gboolean gvdb_table_has_value (GvdbTable *file, @@ -586,7 +590,6 @@ gvdb_table_value_from_item (GvdbTable *table, * gvdb_table_get_value: * @file: a #GvdbTable * @key: a string - * @returns: a #GVariant, or %NULL * * Looks up a value named @key in @file. * @@ -596,6 +599,8 @@ gvdb_table_value_from_item (GvdbTable *table, * * You should call g_variant_unref() on the return result when you no * longer require it. + * + * Returns: a #GVariant, or %NULL **/ GVariant * gvdb_table_get_value (GvdbTable *file, @@ -625,12 +630,13 @@ gvdb_table_get_value (GvdbTable *file, * gvdb_table_get_raw_value: * @table: a #GvdbTable * @key: a string - * @returns: a #GVariant, or %NULL * * Looks up a value named @key in @file. * * This call is equivalent to gvdb_table_get_value() except that it * never byteswaps the value. + * + * Returns: a #GVariant, or %NULL **/ GVariant * gvdb_table_get_raw_value (GvdbTable *table, @@ -648,7 +654,6 @@ gvdb_table_get_raw_value (GvdbTable *table, * gvdb_table_get_table: * @file: a #GvdbTable * @key: a string - * @returns: a new #GvdbTable, or %NULL * * Looks up the hash table named @key in @file. * @@ -662,6 +667,8 @@ gvdb_table_get_raw_value (GvdbTable *table, * * You should call gvdb_table_free() on the return result when you no * longer require it. + * + * Returns: a new #GvdbTable, or %NULL **/ GvdbTable * gvdb_table_get_table (GvdbTable *file, @@ -703,13 +710,14 @@ gvdb_table_free (GvdbTable *file) /** * gvdb_table_is_valid: * @table: a #GvdbTable - * @returns: %TRUE if @table is still valid * * Checks if the table is still valid. * * An on-disk GVDB can be marked as invalid. This happens when the file * has been replaced. The appropriate action is typically to reopen the * file. + * + * Returns: %TRUE if @table is still valid **/ gboolean gvdb_table_is_valid (GvdbTable *table) From 57962aac85a41f554e14da14906a3ba0bd8f9156 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 13 Aug 2018 14:27:46 +0100 Subject: [PATCH 15/19] docs: Clarify error values for empty files when loading Signed-off-by: Philip Withnall --- gvdb-reader.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/gvdb-reader.c b/gvdb-reader.c index 1ec66332a..aa3154feb 100644 --- a/gvdb-reader.c +++ b/gvdb-reader.c @@ -128,7 +128,8 @@ gvdb_table_setup_root (GvdbTable *file, * * Creates a new #GvdbTable from the contents of @bytes. * - * This call can fail if the header contained in @bytes is invalid. + * This call can fail if the header contained in @bytes is invalid or if @bytes + * is empty; if so, %G_FILE_ERROR_INVAL will be returned. * * You should call gvdb_table_free() on the return result when you no * longer require it. @@ -189,6 +190,12 @@ invalid: * Creates a new #GvdbTable using the #GMappedFile for @filename as the * #GBytes. * + * This function will fail if the file cannot be opened. + * In that case, the #GError that is returned will be an error from + * g_mapped_file_new(). + * + * An empty or corrupt file will result in %G_FILE_ERROR_INVAL. + * * Returns: a new #GvdbTable **/ GvdbTable * From ce86a3aed7c73c056b90324902bf40d60a487a37 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 1 Aug 2018 13:01:51 +0100 Subject: [PATCH 16/19] glib-compile-schemas: Fix a minor memory leak Signed-off-by: Philip Withnall --- gio/glib-compile-schemas.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gio/glib-compile-schemas.c b/gio/glib-compile-schemas.c index d4340d463..5e1bebbba 100644 --- a/gio/glib-compile-schemas.c +++ b/gio/glib-compile-schemas.c @@ -2112,6 +2112,7 @@ set_overrides (GHashTable *schema_table, } g_strfreev (groups); + g_key_file_free (key_file); } return TRUE; From 705dd2b9a9d4bdbd4b5a59dc7a4d64755b956033 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 1 Aug 2018 14:31:57 +0100 Subject: [PATCH 17/19] gresource: Port to new GVDB API This should introduce no functional changes. Signed-off-by: Philip Withnall --- gio/gresource.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/gio/gresource.c b/gio/gresource.c index bf54f1d78..066306d1b 100644 --- a/gio/gresource.c +++ b/gio/gresource.c @@ -489,7 +489,7 @@ g_resource_unref (GResource *resource) { if (g_atomic_int_dec_and_test (&resource->ref_count)) { - gvdb_table_unref (resource->table); + gvdb_table_free (resource->table); g_free (resource); } } @@ -546,13 +546,7 @@ g_resource_new_from_data (GBytes *data, unref_data = TRUE; } - table = gvdb_table_new_from_data (g_bytes_get_data (data, NULL), - g_bytes_get_size (data), - TRUE, - g_bytes_ref (data), - (GvdbRefFunc)g_bytes_ref, - (GDestroyNotify)g_bytes_unref, - error); + table = gvdb_table_new_from_bytes (data, TRUE, error); if (unref_data) g_bytes_unref (data); From c652d45c97f18ab4976b975766fe6d65e7cf3d94 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 1 Aug 2018 14:32:15 +0100 Subject: [PATCH 18/19] gsettingsschema: Port to new GVDB API This should introduce no functional changes. Signed-off-by: Philip Withnall --- gio/gsettingsschema.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gio/gsettingsschema.c b/gio/gsettingsschema.c index 17b7e3b01..725e97087 100644 --- a/gio/gsettingsschema.c +++ b/gio/gsettingsschema.c @@ -231,7 +231,7 @@ g_settings_schema_source_unref (GSettingsSchemaSource *source) if (source->parent) g_settings_schema_source_unref (source->parent); - gvdb_table_unref (source->table); + gvdb_table_free (source->table); g_free (source->directory); if (source->text_tables) @@ -802,7 +802,7 @@ g_settings_schema_source_list_schemas (GSettingsSchemaSource *source, else g_hash_table_add (reloc, schema); - gvdb_table_unref (table); + gvdb_table_free (table); } } @@ -928,7 +928,7 @@ g_settings_schema_unref (GSettingsSchema *schema) g_settings_schema_unref (schema->extends); g_settings_schema_source_unref (schema->source); - gvdb_table_unref (schema->table); + gvdb_table_free (schema->table); g_free (schema->items); g_free (schema->id); @@ -1188,7 +1188,7 @@ g_settings_schema_list (GSettingsSchema *schema, g_hash_table_iter_remove (&iter); } - gvdb_table_unref (child_table); + gvdb_table_free (child_table); } /* Now create the list */ From 614adf8a7503913de10203c05a41434d6051c0c7 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 1 Aug 2018 13:03:07 +0100 Subject: [PATCH 19/19] gvdb: Fix error handling in gvdb_table_new() The documentation was unclear about what error codes would be returned on attempting to open an empty or corrupt GVDB file. Previous versions of the documentation incorrectly said that corrupt GVDB files were considered equivalent to empty ones. A recent commit has clarified the documentation to include its error handling behaviour. Update the two users of GVDB within GLib, GResource and GSettingsSource, to follow this change, and add unit tests for them both. Other users of the GVDB copylib will need to update their copy and make appropriate changes if they have bugs in their handling of this situation. dconf is one example of this. GVDB should be updated from https://gitlab.gnome.org/GNOME/gvdb. Signed-off-by: Philip Withnall https://gitlab.gnome.org/GNOME/glib/issues/1454 --- gio/gresource.c | 36 ++++++++++++++++++++++++++++++++---- gio/gsettingsschema.c | 3 +++ gio/tests/gsettings.c | 33 +++++++++++++++++++++++++++++++++ gio/tests/resources.c | 41 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 4 deletions(-) diff --git a/gio/gresource.c b/gio/gresource.c index 066306d1b..2844f4808 100644 --- a/gio/gresource.c +++ b/gio/gresource.c @@ -512,6 +512,19 @@ g_resource_new_from_table (GvdbTable *table) return resource; } +static void +g_resource_error_from_gvdb_table_error (GError **g_resource_error, + GError *gvdb_table_error /* (transfer full) */) +{ + if (g_error_matches (gvdb_table_error, G_FILE_ERROR, G_FILE_ERROR_INVAL)) + g_set_error_literal (g_resource_error, + G_RESOURCE_ERROR, G_RESOURCE_ERROR_INTERNAL, + gvdb_table_error->message); + else + g_propagate_error (g_resource_error, g_steal_pointer (&gvdb_table_error)); + g_clear_error (&gvdb_table_error); +} + /** * g_resource_new_from_data: * @data: A #GBytes @@ -528,6 +541,8 @@ g_resource_new_from_table (GvdbTable *table) * Otherwise this function will internally create a copy of the memory since * GLib 2.56, or in older versions fail and exit the process. * + * If @data is empty or corrupt, %G_RESOURCE_ERROR_INTERNAL will be returned. + * * Returns: (transfer full): a new #GResource, or %NULL on error * * Since: 2.32 @@ -538,6 +553,7 @@ g_resource_new_from_data (GBytes *data, { GvdbTable *table; gboolean unref_data = FALSE; + GError *local_error = NULL; if (((guintptr) g_bytes_get_data (data, NULL)) % sizeof (gpointer) != 0) { @@ -546,13 +562,16 @@ g_resource_new_from_data (GBytes *data, unref_data = TRUE; } - table = gvdb_table_new_from_bytes (data, TRUE, error); + table = gvdb_table_new_from_bytes (data, TRUE, &local_error); if (unref_data) g_bytes_unref (data); if (table == NULL) - return NULL; + { + g_resource_error_from_gvdb_table_error (error, g_steal_pointer (&local_error)); + return NULL; + } return g_resource_new_from_table (table); } @@ -568,6 +587,11 @@ g_resource_new_from_data (GBytes *data, * If you want to use this resource in the global resource namespace you need * to register it with g_resources_register(). * + * If @filename is empty or the data in it is corrupt, + * %G_RESOURCE_ERROR_INTERNAL will be returned. If @filename doesn’t exist, or + * there is an error in reading it, an error from g_mapped_file_new() will be + * returned. + * * Returns: (transfer full): a new #GResource, or %NULL on error * * Since: 2.32 @@ -577,10 +601,14 @@ g_resource_load (const gchar *filename, GError **error) { GvdbTable *table; + GError *local_error = NULL; - table = gvdb_table_new (filename, FALSE, error); + table = gvdb_table_new (filename, FALSE, &local_error); if (table == NULL) - return NULL; + { + g_resource_error_from_gvdb_table_error (error, g_steal_pointer (&local_error)); + return NULL; + } return g_resource_new_from_table (table); } diff --git a/gio/gsettingsschema.c b/gio/gsettingsschema.c index 725e97087..38c9d78b9 100644 --- a/gio/gsettingsschema.c +++ b/gio/gsettingsschema.c @@ -267,6 +267,9 @@ g_settings_schema_source_unref (GSettingsSchemaSource *source) * Generally, you should set @trusted to %TRUE for files installed by the * system and to %FALSE for files in the home directory. * + * In either case, an empty file or some types of corruption in the file will + * result in %G_FILE_ERROR_INVAL being returned. + * * If @parent is non-%NULL then there are two effects. * * First, if g_settings_schema_source_lookup() is called with the diff --git a/gio/tests/gsettings.c b/gio/tests/gsettings.c index 2d18d4dd2..852a8b710 100644 --- a/gio/tests/gsettings.c +++ b/gio/tests/gsettings.c @@ -2353,6 +2353,18 @@ test_schema_source (void) g_assert_error (error, G_FILE_ERROR, G_FILE_ERROR_NOENT); g_clear_error (&error); + /* Test error handling of corrupt compiled files. */ + source = g_settings_schema_source_new_from_directory ("schema-source-corrupt", parent, TRUE, &error); + g_assert_error (error, G_FILE_ERROR, G_FILE_ERROR_INVAL); + g_assert_null (source); + g_clear_error (&error); + + /* Test error handling of empty compiled files. */ + source = g_settings_schema_source_new_from_directory ("schema-source-empty", parent, TRUE, &error); + g_assert_error (error, G_FILE_ERROR, G_FILE_ERROR_INVAL); + g_assert_null (source); + g_clear_error (&error); + /* create a source with the parent */ source = g_settings_schema_source_new_from_directory ("schema-source", parent, TRUE, &error); g_assert_no_error (error); @@ -2770,6 +2782,12 @@ main (int argc, char *argv[]) if (!g_test_subprocess ()) { + GError *local_error = NULL; + /* A GVDB header is 6 guint32s, and requires a magic number in the first + * two guint32s. A set of zero bytes of a greater length is considered + * corrupt. */ + const guint8 gschemas_compiled_corrupt[sizeof (guint32) * 7] = { 0, }; + backend_set = g_getenv ("GSETTINGS_BACKEND") != NULL; g_setenv ("XDG_DATA_DIRS", ".", TRUE); @@ -2821,6 +2839,21 @@ main (int argc, char *argv[]) "--schema-file=" SRCDIR "/org.gtk.schemasourcecheck.gschema.xml", NULL, NULL, &result, NULL)); g_assert (result == 0); + + g_remove ("schema-source-corrupt/gschemas.compiled"); + g_mkdir ("schema-source-corrupt", 0777); + g_file_set_contents ("schema-source-corrupt/gschemas.compiled", + (const gchar *) gschemas_compiled_corrupt, + sizeof (gschemas_compiled_corrupt), + &local_error); + g_assert_no_error (local_error); + + g_remove ("schema-source-empty/gschemas.compiled"); + g_mkdir ("schema-source-empty", 0777); + g_file_set_contents ("schema-source-empty/gschemas.compiled", + "", 0, + &local_error); + g_assert_no_error (local_error); } g_test_add_func ("/gsettings/basic", test_basic); diff --git a/gio/tests/resources.c b/gio/tests/resources.c index 6ae8e7d64..70d8c03a6 100644 --- a/gio/tests/resources.c +++ b/gio/tests/resources.c @@ -317,6 +317,45 @@ test_resource_data_unaligned (void) g_resource_unref (resource); } +/* Test error handling for corrupt GResource files (specifically, a corrupt + * GVDB header). */ +static void +test_resource_data_corrupt (void) +{ + /* A GVDB header is 6 guint32s, and requires a magic number in the first two + * guint32s. A set of zero bytes of a greater length is considered corrupt. */ + static const guint8 data[sizeof (guint32) * 7] = { 0, }; + GBytes *bytes = NULL; + GResource *resource = NULL; + GError *local_error = NULL; + + bytes = g_bytes_new_static (data, sizeof (data)); + resource = g_resource_new_from_data (bytes, &local_error); + g_bytes_unref (bytes); + g_assert_error (local_error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_INTERNAL); + g_assert_null (resource); + + g_clear_error (&local_error); +} + +/* Test handling for empty GResource files. They should also be treated as + * corrupt. */ +static void +test_resource_data_empty (void) +{ + GBytes *bytes = NULL; + GResource *resource = NULL; + GError *local_error = NULL; + + bytes = g_bytes_new_static (NULL, 0); + resource = g_resource_new_from_data (bytes, &local_error); + g_bytes_unref (bytes); + g_assert_error (local_error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_INTERNAL); + g_assert_null (resource); + + g_clear_error (&local_error); +} + static void test_resource_registered (void) { @@ -785,6 +824,8 @@ main (int argc, g_test_add_func ("/resource/file-path", test_resource_file_path); g_test_add_func ("/resource/data", test_resource_data); g_test_add_func ("/resource/data_unaligned", test_resource_data_unaligned); + g_test_add_func ("/resource/data-corrupt", test_resource_data_corrupt); + g_test_add_func ("/resource/data-empty", test_resource_data_empty); g_test_add_func ("/resource/registered", test_resource_registered); g_test_add_func ("/resource/manual", test_resource_manual); g_test_add_func ("/resource/manual2", test_resource_manual2);