Merge branch '3465-gresource-error-allocations' into 'main'

gresource: Reduce allocations in g_resources_*() functions

Closes #3465

See merge request GNOME/glib!4242
This commit is contained in:
Philip Withnall 2024-09-17 13:34:06 +00:00
commit acd4a78173
4 changed files with 205 additions and 71 deletions

View File

@ -675,6 +675,18 @@ g_resource_load (const gchar *filename,
return g_resource_new_from_table (table);
}
static void
set_error_not_found (GError **error,
const char *path)
{
/* Avoid looking up the translation if its not going to be used. This is a hot path. */
if (error != NULL)
g_set_error (error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND,
_("The resource at “%s” does not exist"),
path);
}
/* The only error this can return is %G_RESOURCE_ERROR_NOT_FOUND. */
static gboolean
do_lookup (GResource *resource,
const gchar *path,
@ -702,9 +714,7 @@ do_lookup (GResource *resource,
if (value == NULL)
{
g_set_error (error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND,
_("The resource at “%s” does not exist"),
path);
set_error_not_found (error, path);
}
else
{
@ -755,6 +765,9 @@ do_lookup (GResource *resource,
*
* @lookup_flags controls the behaviour of the lookup.
*
* The only error this can return is %G_RESOURCE_ERROR_NOT_FOUND, if @path was
* not found in @resource.
*
* Returns: (transfer full): #GInputStream or %NULL on error.
* Free the returned object with g_object_unref()
*
@ -793,6 +806,14 @@ g_resource_open_stream (GResource *resource,
return stream;
}
static GBytes *resource_to_bytes (GResource *resource,
const char *path,
size_t size,
const void *data,
size_t data_size,
guint32 flags,
GError **error);
/**
* g_resource_lookup_data:
* @resource: A #GResource
@ -815,6 +836,10 @@ g_resource_open_stream (GResource *resource,
*
* @lookup_flags controls the behaviour of the lookup.
*
* This can return error %G_RESOURCE_ERROR_NOT_FOUND if @path was not found in
* @resource, or %G_RESOURCE_ERROR_INTERNAL if decompression of a compressed
* resource failed.
*
* Returns: (transfer full): #GBytes or %NULL on error.
* Free the returned object with g_bytes_unref()
*
@ -834,6 +859,18 @@ g_resource_lookup_data (GResource *resource,
if (!do_lookup (resource, path, lookup_flags, &size, &flags, &data, &data_size, error))
return NULL;
return resource_to_bytes (resource, path, size, data, data_size, flags, error);
}
static GBytes *
resource_to_bytes (GResource *resource,
const char *path,
size_t size,
const void *data,
size_t data_size,
guint32 flags,
GError **error)
{
if (size == 0)
return g_bytes_new_with_free_func ("", 0, (GDestroyNotify) g_resource_unref, g_resource_ref (resource));
else if (flags & G_RESOURCE_FLAGS_COMPRESSED)
@ -908,6 +945,9 @@ g_resource_lookup_data (GResource *resource,
*
* @lookup_flags controls the behaviour of the lookup.
*
* The only error this can return is %G_RESOURCE_ERROR_NOT_FOUND, if @path was
* not found in @resource.
*
* Returns: %TRUE if the file was found. %FALSE if there were errors
*
* Since: 2.32
@ -998,10 +1038,7 @@ g_resource_enumerate_children (GResource *resource,
if (*path == 0)
{
if (error)
g_set_error (error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND,
_("The resource at “%s” does not exist"),
path);
set_error_not_found (error, path);
return NULL;
}
@ -1013,10 +1050,7 @@ g_resource_enumerate_children (GResource *resource,
if (children == NULL)
{
if (error)
g_set_error (error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND,
_("The resource at “%s” does not exist"),
path);
set_error_not_found (error, path);
return NULL;
}
@ -1159,27 +1193,22 @@ g_resources_open_stream (const gchar *path,
for (l = registered_resources; l != NULL; l = l->next)
{
GResource *r = l->data;
GError *my_error = NULL;
stream = g_resource_open_stream (r, path, lookup_flags, &my_error);
if (stream == NULL &&
g_error_matches (my_error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND))
stream = g_resource_open_stream (r, path, lookup_flags, NULL);
if (stream == NULL)
{
g_clear_error (&my_error);
/* g_resource_open_stream() guarantees it only fails with
* %G_RESOURCE_ERROR_NOT_FOUND */
}
else
{
if (stream == NULL)
g_propagate_error (error, my_error);
res = stream;
break;
}
}
if (l == NULL)
g_set_error (error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND,
_("The resource at “%s” does not exist"),
path);
set_error_not_found (error, path);
g_rw_lock_reader_unlock (&resources_lock);
@ -1219,7 +1248,6 @@ g_resources_lookup_data (const gchar *path,
{
GBytes *res = NULL;
GList *l;
GBytes *data;
if (g_resource_find_overlay (path, get_overlay_bytes, &res))
return res;
@ -1231,27 +1259,22 @@ g_resources_lookup_data (const gchar *path,
for (l = registered_resources; l != NULL; l = l->next)
{
GResource *r = l->data;
GError *my_error = NULL;
const void *data;
guint32 flags;
gsize data_size;
gsize size;
data = g_resource_lookup_data (r, path, lookup_flags, &my_error);
if (data == NULL &&
g_error_matches (my_error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND))
/* This is essentially g_resource_lookup_data(), but split up so we can
* avoid allocating a #GError if the resource is not found. */
if (do_lookup (r, path, lookup_flags, &size, &flags, &data, &data_size, NULL))
{
g_clear_error (&my_error);
}
else
{
if (data == NULL)
g_propagate_error (error, my_error);
res = data;
res = resource_to_bytes (r, path, size, data, data_size, flags, error);
break;
}
}
if (l == NULL)
g_set_error (error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND,
_("The resource at “%s” does not exist"),
path);
set_error_not_found (error, path);
g_rw_lock_reader_unlock (&resources_lock);
@ -1322,10 +1345,7 @@ g_resources_enumerate_children (const gchar *path,
if (hash == NULL)
{
if (error)
g_set_error (error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND,
_("The resource at “%s” does not exist"),
path);
set_error_not_found (error, path);
return NULL;
}
else
@ -1399,7 +1419,6 @@ g_resources_get_info (const gchar *path,
{
gboolean res = FALSE;
GList *l;
gboolean r_res;
InfoData info;
if (g_resource_find_overlay (path, get_overlay_info, &info))
@ -1419,27 +1438,16 @@ g_resources_get_info (const gchar *path,
for (l = registered_resources; l != NULL; l = l->next)
{
GResource *r = l->data;
GError *my_error = NULL;
r_res = g_resource_get_info (r, path, lookup_flags, size, flags, &my_error);
if (!r_res &&
g_error_matches (my_error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND))
if (g_resource_get_info (r, path, lookup_flags, size, flags, NULL))
{
g_clear_error (&my_error);
}
else
{
if (!r_res)
g_propagate_error (error, my_error);
res = r_res;
res = TRUE;
break;
}
}
if (l == NULL)
g_set_error (error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND,
_("The resource at “%s” does not exist"),
path);
set_error_not_found (error, path);
g_rw_lock_reader_unlock (&resources_lock);

View File

@ -859,6 +859,21 @@ if not meson.is_cross_build()
install_tag : 'tests',
install : installed_tests_enabled)
test6_gresource = custom_target('test6.gresource',
input : 'test6.gresource.xml',
depends : [gspawn_helpers],
output : 'test6.gresource',
command : [glib_compile_resources,
compiler_type,
'--target=@OUTPUT@',
'--sourcedir=' + meson.current_source_dir(),
'--sourcedir=' + meson.current_build_dir(),
'--internal',
'@INPUT@'],
install_dir : installed_tests_execdir,
install_tag : 'tests',
install : installed_tests_enabled)
test_resources2_c = custom_target('test_resources2.c',
input : 'test3.gresource.xml',
depends : [gspawn_helpers],
@ -930,6 +945,7 @@ if not meson.is_cross_build()
resources_extra_sources = [
test_gresource,
test6_gresource,
test_resources_c,
test_resources2_c,
test_resources2_h,

View File

@ -401,6 +401,55 @@ test_resource_data_empty (void)
g_clear_error (&local_error);
}
static void
test_resource_data_corrupt_compression (void)
{
GFile *resource_file = NULL;
GBytes *resource_bytes = NULL, *corrupt_bytes = NULL, *data_bytes = NULL;
guint8 *corrupt_data = NULL;
GResource *resource = NULL;
GError *local_error = NULL;
g_test_summary ("Test error handling for corrupt GResource files (specifically, corrupt zlib compression).");
resource_file = g_file_new_for_path (g_test_get_filename (G_TEST_BUILT, "test6.gresource", NULL));
resource_bytes = g_file_load_bytes (resource_file, NULL, NULL, &local_error);
g_assert_no_error (local_error);
g_clear_object (&resource_file);
/* Test loading the resource normally, to check it works. */
resource = g_resource_new_from_data (resource_bytes, &local_error);
g_assert_no_error (local_error);
data_bytes = g_resource_lookup_data (resource, "/test-corrupt-compression.txt",
G_RESOURCE_LOOKUP_FLAGS_NONE, &local_error);
g_assert_no_error (local_error);
g_assert_nonnull (data_bytes);
g_clear_pointer (&data_bytes, g_bytes_unref);
g_clear_pointer (&resource, g_resource_unref);
/* Modify the data to zero out bytes 0x90 to 0x100. These are comfortably
* within the compressed file data, so should break that while not breaking
* the GVDB header. */
corrupt_data = g_memdup2 (g_bytes_get_data (resource_bytes, NULL), g_bytes_get_size (resource_bytes));
memset (corrupt_data + 0x90, 0, 0x10);
corrupt_bytes = g_bytes_new_take (g_steal_pointer (&corrupt_data), g_bytes_get_size (resource_bytes));
resource = g_resource_new_from_data (corrupt_bytes, &local_error);
g_assert_no_error (local_error);
g_bytes_unref (corrupt_bytes);
data_bytes = g_resource_lookup_data (resource, "/test-corrupt-compression.txt",
G_RESOURCE_LOOKUP_FLAGS_NONE, &local_error);
g_assert_error (local_error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_INTERNAL);
g_assert_null (data_bytes);
g_clear_error (&local_error);
g_clear_pointer (&resource_bytes, g_bytes_unref);
g_clear_pointer (&resource, g_resource_unref);
}
static void
test_resource_registered (void)
{
@ -553,6 +602,20 @@ test_resource_registered (void)
g_assert_false (found);
g_assert_error (error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND);
g_clear_error (&error);
data = g_resources_lookup_data ("/test1.txt",
G_RESOURCE_LOOKUP_FLAGS_NONE,
&error);
g_assert_error (error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND);
g_assert_null (data);
g_clear_error (&error);
in = g_resources_open_stream ("/test1.txt",
G_RESOURCE_LOOKUP_FLAGS_NONE,
&error);
g_assert_error (error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND);
g_assert_null (in);
g_clear_error (&error);
}
static void
@ -1021,26 +1084,65 @@ test_overlay (void)
{
if (g_test_subprocess ())
{
GError *error = NULL;
gboolean res;
gsize size;
char *overlay;
char *path;
GError *error = NULL;
gboolean res;
gsize size;
char *overlay;
char *path;
GInputStream *in = NULL;
char buffer[128];
GBytes *data = NULL;
char *expected_overlay_data = NULL;
size_t expected_overlay_size = 0;
path = g_test_build_filename (G_TEST_DIST, "test1.overlay", NULL);
overlay = g_strconcat ("/auto_loaded/test1.txt=", path, NULL);
path = g_test_build_filename (G_TEST_DIST, "test1.overlay", NULL);
res = g_file_get_contents (path, &expected_overlay_data, &expected_overlay_size, NULL);
g_assert (res);
g_setenv ("G_RESOURCE_OVERLAYS", overlay, TRUE);
res = g_resources_get_info ("/auto_loaded/test1.txt", 0, &size, NULL, &error);
g_assert_true (res);
g_assert_no_error (error);
/* test1.txt is 6 bytes, test1.overlay is 23 */
g_assert_cmpint (size, ==, 23);
overlay = g_strconcat ("/auto_loaded/test1.txt=", path, NULL);
g_setenv ("G_RESOURCE_OVERLAYS", overlay, TRUE);
g_free (overlay);
g_free (path);
/* Test getting its info. */
res = g_resources_get_info ("/auto_loaded/test1.txt", 0, &size, NULL, &error);
g_assert_true (res);
g_assert_no_error (error);
/* test1.txt is 6 bytes, test1.overlay is 23 */
g_assert_cmpuint (size, ==, expected_overlay_size);
return;
/* Test it as a stream too. */
in = g_resources_open_stream ("/auto_loaded/test1.txt",
G_RESOURCE_LOOKUP_FLAGS_NONE,
&error);
g_assert_no_error (error);
g_assert_nonnull (in);
res = g_input_stream_read_all (in, buffer, sizeof (buffer) - 1,
&size,
NULL, &error);
g_assert_no_error (error);
g_assert_true (res);
g_assert_cmpuint (size, ==, expected_overlay_size);
g_input_stream_close (in, NULL, &error);
g_assert_no_error (error);
g_clear_object (&in);
/* Test data lookup. */
data = g_resources_lookup_data ("/auto_loaded/test1.txt",
G_RESOURCE_LOOKUP_FLAGS_NONE,
&error);
g_assert_nonnull (data);
g_assert_no_error (error);
size = g_bytes_get_size (data);
g_assert_cmpuint (size, ==, expected_overlay_size);
g_assert_cmpstr (g_bytes_get_data (data, NULL), ==, expected_overlay_data);
g_bytes_unref (data);
g_free (overlay);
g_free (path);
g_free (expected_overlay_data);
return;
}
g_test_trap_subprocess (NULL, 0, G_TEST_SUBPROCESS_INHERIT_STDERR);
g_test_trap_assert_passed ();
@ -1085,6 +1187,7 @@ main (int argc,
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-corrupt-compression", test_resource_data_corrupt_compression);
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);

View File

@ -0,0 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<gresources>
<gresource>
<!-- An arbitrary text file which is large enough to have a non-trivial compressed form -->
<file compressed="true" alias="test-corrupt-compression.txt">test.gresource.xml</file>
</gresource>
</gresources>