From ac7b60c83e696e02b333b3ae0ddc14aef2b6a439 Mon Sep 17 00:00:00 2001 From: Krzesimir Nowak Date: Fri, 17 Jun 2016 23:05:58 +0200 Subject: [PATCH] Extend GError to store extra data in the key-value store Extra data can be used to store additional context of the failure. --- docs/reference/glib/glib-sections.txt | 5 + glib/gerror.c | 218 ++++++++++++++++++++++++++ glib/gerror.h | 35 +++++ glib/tests/error.c | 105 +++++++++++++ 4 files changed, 363 insertions(+) diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt index f70d869773..eed8a088e0 100644 --- a/docs/reference/glib/glib-sections.txt +++ b/docs/reference/glib/glib-sections.txt @@ -693,18 +693,23 @@ g_macro__has_feature Error Reporting error_reporting GError +GErrorDataCopyFunc g_error_new g_error_new_literal g_error_new_valist g_error_free g_error_copy g_error_matches +g_error_add_extra_data +g_error_get_extra_data +g_error_get_extra_data_keys g_set_error g_set_error_literal g_propagate_error g_clear_error g_prefix_error g_propagate_prefixed_error +g_add_error_extra_data
diff --git a/glib/gerror.c b/glib/gerror.c index b190d68810..d47f4a2e8b 100644 --- a/glib/gerror.c +++ b/glib/gerror.c @@ -369,12 +369,22 @@ * 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.62 #GError can also hold extra data in the key-value + * store. It is recommended to specify some rules about adding and + * modifying extra data. For example documenting what keys will be + * added for each error domain or error code and what is the type of + * the extra data for each key; documenting which extra data can be + * modified during propagation (say, propagators can append a string + * to the GPtrArray holding strings under the "context" key). */ #include "config.h" #include "gerror.h" +#include "gdataset.h" +#include "ghash.h" #include "gslice.h" #include "gstrfuncs.h" #include "gtestutils.h" @@ -492,11 +502,72 @@ g_error_free (GError *error) { g_return_if_fail (error != NULL); + g_dataset_destroy (error); g_free (error->message); g_slice_free (GError, error); } +typedef struct +{ + gpointer data; + GErrorDataCopyFunc copy; + GDestroyNotify destroy; +} GErrorExtraDataNode; + +static GErrorExtraDataNode* +g_error_extra_data_node_new (gpointer data, + GErrorDataCopyFunc copy, + GDestroyNotify destroy) +{ + GErrorExtraDataNode *node; + + node = g_slice_new (GErrorExtraDataNode); + + node->data = data; + node->copy = copy; + node->destroy = destroy; + + return node; +} + +static GErrorExtraDataNode* +g_error_extra_data_node_copy (GErrorExtraDataNode *node) +{ + gpointer data_copy = node->data; + + if (node->copy != NULL) + data_copy = node->copy (node->data); + + return g_error_extra_data_node_new (data_copy, node->copy, node->destroy); +} + +static void +g_error_extra_data_node_destroy (gpointer ptr) +{ + GErrorExtraDataNode *node = ptr; + + if (node->destroy) + node->destroy (node->data); + + g_slice_free (GErrorExtraDataNode, node); +} + +static void +copy_extra_data (GQuark key_id, + gpointer data, + gpointer user_data) +{ + GError *copy = user_data; + GErrorExtraDataNode *extra_data = data; + + g_dataset_id_set_data_full (copy, + key_id, + g_error_extra_data_node_copy (extra_data), + g_error_extra_data_node_destroy); +} + + /** * g_error_copy: * @error: a #GError @@ -520,6 +591,9 @@ g_error_copy (const GError *error) *copy = *error; copy->message = g_strdup (error->message); + g_dataset_foreach (error, + copy_extra_data, + copy); return copy; } @@ -553,6 +627,112 @@ g_error_matches (const GError *error, error->code == code; } +/** + * g_error_add_extra_data: + * @error: a #GError + * @key: a key + * @extra_data: data to add under the @key + * @copy: function for copying @extra_data + * @destroy: function for destroying @extra_data + * + * Adds extra data to the key-value store inside @error under the + * @key. When @error is copied with g_error_copy() extra data will be + * also copied if @copy is not %NULL; otherwise it will be just a + * shallow copy. When @error is destroyed with g_error_free(), extra + * data will be freed with @destroy, if @destroy is not %NULL. + * + * Both @copy and @destroy must be either %NULL or not - it is not + * allowed to provide only the copy function or only the destroy + * function. Also please note, that @extra_data is not checked for + * %NULL to omit copying or destructing. + * + * Since: 2.62 + */ +void +g_error_add_extra_data (GError *error, + const gchar *key, + gpointer extra_data, + GErrorDataCopyFunc copy, + GDestroyNotify destroy) +{ + g_return_if_fail (error != NULL); + g_return_if_fail (key != NULL); + g_return_if_fail ((copy == NULL && destroy == NULL) || (copy != NULL && destroy != NULL)); + + g_dataset_set_data_full (error, + key, + g_error_extra_data_node_new (extra_data, + copy, + destroy), + g_error_extra_data_node_destroy); +} + +/** + * g_error_get_extra_data: + * @error: a #GError + * @key: a key + * @exists: (out) (optional): whether the key exists + * + * Returns: (transfer none): an extra data + * + * Since: 2.62 + */ +gpointer +g_error_get_extra_data (const GError *error, + const gchar *key, + gboolean *exists) +{ + GErrorExtraDataNode *node; + + g_return_val_if_fail (error != NULL, NULL); + g_return_val_if_fail (key != NULL, NULL); + + node = g_dataset_get_data (error, key); + if (exists != NULL) + *exists = node != NULL; + if (node != NULL) + return node->data; + return NULL; +} + +static void +get_extra_key (GQuark key_id, + gpointer data, + gpointer user_data) +{ + GPtrArray *keys = user_data; + + /* cast to ignore the warning about discard const qualifier - we are + * not freeing those strings anyway. + */ + g_ptr_array_add (keys, (gpointer)g_quark_to_string (key_id)); +} + +/** + * g_error_get_extra_data_keys: + * @error: a #GError + * + * Gets all the keys in the @error as a %NULL-terminated array. The + * array should be freed with g_free(). + * + * Returns: (transfer container): an array of keys + * + * Since: 2.62 + */ +const gchar** +g_error_get_extra_data_keys (const GError *error) +{ + GPtrArray *keys; + + g_return_val_if_fail (error != NULL, NULL); + + keys = g_ptr_array_new (); + g_dataset_foreach (error, get_extra_key, keys); + g_ptr_array_add (keys, NULL); + + return (const gchar**) g_ptr_array_free (keys, FALSE); +} + #define ERROR_OVERWRITTEN_WARNING "GError set over the top of a previous GError or uninitialized memory.\n" \ "This indicates a bug in someone's code. You must ensure an error is NULL before it's set.\n" \ "The overwriting error message was: %s" @@ -756,3 +936,41 @@ g_propagate_prefixed_error (GError **dest, va_end (ap); } } + +/** + * g_add_error_extra_data: + * @error: a #GError return location + * @key: a key + * @extra_data: data to add under the @key + * @copy: function for copying @extra_data + * @destroy: function for destroying @extra_data + * + * If @error is %NULL, frees @extra_data with @destroy if @destroy is + * not %NULL. Otherwise adds extra data to the key-value store inside + * @error under the @key. When @error is copied with g_error_copy() + * extra data will be also copied if @copy is not %NULL; otherwise it + * will be just a shallow copy. When @error is destroyed with + * g_error_free(), extra data will be freed with @destroy, if @destroy + * is not %NULL. + * + * Both @copy and @destroy must be either %NULL or not - it is not + * allowed to provide only the copy function or only the destroy + * function. Also please note, that @extra_data is not checked for + * %NULL to omit copying or destructing. + * + * If @error is not %NULL, then *@error can't be %NULL too. + * + * Since: 2.62 + */ +void +g_add_error_extra_data (GError **error, + const gchar *key, + gpointer extra_data, + GErrorDataCopyFunc copy, + GDestroyNotify destroy) +{ + if (error != NULL) + g_error_add_extra_data (*error, key, extra_data, copy, destroy); + else if (destroy != NULL) + destroy (extra_data); +} diff --git a/glib/gerror.h b/glib/gerror.h index 8ecff04e16..fd28b08444 100644 --- a/glib/gerror.h +++ b/glib/gerror.h @@ -47,6 +47,19 @@ struct _GError gchar *message; }; +/** + * GErrorDataCopyFunc: + * @data: data to copy + * + * A function type for copying #GError extra data. It must handle the + * case when @data is %NULL. + * + * Returns: a copy of @data. + * + * Since: 2.62 + */ +typedef gpointer (*GErrorDataCopyFunc) (gpointer data); + GLIB_AVAILABLE_IN_ALL GError* g_error_new (GQuark domain, gint code, @@ -72,6 +85,21 @@ GLIB_AVAILABLE_IN_ALL gboolean g_error_matches (const GError *error, GQuark domain, gint code); +GLIB_AVAILABLE_IN_2_62 +void g_error_add_extra_data (GError *error, + const gchar *key, + gpointer extra_data, + GErrorDataCopyFunc copy, + GDestroyNotify destroy); + +GLIB_AVAILABLE_IN_2_62 +gpointer g_error_get_extra_data (const GError *error, + const gchar *key, + gboolean *exists); + +GLIB_AVAILABLE_IN_2_62 +const gchar** g_error_get_extra_data_keys (const GError *error); + /* if (err) *err = g_error_new(domain, code, format, ...), also has * some sanity checks. @@ -112,6 +140,13 @@ void g_propagate_prefixed_error (GError **dest, const gchar *format, ...) G_GNUC_PRINTF (3, 4); +GLIB_AVAILABLE_IN_2_62 +void g_add_error_extra_data (GError **error, + const gchar *key, + gpointer extra_data, + GErrorDataCopyFunc copy, + GDestroyNotify destroy); + G_END_DECLS #endif /* __G_ERROR_H__ */ diff --git a/glib/tests/error.c b/glib/tests/error.c index ebbd965dc0..aa1c2619e8 100644 --- a/glib/tests/error.c +++ b/glib/tests/error.c @@ -82,6 +82,110 @@ test_copy (void) g_error_free (copy); } +typedef struct +{ + guint copy_calls; + guint free_calls; +} ExtraDataStats; + +static gpointer +extra_data_stats_fake_copy (gpointer data) +{ + ExtraDataStats *stats = data; + + stats->copy_calls++; + + return stats; +} + +static void +extra_data_stats_fake_free (gpointer data) +{ + ExtraDataStats *stats = data; + + stats->free_calls++; +} + +static void +test_extra_data (void) +{ + GError *error; + GError *copy; + ExtraDataStats stats = { 0, 0 }; + gboolean exists; + const gchar** keys; + + error = g_error_new_literal (G_MARKUP_ERROR, G_MARKUP_ERROR_EMPTY, "foo"); + copy = NULL; + + /* check if we get an empty strings array when there are no extra + * data + */ + keys = g_error_get_extra_data_keys (error); + g_assert_nonnull (keys); + g_assert_cmpuint (g_strv_length ((gchar**)keys), ==, 0); + g_free (g_steal_pointer (&keys)); + + /* check if extra data is stolen, so no copies happen */ + g_add_error_extra_data (&error, "test", &stats, extra_data_stats_fake_copy, extra_data_stats_fake_free); + g_assert_cmpuint (stats.copy_calls, ==, 0); + g_assert_cmpuint (stats.free_calls, ==, 0); + + /* check if extra data is freed when no error is passed */ + g_add_error_extra_data (NULL, "test", &stats, extra_data_stats_fake_copy, extra_data_stats_fake_free); + g_assert_cmpuint (stats.copy_calls, ==, 0); + g_assert_cmpuint (stats.free_calls, ==, 1); + + /* check if extra data is freed when being replaced */ + g_add_error_extra_data (&error, "test", &stats, extra_data_stats_fake_copy, extra_data_stats_fake_free); + g_assert_cmpuint (stats.copy_calls, ==, 0); + g_assert_cmpuint (stats.free_calls, ==, 2); + + /* add another extra data */ + g_add_error_extra_data (&error, "test2", &stats, extra_data_stats_fake_copy, extra_data_stats_fake_free); + g_assert_cmpuint (stats.copy_calls, ==, 0); + g_assert_cmpuint (stats.free_calls, ==, 2); + + /* check if copy func was called twice, once for "test" and once for + * "test2" + */ + copy = g_error_copy (error); + g_assert_cmpuint (stats.copy_calls, ==, 2); + g_assert_cmpuint (stats.free_calls, ==, 2); + + g_assert_nonnull (g_error_get_extra_data (copy, "test", &exists)); + g_assert_true (exists); + g_assert_null (g_error_get_extra_data (copy, "foo", &exists)); + g_assert_false (exists); + /* also possible to pass NULL for exists parameter */ + g_assert_null (g_error_get_extra_data (copy, "bar", NULL)); + g_assert_nonnull (g_error_get_extra_data (copy, "test", NULL)); + /* ensure no copying happened in extra data getters */ + g_assert_cmpuint (stats.copy_calls, ==, 2); + g_assert_cmpuint (stats.free_calls, ==, 2); + + /* check if we get the keys we expect (test and test2) */ + keys = g_error_get_extra_data_keys (copy); + g_assert_nonnull (keys); + g_assert_cmpuint (g_strv_length ((gchar**)keys), ==, 2); + g_assert_true (g_strv_contains (keys, "test")); + g_assert_true (g_strv_contains (keys, "test2")); + g_free (g_steal_pointer (&keys)); + /* ensure no copying happened in extra data keys getter */ + g_assert_cmpuint (stats.copy_calls, ==, 2); + g_assert_cmpuint (stats.free_calls, ==, 2); + + /* check if extra data is freed when error is freed */ + g_error_free (g_steal_pointer (&error)); + g_assert_cmpuint (stats.copy_calls, ==, 2); + g_assert_cmpuint (stats.free_calls, ==, 4); + + /* check if extra data is freed when error is freed */ + g_clear_error (©); + g_assert_cmpuint (stats.copy_calls, ==, 2); + g_assert_cmpuint (stats.free_calls, ==, 6); +} + int main (int argc, char *argv[]) { @@ -91,6 +195,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/extra-data", test_extra_data); return g_test_run (); } -- GitLab