From b715e4c9d00a05b7edfb93692e6930563349bac2 Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Thu, 26 Dec 2019 16:23:23 +0100 Subject: [PATCH 1/5] gerror: Simplify error construction Factor out the GError creation to a common function. When adding a support for extended error types, this will limit the number of places where these errors are allocated. --- glib/gerror.c | 46 +++++++++++++++++++--------------------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/glib/gerror.c b/glib/gerror.c index 86167d7e5..129a82f2b 100644 --- a/glib/gerror.c +++ b/glib/gerror.c @@ -382,6 +382,20 @@ #include "gstrfuncs.h" #include "gtestutils.h" +static GError * +g_error_new_steal (GQuark domain, + gint code, + gchar *message) +{ + GError *error = g_slice_new (GError); + + error->domain = domain; + error->code = code; + error->message = message; + + return error; +} + /** * g_error_new_valist: * @domain: error domain @@ -402,8 +416,6 @@ g_error_new_valist (GQuark domain, const gchar *format, va_list args) { - GError *error; - /* Historically, GError allowed this (although it was never meant to work), * and it has significant use in the wild, which g_return_val_if_fail * would break. It should maybe g_return_val_if_fail in GLib 4. @@ -412,13 +424,7 @@ g_error_new_valist (GQuark domain, g_warn_if_fail (domain != 0); g_warn_if_fail (format != NULL); - error = g_slice_new (GError); - - error->domain = domain; - error->code = code; - error->message = g_strdup_vprintf (format, args); - - return error; + return g_error_new_steal (domain, code, g_strdup_vprintf (format, args)); } /** @@ -470,18 +476,10 @@ g_error_new_literal (GQuark domain, gint code, const gchar *message) { - GError* err; - g_return_val_if_fail (message != NULL, NULL); g_return_val_if_fail (domain != 0, NULL); - err = g_slice_new (GError); - - err->domain = domain; - err->code = code; - err->message = g_strdup (message); - - return err; + return g_error_new_steal (domain, code, g_strdup (message)); } /** @@ -511,20 +509,14 @@ g_error_free (GError *error) GError* g_error_copy (const GError *error) { - GError *copy; - g_return_val_if_fail (error != NULL, NULL); /* See g_error_new_valist for why these don't return */ g_warn_if_fail (error->domain != 0); g_warn_if_fail (error->message != NULL); - copy = g_slice_new (GError); - - *copy = *error; - - copy->message = g_strdup (error->message); - - return copy; + return g_error_new_steal (error->domain, + error->code, + g_strdup (error->message)); } /** From ae72f9de352af9a84679f98f81b4462f7a9cc2fa Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Wed, 25 Dec 2019 13:07:32 +0100 Subject: [PATCH 2/5] gerror: Add support for extended errors This commit adds a G_DEFINE_EXTENDED_ERROR macro and g_error_domain_register() functions to register extended error domains. --- docs/reference/glib/glib-sections.txt | 7 + glib/gerror.c | 359 +++++++++++++++++++++++++- glib/gerror.h | 137 ++++++++++ glib/glib-init.c | 1 + glib/glib-init.h | 1 + glib/tests/error.c | 100 +++++++ 6 files changed, 595 insertions(+), 10 deletions(-) diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt index 690c0de9c..eed764893 100644 --- a/docs/reference/glib/glib-sections.txt +++ b/docs/reference/glib/glib-sections.txt @@ -754,6 +754,13 @@ g_propagate_error g_clear_error g_prefix_error g_propagate_prefixed_error + +GErrorInitFunc +GErrorCopyFunc +GErrorClearFunc +G_DEFINE_EXTENDED_ERROR +g_error_domain_register_static +g_error_domain_register
diff --git a/glib/gerror.c b/glib/gerror.c index 129a82f2b..dd69283eb 100644 --- a/glib/gerror.c +++ b/glib/gerror.c @@ -372,27 +372,338 @@ * to add a check at the top of your function that the error return * location is either %NULL or contains a %NULL error (e.g. * `g_return_if_fail (error == NULL || *error == NULL);`). + * + * Since GLib 2.68 it is possible to extend the #GError type. This is + * done with the G_DEFINE_EXTENDED_ERROR() macro. To create an + * extended #GError type do something like this in the header file: + * |[ + * typedef enum + * { + * MY_ERROR_BAD_REQUEST, + * } MyError; + * #define MY_ERROR (my_error_quark ()) + * GQuark my_error_quark (void); + * int + * my_error_get_parse_error_id (GError *error); + * const char * + * my_error_get_bad_request_details (GError *error); + * ]| + * and in implementation: + * |[ + * typedef struct + * { + * int parse_error_id; + * char *bad_request_details; + * } MyErrorPrivate; + * + * static void + * my_error_private_init (MyErrorPrivate *priv) + * { + * priv->parse_error_id = -1; + * // No need to set priv->bad_request_details to NULL, + * // the struct is initialized with zeros. + * } + * + * static void + * my_error_private_copy (const MyErrorPrivate *src_priv, MyErrorPrivate *dest_priv) + * { + * dest_priv->parse_error_id = src_priv->parse_error_id; + * dest_priv->bad_request_details = g_strdup (src_priv->bad_request_details); + * } + * + * static void + * my_error_private_clear (MyErrorPrivate *priv) + * { + * g_free (priv->bad_request_details); + * } + * + * // This defines the my_error_get_private and my_error_quark functions. + * G_DEFINE_EXTENDED_ERROR (MyError, my_error) + * + * int + * my_error_get_parse_error_id (GError *error) + * { + * MyErrorPrivate *priv = my_error_get_private (error); + * g_return_val_if_fail (priv != NULL, -1); + * return priv->parse_error_id; + * } + * + * const char * + * my_error_get_bad_request_details (GError *error) + * { + * MyErrorPrivate *priv = my_error_get_private (error); + * g_return_val_if_fail (priv != NULL, NULL); + * g_return_val_if_fail (error->code != MY_ERROR_BAD_REQUEST, NULL); + * return priv->bad_request_details; + * } + * + * static void + * my_error_set_bad_request (GError **error, + * const char *reason, + * int error_id, + * const char *details) + * { + * MyErrorPrivate *priv; + * g_set_error (error, MY_ERROR, MY_ERROR_BAD_REQUEST, "Invalid request: %s", reason); + * if (error != NULL && *error != NULL) + * { + * priv = my_error_get_private (error); + * g_return_val_if_fail (priv != NULL, NULL); + * priv->parse_error_id = error_id; + * priv->bad_request_details = g_strdup (details); + * } + * } + * ]| + * An example of use of the error could be: + * |[ + * gboolean + * send_request (GBytes *request, GError **error) + * { + * ParseFailedStatus *failure = validate_request (request); + * if (failure != NULL) + * { + * my_error_set_bad_request (error, failure->reason, failure->error_id, failure->details); + * parse_failed_status_free (failure); + * return FALSE; + * } + * + * return send_one (request, error); + * } + * ]| + * + * Please note that if you are a library author and your library + * exposes an existing error domain, then you can't make this error + * domain an extended one without breaking ABI. This is because + * earlier it was possible to create an error with this error domain + * on the stack and then copy it with g_error_copy(). If the new + * version of your library makes the error domain an extended one, + * then g_error_copy() called by code that allocated the error on the + * stack will try to copy more data than it used to, which will lead + * to undefined behavior. You must not stack-allocate errors with an + * extended error domain, and it is bad practice to stack-allocate any + * other #GErrors. + * + * Extended error domains in unloadable plugins/modules are not + * supported. */ #include "config.h" +#include + #include "gerror.h" +#include "ghash.h" +#include "glib-init.h" #include "gslice.h" #include "gstrfuncs.h" #include "gtestutils.h" +#include "gthread.h" + +static GRWLock error_domain_global; +/* error_domain_ht must be accessed with error_domain_global + * locked. + */ +static GHashTable *error_domain_ht = NULL; + +void +g_error_init (void) +{ + error_domain_ht = g_hash_table_new (NULL, NULL); +} + +typedef struct +{ + /* private_size is already aligned. */ + gsize private_size; + GErrorInitFunc init; + GErrorCopyFunc copy; + GErrorClearFunc clear; +} ErrorDomainInfo; + +/* Must be called with error_domain_global locked. + */ +static inline ErrorDomainInfo * +error_domain_lookup (GQuark domain) +{ + return g_hash_table_lookup (error_domain_ht, + GUINT_TO_POINTER (domain)); +} + +/* Copied from gtype.c. */ +#define STRUCT_ALIGNMENT (2 * sizeof (gsize)) +#define ALIGN_STRUCT(offset) \ + ((offset + (STRUCT_ALIGNMENT - 1)) & -STRUCT_ALIGNMENT) + +static void +error_domain_register (GQuark error_quark, + gsize error_type_private_size, + GErrorInitFunc error_type_init, + GErrorCopyFunc error_type_copy, + GErrorClearFunc error_type_clear) +{ + g_rw_lock_writer_lock (&error_domain_global); + if (error_domain_lookup (error_quark) == NULL) + { + ErrorDomainInfo *info = g_new (ErrorDomainInfo, 1); + info->private_size = ALIGN_STRUCT (error_type_private_size); + info->init = error_type_init; + info->copy = error_type_copy; + info->clear = error_type_clear; + + g_hash_table_insert (error_domain_ht, + GUINT_TO_POINTER (error_quark), + info); + } + else + { + const char *name = g_quark_to_string (error_quark); + + g_critical ("Attempted to register an extended error domain for %s more than once", name); + } + g_rw_lock_writer_unlock (&error_domain_global); +} + +/** + * g_error_domain_register_static: + * @error_type_name: static string to create a #GQuark from + * @error_type_private_size: size of the private error data in bytes + * @error_type_init: function initializing fields of the private error data + * @error_type_copy: function copying fields of the private error data + * @error_type_clear: function freeing fields of the private error data + * + * This function registers an extended #GError domain. + * + * @error_type_name should not be freed. @error_type_private_size must + * be greater than 0. + * + * @error_type_init receives an initialized #GError and should then initialize + * the private data. + * + * @error_type_copy is a function that receives both original and a copy + * #GError and should copy the fields of the private error data. The standard + * #GError fields are already handled. + * + * @error_type_clear receives the pointer to the error, and it should free the + * fields of the private error data. It should not free the struct itself though. + * + * Normally, it is better to use G_DEFINE_EXTENDED_ERROR(), as it + * already takes care of passing valid information to this function. + * + * Returns: #GQuark representing the error domain + * Since: 2.68 + */ +GQuark +g_error_domain_register_static (const char *error_type_name, + gsize error_type_private_size, + GErrorInitFunc error_type_init, + GErrorCopyFunc error_type_copy, + GErrorClearFunc error_type_clear) +{ + GQuark error_quark; + + g_return_val_if_fail (error_type_name != NULL, 0); + g_return_val_if_fail (error_type_private_size > 0, 0); + g_return_val_if_fail (error_type_init != NULL, 0); + g_return_val_if_fail (error_type_copy != NULL, 0); + g_return_val_if_fail (error_type_clear != NULL, 0); + + error_quark = g_quark_from_static_string (error_type_name); + error_domain_register (error_quark, + error_type_private_size, + error_type_init, + error_type_copy, + error_type_clear); + return error_quark; +} + +/** + * g_error_domain_register: + * @error_type_name: string to create a #GQuark from + * @error_type_private_size: size of the private error data in bytes + * @error_type_init: function initializing fields of the private error data + * @error_type_copy: function copying fields of the private error data + * @error_type_clear: function freeing fields of the private error data + * + * This function registers an extended #GError domain. + * @error_type_name will be duplicated. Otherwise does the same as + * g_error_domain_register_static(). + * + * Returns: #GQuark representing the error domain + * Since: 2.68 + */ +GQuark +g_error_domain_register (const char *error_type_name, + gsize error_type_private_size, + GErrorInitFunc error_type_init, + GErrorCopyFunc error_type_copy, + GErrorClearFunc error_type_clear) +{ + GQuark error_quark; + + g_return_val_if_fail (error_type_name != NULL, 0); + g_return_val_if_fail (error_type_private_size > 0, 0); + g_return_val_if_fail (error_type_init != NULL, 0); + g_return_val_if_fail (error_type_copy != NULL, 0); + g_return_val_if_fail (error_type_clear != NULL, 0); + + error_quark = g_quark_from_string (error_type_name); + error_domain_register (error_quark, + error_type_private_size, + error_type_init, + error_type_copy, + error_type_clear); + return error_quark; +} static GError * -g_error_new_steal (GQuark domain, - gint code, - gchar *message) +g_error_allocate (GQuark domain, ErrorDomainInfo *out_info) { - GError *error = g_slice_new (GError); + guint8 *allocated; + GError *error; + ErrorDomainInfo *info; + gsize private_size; + + g_rw_lock_reader_lock (&error_domain_global); + info = error_domain_lookup (domain); + if (info != NULL) + { + if (out_info != NULL) + *out_info = *info; + private_size = info->private_size; + g_rw_lock_reader_unlock (&error_domain_global); + } + else + { + g_rw_lock_reader_unlock (&error_domain_global); + if (out_info != NULL) + memset (out_info, 0, sizeof (*out_info)); + private_size = 0; + } + allocated = g_slice_alloc0 (private_size + sizeof (GError)); + error = (GError *) (allocated + private_size); + return error; +} + +/* This function takes ownership of @message. */ +static GError * +g_error_new_steal (GQuark domain, + gint code, + gchar *message, + ErrorDomainInfo *out_info) +{ + ErrorDomainInfo info; + GError *error = g_error_allocate (domain, &info); error->domain = domain; error->code = code; error->message = message; + if (info.init != NULL) + info.init (error); + if (out_info != NULL) + *out_info = info; + return error; } @@ -424,7 +735,7 @@ g_error_new_valist (GQuark domain, g_warn_if_fail (domain != 0); g_warn_if_fail (format != NULL); - return g_error_new_steal (domain, code, g_strdup_vprintf (format, args)); + return g_error_new_steal (domain, code, g_strdup_vprintf (format, args), NULL); } /** @@ -479,7 +790,7 @@ g_error_new_literal (GQuark domain, g_return_val_if_fail (message != NULL, NULL); g_return_val_if_fail (domain != 0, NULL); - return g_error_new_steal (domain, code, g_strdup (message)); + return g_error_new_steal (domain, code, g_strdup (message), NULL); } /** @@ -491,11 +802,31 @@ g_error_new_literal (GQuark domain, void g_error_free (GError *error) { + gsize private_size; + ErrorDomainInfo *info; + guint8 *allocated; + g_return_if_fail (error != NULL); - g_free (error->message); + g_rw_lock_reader_lock (&error_domain_global); + info = error_domain_lookup (error->domain); + if (info != NULL) + { + GErrorClearFunc clear = info->clear; - g_slice_free (GError, error); + private_size = info->private_size; + g_rw_lock_reader_unlock (&error_domain_global); + clear (error); + } + else + { + g_rw_lock_reader_unlock (&error_domain_global); + private_size = 0; + } + + g_free (error->message); + allocated = ((guint8 *) error) - private_size; + g_slice_free1 (private_size + sizeof (GError), allocated); } /** @@ -509,14 +840,22 @@ g_error_free (GError *error) GError* g_error_copy (const GError *error) { + GError *copy; + ErrorDomainInfo info; + g_return_val_if_fail (error != NULL, NULL); /* See g_error_new_valist for why these don't return */ g_warn_if_fail (error->domain != 0); g_warn_if_fail (error->message != NULL); - return g_error_new_steal (error->domain, + copy = g_error_new_steal (error->domain, error->code, - g_strdup (error->message)); + g_strdup (error->message), + &info); + if (info.copy != NULL) + info.copy (error, copy); + + return copy; } /** diff --git a/glib/gerror.h b/glib/gerror.h index 8ecff04e1..8773a1023 100644 --- a/glib/gerror.h +++ b/glib/gerror.h @@ -47,6 +47,143 @@ struct _GError gchar *message; }; +/** + * G_DEFINE_EXTENDED_ERROR: + * @ErrorType: name to return a #GQuark for + * @error_type: prefix for the function name + * + * A convenience macro which defines two functions. First, returning + * the #GQuark for the extended error type @ErrorType; it is called + * `@error_type_quark()`. Second, returning the private data from a + * passed #GError; it is called `@error_type_get_private()`. + * + * For this macro to work, a type named `@ErrorTypePrivate` should be + * defined, `@error_type_private_init()`, `@error_type_private_copy()` + * and `@error_type_private_clear()` functions need to be either + * declared or defined. The functions should be similar to + * #GErrorInitFunc, #GErrorCopyFunc and #GErrorClearFunc, + * respectively, but they should receive the private data type instead + * of #GError. + * + * Since: 2.68 + */ +#define G_DEFINE_EXTENDED_ERROR(ErrorType, error_type) \ +static inline ErrorType ## Private * \ +error_type ## _get_private (const GError *error) \ +{ \ + /* Copied from gtype.c (STRUCT_ALIGNMENT and ALIGN_STRUCT macros). */ \ + const gsize sa = 2 * sizeof (gsize); \ + const gsize as = (sizeof (ErrorType ## Private) + (sa - 1)) & -sa; \ + g_return_val_if_fail (error != NULL, NULL); \ + g_return_val_if_fail (error->domain == error_type ## _quark (), NULL); \ + return (ErrorType ## Private *) (((guint8 *)error) - as); \ +} \ + \ +static void \ +g_error_with_ ## error_type ## _private_init (GError *error) \ +{ \ + ErrorType ## Private *priv = error_type ## _get_private (error); \ + error_type ## _private_init (priv); \ +} \ + \ +static void \ +g_error_with_ ## error_type ## _private_copy (const GError *src_error, \ + GError *dest_error) \ +{ \ + const ErrorType ## Private *src_priv = error_type ## _get_private (src_error); \ + ErrorType ## Private *dest_priv = error_type ## _get_private (dest_error); \ + error_type ## _private_copy (src_priv, dest_priv); \ +} \ + \ +static void \ +g_error_with_ ## error_type ## _private_clear (GError *error) \ +{ \ + ErrorType ## Private *priv = error_type ## _get_private (error); \ + error_type ## _private_clear (priv); \ +} \ + \ +GQuark \ +error_type ## _quark (void) \ +{ \ + static GQuark q; \ + static gsize initialized = 0; \ + \ + if (g_once_init_enter (&initialized)) \ + { \ + q = g_error_domain_register_static (#ErrorType, \ + sizeof (ErrorType ## Private), \ + g_error_with_ ## error_type ## _private_init, \ + g_error_with_ ## error_type ## _private_copy, \ + g_error_with_ ## error_type ## _private_clear); \ + g_once_init_leave (&initialized, 1); \ + } \ + \ + return q; \ +} + +/** + * GErrorInitFunc: + * @error: extended error + * + * Specifies the type of function which is called just after an + * extended error instance is created and its fields filled. It should + * only initialize the fields in the private data, which can be + * received with the generated `*_get_private()` function. + * + * Normally, it is better to use G_DEFINE_EXTENDED_ERROR(), as it + * already takes care of getting the private data from @error. + * + * Since: 2.68 + */ +typedef void (*GErrorInitFunc) (GError *error); + +/** + * GErrorCopyFunc: + * @src_error: source extended error + * @dest_error: destination extended error + * + * Specifies the type of function which is called when an extended + * error instance is copied. It is passed the pointer to the + * destination error and source error, and should copy only the fields + * of the private data from @src_error to @dest_error. + * + * Normally, it is better to use G_DEFINE_EXTENDED_ERROR(), as it + * already takes care of getting the private data from @src_error and + * @dest_error. + * + * Since: 2.68 + */ +typedef void (*GErrorCopyFunc) (const GError *src_error, GError *dest_error); + +/** + * GErrorClearFunc: + * @error: extended error to clear + * + * Specifies the type of function which is called when an extended + * error instance is freed. It is passed the error pointer about to be + * freed, and should free the error's private data fields. + * + * Normally, it is better to use G_DEFINE_EXTENDED_ERROR(), as it + * already takes care of getting the private data from @error. + * + * Since: 2.68 + */ +typedef void (*GErrorClearFunc) (GError *error); + +GLIB_AVAILABLE_IN_2_68 +GQuark g_error_domain_register_static (const char *error_type_name, + gsize error_type_private_size, + GErrorInitFunc error_type_init, + GErrorCopyFunc error_type_copy, + GErrorClearFunc error_type_clear); + +GLIB_AVAILABLE_IN_2_68 +GQuark g_error_domain_register (const char *error_type_name, + gsize error_type_private_size, + GErrorInitFunc error_type_init, + GErrorCopyFunc error_type_copy, + GErrorClearFunc error_type_clear); + GLIB_AVAILABLE_IN_ALL GError* g_error_new (GQuark domain, gint code, diff --git a/glib/glib-init.c b/glib/glib-init.c index 902e18f4f..2958fb5be 100644 --- a/glib/glib-init.c +++ b/glib/glib-init.c @@ -337,6 +337,7 @@ glib_init (void) g_messages_prefixed_init (); g_debug_init (); g_quark_init (); + g_error_init (); } #if defined (G_OS_WIN32) diff --git a/glib/glib-init.h b/glib/glib-init.h index f44b9a4d2..c25cbb0a1 100644 --- a/glib/glib-init.h +++ b/glib/glib-init.h @@ -27,6 +27,7 @@ extern GLogLevelFlags g_log_msg_prefix; void glib_init (void); void g_quark_init (void); +void g_error_init (void); #ifdef G_OS_WIN32 #include diff --git a/glib/tests/error.c b/glib/tests/error.c index 44e501506..c2e35d898 100644 --- a/glib/tests/error.c +++ b/glib/tests/error.c @@ -82,6 +82,105 @@ test_copy (void) g_error_free (copy); } +typedef struct +{ + int init_called; + int copy_called; + int free_called; +} TestErrorCheck; + +static TestErrorCheck *init_check; + +GQuark test_error_quark (void); +#define TEST_ERROR (test_error_quark ()) + +typedef struct +{ + int foo; + TestErrorCheck *check; +} TestErrorPrivate; + +static void +test_error_private_init (TestErrorPrivate *priv) +{ + priv->foo = 13; + /* If that triggers, it's test bug. + */ + g_assert_nonnull (init_check); + /* Using global init_check, because error->check is still nil at + * this point. + */ + init_check->init_called++; +} + +static void +test_error_private_copy (const TestErrorPrivate *src_priv, + TestErrorPrivate *dest_priv) +{ + dest_priv->foo = src_priv->foo; + dest_priv->check = src_priv->check; + + dest_priv->check->copy_called++; +} + +static void +test_error_private_clear (TestErrorPrivate *priv) +{ + priv->check->free_called++; +} + +G_DEFINE_EXTENDED_ERROR (TestError, test_error) + +static TestErrorPrivate * +fill_test_error (GError *error, TestErrorCheck *check) +{ + TestErrorPrivate *test_error = test_error_get_private (error); + + test_error->check = check; + + return test_error; +} + +static void +test_extended (void) +{ + TestErrorCheck check = { 0, 0, 0 }; + GError *error; + TestErrorPrivate *test_priv; + GError *copy_error; + TestErrorPrivate *copy_test_priv; + + init_check = ✓ + error = g_error_new_literal (TEST_ERROR, 0, "foo"); + test_priv = fill_test_error (error, &check); + + g_assert_cmpint (check.init_called, ==, 1); + g_assert_cmpint (check.copy_called, ==, 0); + g_assert_cmpint (check.free_called, ==, 0); + + g_assert_cmpuint (error->domain, ==, TEST_ERROR); + g_assert_cmpint (test_priv->foo, ==, 13); + + copy_error = g_error_copy (error); + g_assert_cmpint (check.init_called, ==, 2); + g_assert_cmpint (check.copy_called, ==, 1); + g_assert_cmpint (check.free_called, ==, 0); + + g_assert_cmpuint (error->domain, ==, copy_error->domain); + g_assert_cmpint (error->code, ==, copy_error->code); + g_assert_cmpstr (error->message, ==, copy_error->message); + + copy_test_priv = test_error_get_private (copy_error); + g_assert_cmpint (test_priv->foo, ==, copy_test_priv->foo); + + g_error_free (error); + g_error_free (copy_error); + + g_assert_cmpint (check.init_called, ==, 2); + g_assert_cmpint (check.copy_called, ==, 1); + g_assert_cmpint (check.free_called, ==, 2); +} + int main (int argc, char *argv[]) { @@ -91,6 +190,7 @@ main (int argc, char *argv[]) g_test_add_func ("/error/prefix", test_prefix); g_test_add_func ("/error/literal", test_literal); g_test_add_func ("/error/copy", test_copy); + g_test_add_func ("/error/extended", test_extended); return g_test_run (); } From 3af6849881d9a5895b0cddcc4255d75bc206bcd8 Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Sat, 25 Jul 2020 11:22:37 +0200 Subject: [PATCH 3/5] glib.supp: Ignore one-off allocations in error registration. --- glib.supp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/glib.supp b/glib.supp index 06c5f542f..792d5a51b 100644 --- a/glib.supp +++ b/glib.supp @@ -1038,3 +1038,26 @@ ... fun:g_set_prgname } + +# Error domains hash +{ + g_error_init + Memcheck:Leak + match-leak-kinds: reachable + fun:malloc + ... + fun:g_hash_table_new_full + fun:g_error_init +} + +# Error domain static registration +{ + g_error_domain_register_static + Memcheck:Leak + match-leak-kinds: reachable + fun:malloc + ... + fun:g_hash_table_insert + fun:error_domain_register + fun:g_error_domain_register_static +} From 3ee17447cf4a134cda7311983eb7ab6dcf57cd2d Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Sat, 25 Jul 2020 18:03:47 +0200 Subject: [PATCH 4/5] glib.supp: Ignore allocations while adding quarks --- glib.supp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/glib.supp b/glib.supp index 792d5a51b..e89fd87fc 100644 --- a/glib.supp +++ b/glib.supp @@ -1061,3 +1061,13 @@ fun:error_domain_register fun:g_error_domain_register_static } + +{ + new_quark + Memcheck:Leak + match-leak-kinds:reachable + fun:malloc + ... + fun:g_hash_table_insert + fun:quark_new +} From 80014804e2cb603df327f884719a70ca22456dcc Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Sun, 20 Dec 2020 01:09:25 +0100 Subject: [PATCH 5/5] gerror: Inform valgrind about our memory trickery This is mostly duplicated code from gtype.c. --- glib/gerror.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/glib/gerror.c b/glib/gerror.c index dd69283eb..c54375364 100644 --- a/glib/gerror.c +++ b/glib/gerror.c @@ -489,6 +489,7 @@ #include "config.h" +#include "gvalgrind.h" #include #include "gerror.h" @@ -680,7 +681,22 @@ g_error_allocate (GQuark domain, ErrorDomainInfo *out_info) memset (out_info, 0, sizeof (*out_info)); private_size = 0; } - allocated = g_slice_alloc0 (private_size + sizeof (GError)); + /* See comments in g_type_create_instance in gtype.c to see what + * this magic is about. + */ +#ifdef ENABLE_VALGRIND + if (private_size > 0 && RUNNING_ON_VALGRIND) + { + private_size += ALIGN_STRUCT (1); + allocated = g_slice_alloc0 (private_size + sizeof (GError) + sizeof (gpointer)); + *(gpointer *) (allocated + private_size + sizeof (GError)) = allocated + ALIGN_STRUCT (1); + VALGRIND_MALLOCLIKE_BLOCK (allocated + private_size, sizeof (GError) + sizeof (gpointer), 0, TRUE); + VALGRIND_MALLOCLIKE_BLOCK (allocated + ALIGN_STRUCT (1), private_size - ALIGN_STRUCT (1), 0, TRUE); + } + else +#endif + allocated = g_slice_alloc0 (private_size + sizeof (GError)); + error = (GError *) (allocated + private_size); return error; } @@ -826,6 +842,21 @@ g_error_free (GError *error) g_free (error->message); allocated = ((guint8 *) error) - private_size; + /* See comments in g_type_free_instance in gtype.c to see what this + * magic is about. + */ +#ifdef ENABLE_VALGRIND + if (private_size > 0 && RUNNING_ON_VALGRIND) + { + private_size += ALIGN_STRUCT (1); + allocated -= ALIGN_STRUCT (1); + *(gpointer *) (allocated + private_size + sizeof (GError)) = NULL; + g_slice_free1 (private_size + sizeof (GError) + sizeof (gpointer), allocated); + VALGRIND_FREELIKE_BLOCK (allocated + ALIGN_STRUCT (1), 0); + VALGRIND_FREELIKE_BLOCK (error, 0); + } + else +#endif g_slice_free1 (private_size + sizeof (GError), allocated); }