From c5d2417d07d055e6b91f2573a1a0237b60475393 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Sun, 10 Jun 2018 12:20:34 +0100 Subject: [PATCH 01/19] Add refcounted data It is useful to provide a "reference counted allocation" API that can add reference counting semantics to any memory allocation. This allows turning data structures that usually are placed on the stack into memory that can be placed on the heap without: - adding a public reference count field - implementing copy/free semantics This mechanism is similar to Rust's Rc> combination of traits, and uses a Valgrind-friendly overallocation mechanism to store the reference count into a private data segment, like we do with GObject's private instance data. --- docs/reference/glib/glib-docs.xml | 1 + docs/reference/glib/glib-sections.txt | 12 + glib/Makefile.am | 2 + glib/glib.h | 1 + glib/grcbox.c | 398 ++++++++++++++++++++++++++ glib/grcbox.h | 55 ++++ glib/meson.build | 2 + glib/tests/Makefile.am | 1 + glib/tests/meson.build | 1 + glib/tests/rcbox.c | 82 ++++++ 10 files changed, 555 insertions(+) create mode 100644 glib/grcbox.c create mode 100644 glib/grcbox.h create mode 100644 glib/tests/rcbox.c diff --git a/docs/reference/glib/glib-docs.xml b/docs/reference/glib/glib-docs.xml index 26cdafb67..d7ac0a500 100644 --- a/docs/reference/glib/glib-docs.xml +++ b/docs/reference/glib/glib-docs.xml @@ -120,6 +120,7 @@ + diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt index 01da779e8..fa83dd832 100644 --- a/docs/reference/glib/glib-sections.txt +++ b/docs/reference/glib/glib-sections.txt @@ -3466,3 +3466,15 @@ g_atomic_ref_count_inc g_atomic_ref_count_dec g_atomic_ref_count_compare + +
+rcbox +g_rc_box_alloc +g_rc_box_alloc0 +g_rc_box_new +g_rc_box_new0 +g_rc_box_dup +g_rc_box_acquire +g_rc_box_release +g_rc_box_release_full +
diff --git a/glib/Makefile.am b/glib/Makefile.am index 7252a67d0..8ec5f4454 100644 --- a/glib/Makefile.am +++ b/glib/Makefile.am @@ -149,6 +149,7 @@ libglib_2_0_la_SOURCES = \ gquark.c \ gqueue.c \ grand.c \ + grcbox.c \ grefcount.c \ gregex.c \ gscanner.c \ @@ -286,6 +287,7 @@ glibsubinclude_HEADERS = \ gquark.h \ gqueue.h \ grand.h \ + grcbox.h \ grefcount.h \ gregex.h \ gscanner.h \ diff --git a/glib/glib.h b/glib/glib.h index 84299c4f9..309366281 100644 --- a/glib/glib.h +++ b/glib/glib.h @@ -69,6 +69,7 @@ #include #include #include +#include #include #include #include diff --git a/glib/grcbox.c b/glib/grcbox.c new file mode 100644 index 000000000..0860634be --- /dev/null +++ b/glib/grcbox.c @@ -0,0 +1,398 @@ +/* grcbox.h: Reference counted data + * + * Copyright 2018 Emmanuele Bassi + * + * 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.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 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * 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, see . + */ + +#include "config.h" + +#include "grcbox.h" + +#include "gmessages.h" +#include "grefcount.h" + +#ifdef ENABLE_VALGRIND +#include "valgrind.h" +#endif + +#include + +/** + * SECTION:rcbox + * @Title: Reference counted data + * @Short_description: Allocated memory with reference counting semantics + * + * A "reference counted box", or "RcBox", is an opaque wrapper data type + * that is guaranteed to be as big as the size of a given data type, and + * which augments the given data type with reference counting semantics + * for its memory management. + * + * RcBox is useful if you have a plain old data type, like a structure + * typically placed on the stack, and you wish to provide additional API + * to use it on the heap, without necessarily implementing copy/free + * semantics, or your own reference counting. + * + * The typical use is: + * + * |[ + * typedef struct { + * float x, y; + * } Point; + * + * Point * + * point_new (float x, float y) + * { + * Point *res = g_rc_box_new (Point); + * + * res->x = x; + * res->y = y; + * + * return res; + * } + * ]| + * + * Every time you wish to acquire a reference on the memory, you should + * call g_rc_box_acquire(); similarly, when you wish to release a reference + * you should call g_rc_box_release(): + * + * |[ + * Point * + * point_ref (Point *p) + * { + * return g_rc_box_acquire (p); + * } + * + * void + * point_unref (Point *p) + * { + * g_rc_box_release (p); + * } + * ]| + * + * If you have additional memory allocated inside the structure, you can + * use g_rc_box_release_full(), which takes a function pointer, which + * will be called if the reference released was the last: + * + * |[ + * typedef struct { + * char *name; + * char *address; + * char *city; + * char *state; + * int age; + * } Person; + * + * void + * person_clear (Person *p) + * { + * g_free (p->name); + * g_free (p->address); + * g_free (p->city); + * g_free (p->state); + * } + * + * void + * person_unref (Person *p) + * { + * g_rc_box_release_full (p, (GDestroyNotify) person_clear); + * } + * ]| + * + * If you wish to transfer the ownership of a reference counted data + * type without increasing the reference count, you can use g_steal_pointer(): + * + * |[ + * Person *p = g_rc_box_new (Person); + * + * fill_person_details (p); + * + * add_person_to_database (db, g_steal_pointer (&p)); + * ]| + * + * The reference counting operations on data allocated using g_rc_box_alloc(), + * g_rc_box_new(), and g_rc_box_dup() are not thread safe; it is your code's + * responsibility to ensure that references are acquired are released on the + * same thread. + * + * Since: 2.58. + */ + +typedef struct { + grefcount ref_count; + + gsize mem_size; + +#ifndef G_DISABLE_ASSERT + /* A "magic" number, used to perform additional integrity + * checks on the allocated data + */ + guint32 magic; +#endif +} GRcBox; + +#define G_RC_BOX_MAGIC 0x44ae2bf0 +#define G_RC_BOX_SIZE sizeof (GRcBox) +#define G_RC_BOX(p) (GRcBox *) (((char *) (p)) - G_RC_BOX_SIZE) + +/* We use the same alignment as GTypeInstance and GNU libc's malloc */ +#define STRUCT_ALIGNMENT (2 * sizeof (gsize)) +#define ALIGN_STRUCT(offset) ((offset + (STRUCT_ALIGNMENT - 1)) & -STRUCT_ALIGNMENT) + +static gpointer +g_rc_box_alloc_full (gsize block_size, + gboolean clear) +{ + gsize private_size = G_RC_BOX_SIZE; + gsize real_size = private_size + block_size; + char *allocated; + GRcBox *real_box; + +#ifdef ENABLE_VALGRIND + if (RUNNING_ON_VALGRIND) + { + /* When running under Valgrind we massage the memory allocation + * to include a pointer at the tail end of the block; the pointer + * is then set to the start of the block. This trick allows + * Valgrind to keep track of the over-allocation and not be + * confused when passing the pointer around + */ + private_size += ALIGN_STRUCT (1); + + if (clear) + allocated = g_malloc0 (real_size + sizeof (gpointer)); + else + allocated = g_malloc (real_size + sizeof (gpointer)); + + *(gpointer *) (allocated + private_size + block_size) = allocated + ALIGN_STRUCT (1); + + VALGRIND_MALLOCLIKE_BLOCK (allocated + private_size, block_size + sizeof (gpointer), 0, TRUE); + VALGRIND_MALLOCLIKE_BLOCK (allocated + ALIGN_STRUCT (1), private_size - ALIGN_STRUCT (1), 0, TRUE); + } + else +#endif /* ENABLE_VALGRIND */ + { + if (clear) + allocated = g_malloc0 (real_size); + else + allocated = g_malloc (real_size); + } + + real_box = (GRcBox *) allocated; + real_box->mem_size = block_size; +#ifndef G_DISABLE_ASSERT + real_box->magic = G_RC_BOX_MAGIC; +#endif + g_ref_count_init (&real_box->ref_count); + + return allocated + private_size; +} + +/** + * g_rc_box_alloc: + * @block_size: the size of the allocation + * + * Allocates @block_size bytes of memory, and adds reference + * counting semantics to it. + * + * The data will be freed when its reference count drops to + * zero. + * + * Returns: a pointer to the allocated memory + * + * Since: 2.58 + */ +gpointer +g_rc_box_alloc (gsize block_size) +{ + g_return_val_if_fail (block_size > 0, NULL); + + return g_rc_box_alloc_full (block_size, FALSE); +} + +/** + * g_rc_box_alloc0: + * @block_size: the size of the allocation + * + * Allocates @block_size bytes of memory, and adds reference + * counting semantics to it. + * + * The contents of the returned data is set to 0's. + * + * The data will be freed when its reference count drops to + * zero. + * + * Returns: a pointer to the allocated memory + * + * Since: 2.58 + */ +gpointer +g_rc_box_alloc0 (gsize block_size) +{ + g_return_val_if_fail (block_size > 0, NULL); + + return g_rc_box_alloc_full (block_size, TRUE); +} + +/** + * g_rc_box_new: + * @type: the type to allocate, typically a structure name + * + * A convenience macro to allocate reference counted data with + * the size of the given @type. + * + * This macro calls g_rc_box_alloc() with `sizeof (@type)` and + * casts the returned pointer to a pointer of the given @type, + * avoiding a type cast in the source code. + * + * This macro cannot return %NULL, as the minimum allocation + * size from `sizeof (@type)` is 1 byte. + * + * Returns: (not nullable): a pointer to the allocated memory, + * cast to a pointer for the given @type + * + * Since: 2.58 + */ + +/** + * g_rc_box_new0: + * @type: the type to allocate, typically a structure name + * + * A convenience macro to allocate reference counted data with + * the size of the given @type, and set its contents to 0. + * + * This macro calls g_rc_box_alloc0() with `sizeof (@type)` and + * casts the returned pointer to a pointer of the given @type, + * avoiding a type cast in the source code. + * + * This macro cannot return %NULL, as the minimum allocation + * size from `sizeof (@type)` is 1 byte. + * + * Returns: (not nullable): a pointer to the allocated memory, + * cast to a pointer for the given @type + * + * Since: 2.58 + */ + +/** + * g_rc_box_dup: + * @mem_block: (not nullable): a pointer to reference counted data + * + * Allocates a new block of data with reference counting + * semantics, and copies the contents of @mem_block into + * it. + * + * Returns: (not nullable): a pointer to the allocated memory + * + * Since: 2.58 + */ +gpointer +(g_rc_box_dup) (gpointer mem_block) +{ + GRcBox *real_box = G_RC_BOX (mem_block); + gpointer res; + + g_return_val_if_fail (mem_block != NULL, NULL); +#ifndef G_DISABLE_ASSERT + g_return_val_if_fail (real_box->magic == G_RC_BOX_MAGIC, NULL); +#endif + + res = g_rc_box_alloc_full (real_box->mem_size, FALSE); + memcpy (res, mem_block, real_box->mem_size); + + return res; +} + +/** + * g_rc_box_acquire: + * @mem_block: (not nullable): a pointer to reference counted data + * + * Acquires a reference on the data pointed by @mem_block. + * + * Returns: (not nullabl): a pointer to the data, with its reference + * count increased + * + * Since: 2.58 + */ +gpointer +(g_rc_box_acquire) (gpointer mem_block) +{ + GRcBox *real_box = G_RC_BOX (mem_block); + + g_return_val_if_fail (mem_block != NULL, NULL); +#ifndef G_DISABLE_ASSERT + g_return_val_if_fail (real_box->magic == G_RC_BOX_MAGIC, NULL); +#endif + + g_ref_count_inc (&real_box->ref_count); + + return mem_block; +} + +/** + * g_rc_box_release: + * @mem_block: (not nullable): a pointer to reference counted data + * + * Releases a reference on the data pointed by @mem_block. + * + * If the reference was the last one, it will free the + * resources allocated for @mem_block. + * + * Since: 2.58 + */ +void +g_rc_box_release (gpointer mem_block) +{ + GRcBox *real_box = G_RC_BOX (mem_block); + + g_return_if_fail (mem_block != NULL); +#ifndef G_DISABLE_ASSERT + g_return_if_fail (real_box->magic == G_RC_BOX_MAGIC); +#endif + + if (g_ref_count_dec (&real_box->ref_count)) + g_free (real_box); +} + +/** + * g_rc_box_release_full: + * @mem_block: (not nullabl): a pointer to reference counted data + * @clear_func: (not nullable): a function to call when clearing the data + * + * Releases a reference on the data pointed by @mem_block. + * + * If the reference was the last one, it will call @clear_func + * to clear the contents of @mem_block, and then will free the + * resources allocated for @mem_block. + * + * Since: 2.58 + */ +void +g_rc_box_release_full (gpointer mem_block, + GDestroyNotify clear_func) +{ + GRcBox *real_box = G_RC_BOX (mem_block); + + g_return_if_fail (mem_block != NULL); + g_return_if_fail (clear_func != NULL); +#ifndef G_DISABLE_ASSERT + g_return_if_fail (real_box->magic == G_RC_BOX_MAGIC); +#endif + + if (g_ref_count_dec (&real_box->ref_count)) + { + clear_func (mem_block); + g_free (real_box); + } +} diff --git a/glib/grcbox.h b/glib/grcbox.h new file mode 100644 index 000000000..8cd44037a --- /dev/null +++ b/glib/grcbox.h @@ -0,0 +1,55 @@ +/* grcbox.h: Reference counted data + * + * Copyright 2018 Emmanuele Bassi + * + * 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.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 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * 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, see . + */ + +#pragma once + +#if !defined (__GLIB_H_INSIDE__) && !defined (GLIB_COMPILATION) +#error "Only can be included directly." +#endif + +#include + +G_BEGIN_DECLS + +GLIB_AVAILABLE_IN_2_58 +gpointer g_rc_box_alloc (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); +GLIB_AVAILABLE_IN_2_58 +gpointer g_rc_box_alloc0 (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); +GLIB_AVAILABLE_IN_2_58 +gpointer g_rc_box_dup (gpointer mem_block) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); +GLIB_AVAILABLE_IN_2_58 +gpointer g_rc_box_acquire (gpointer mem_block); +GLIB_AVAILABLE_IN_2_58 +void g_rc_box_release (gpointer mem_block); +GLIB_AVAILABLE_IN_2_58 +void g_rc_box_release_full (gpointer mem_block, + GDestroyNotify clear_func); + +#define g_rc_box_new(type) \ + ((type *) g_rc_box_alloc (sizeof (type))) +#define g_rc_box_new0(type) \ + ((type *) g_rc_box_alloc0 (sizeof (type))) + +#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) && !defined(_cplusplus) +/* Type check to avoid assigning references to different types */ +# define g_rc_box_acquire(mem_block) ((__typeof__(mem_block)) (g_rc_box_acquire) (mem_block)) +/* Type check to avoid duplicating data to different types */ +# define g_rc_box_dup(mem_block) ((__typeof__(mem_block)) (g_rc_box_dup) (mem_block)) +#endif + +G_END_DECLS diff --git a/glib/meson.build b/glib/meson.build index 76d354c2a..73ab4928f 100644 --- a/glib/meson.build +++ b/glib/meson.build @@ -76,6 +76,7 @@ glib_sub_headers = files( 'gquark.h', 'gqueue.h', 'grand.h', + 'grcbox.h', 'grefcount.h', 'gregex.h', 'gscanner.h', @@ -160,6 +161,7 @@ glib_sources = files( 'gquark.c', 'gqueue.c', 'grand.c', + 'grcbox.c', 'grefcount.c', 'gregex.c', 'gscanner.c', diff --git a/glib/tests/Makefile.am b/glib/tests/Makefile.am index 7289b419e..0d524d57f 100644 --- a/glib/tests/Makefile.am +++ b/glib/tests/Makefile.am @@ -89,6 +89,7 @@ test_programs = \ protocol \ queue \ rand \ + rcbox \ rec-mutex \ regex \ rwlock \ diff --git a/glib/tests/meson.build b/glib/tests/meson.build index cf05bc74f..3c20bad39 100644 --- a/glib/tests/meson.build +++ b/glib/tests/meson.build @@ -46,6 +46,7 @@ glib_tests = [ 'protocol', 'queue', 'rand', + 'rcbox', 'rec-mutex', 'refcount', 'refcount-macro', diff --git a/glib/tests/rcbox.c b/glib/tests/rcbox.c new file mode 100644 index 000000000..19b3842e5 --- /dev/null +++ b/glib/tests/rcbox.c @@ -0,0 +1,82 @@ +#include + +typedef struct { + float x, y; +} Point; + +static Point *global_point; + +static void +test_rcbox_new (void) +{ + Point *a = g_rc_box_new (Point); + + g_assert_nonnull (a); + + g_rc_box_release (a); + + a = g_rc_box_new0 (Point); + g_assert_nonnull (a); + g_assert_cmpfloat (a->x, ==, 0.f); + g_assert_cmpfloat (a->y, ==, 0.f); + + g_rc_box_release (a); +} + +static void +point_clear (Point *p) +{ + g_assert_nonnull (p); + + g_assert_cmpfloat (p->x, ==, 42.0f); + g_assert_cmpfloat (p->y, ==, 47.0f); + + g_assert_true (global_point == p); + global_point = NULL; +} + +static void +test_rcbox_release_full (void) +{ + Point *p = g_rc_box_new (Point); + + g_assert_nonnull (p); + global_point = p; + + p->x = 42.0f; + p->y = 47.0f; + + g_assert_true (g_rc_box_acquire (p) == p); + + g_rc_box_release_full (p, (GDestroyNotify) point_clear); + g_assert_nonnull (global_point); + g_assert_true (p == global_point); + + g_rc_box_release_full (p, (GDestroyNotify) point_clear); + g_assert_null (global_point); +} + +static void +test_rcbox_dup (void) +{ + Point *a = g_rc_box_new (Point); + Point *b = g_rc_box_dup (a); + + g_assert_true (a != b); + + g_rc_box_release (a); + g_rc_box_release (b); +} + +int +main (int argc, + char *argv[]) +{ + g_test_init (&argc, &argv, NULL); + + g_test_add_func ("/rcbox/new", test_rcbox_new); + g_test_add_func ("/rcbox/dup", test_rcbox_dup); + g_test_add_func ("/rcbox/release-full", test_rcbox_release_full); + + return g_test_run (); +} From b607927a4338cb6906ac67586268828a9aa8eed8 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Mon, 11 Jun 2018 11:52:54 +0100 Subject: [PATCH 02/19] Add atomically refcounted data GArcBox is the atomic reference counting version of GRcBox. Unlike GRcBox, the reference acquisition and release on GArcBox are guaranteed to be atomic, and thus they can be performed from different threads. This is similar to Rust's Arc> combination of traits. --- docs/reference/glib/glib-docs.xml | 1 + docs/reference/glib/glib-sections.txt | 12 + glib/Makefile.am | 2 + glib/garcbox.c | 333 ++++++++++++++++++++++++++ glib/grcbox.c | 59 +++-- glib/grcbox.h | 29 ++- glib/grcboxprivate.h | 42 ++++ glib/meson.build | 1 + 8 files changed, 447 insertions(+), 32 deletions(-) create mode 100644 glib/garcbox.c create mode 100644 glib/grcboxprivate.h diff --git a/docs/reference/glib/glib-docs.xml b/docs/reference/glib/glib-docs.xml index d7ac0a500..9d253ee5f 100644 --- a/docs/reference/glib/glib-docs.xml +++ b/docs/reference/glib/glib-docs.xml @@ -121,6 +121,7 @@ + diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt index fa83dd832..2a4d1f281 100644 --- a/docs/reference/glib/glib-sections.txt +++ b/docs/reference/glib/glib-sections.txt @@ -3478,3 +3478,15 @@ g_rc_box_acquire g_rc_box_release g_rc_box_release_full + +
+arcbox +g_arc_box_alloc +g_arc_box_alloc0 +g_arc_box_new +g_arc_box_new0 +g_arc_box_dup +g_arc_box_acquire +g_arc_box_release +g_arc_box_release_full +
diff --git a/glib/Makefile.am b/glib/Makefile.am index 8ec5f4454..352ecb0ec 100644 --- a/glib/Makefile.am +++ b/glib/Makefile.am @@ -96,6 +96,7 @@ deprecated_sources = \ libglib_2_0_la_SOURCES = \ $(deprecated_sources) \ glib_probes.d \ + garcbox.c \ garray.c \ gasyncqueue.c \ gasyncqueueprivate.h \ @@ -150,6 +151,7 @@ libglib_2_0_la_SOURCES = \ gqueue.c \ grand.c \ grcbox.c \ + grcboxprivate.h \ grefcount.c \ gregex.c \ gscanner.c \ diff --git a/glib/garcbox.c b/glib/garcbox.c new file mode 100644 index 000000000..ea6e742d8 --- /dev/null +++ b/glib/garcbox.c @@ -0,0 +1,333 @@ +/* garcbox.c: Atomically reference counted data + * + * Copyright 2018 Emmanuele Bassi + * + * 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.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 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * 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, see . + */ + +#include "config.h" + +#include "grcbox.h" + +#include "gmessages.h" +#include "grcboxprivate.h" +#include "grefcount.h" + +#ifdef ENABLE_VALGRIND +#include "valgrind.h" +#endif + +#include + +#define G_ARC_BOX(p) (GArcBox *) (((char *) (p)) - G_ARC_BOX_SIZE) + +/** + * SECTION:arcbox + * @Title: Atomically reference counted data + * @Short_description: Allocated memory with atomic reference counting semantics + * + * An "atomically reference counted box", or "ArcBox", is an opaque wrapper + * data type that is guaranteed to be as big as the size of a given data type, + * and which augments the given data type with thread safe reference counting + * semantics for its memory management. + * + * ArcBox is useful if you have a plain old data type, like a structure + * typically placed on the stack, and you wish to provide additional API + * to use it on the heap, without necessarily implementing copy/free + * semantics, or your own reference counting. + * + * The typical use is: + * + * |[ + * typedef struct { + * float x, y; + * } Point; + * + * Point * + * point_new (float x, float y) + * { + * Point *res = g_arc_box_new (Point); + * + * res->x = x; + * res->y = y; + * + * return res; + * } + * ]| + * + * Every time you wish to acquire a reference on the memory, you should + * call g_arc_box_acquire(); similarly, when you wish to release a reference + * you should call g_arc_box_release(): + * + * |[ + * Point * + * point_ref (Point *p) + * { + * return g_arc_box_acquire (p); + * } + * + * void + * point_unref (Point *p) + * { + * g_arc_box_release (p); + * } + * ]| + * + * If you have additional memory allocated inside the structure, you can + * use g_arc_box_release_full(), which takes a function pointer, which + * will be called if the reference released was the last: + * + * |[ + * typedef struct { + * char *name; + * char *address; + * char *city; + * char *state; + * int age; + * } Person; + * + * void + * person_clear (Person *p) + * { + * g_free (p->name); + * g_free (p->address); + * g_free (p->city); + * g_free (p->state); + * } + * + * void + * person_unref (Person *p) + * { + * g_arc_box_release_full (p, (GDestroyNotify) person_clear); + * } + * ]| + * + * If you wish to transfer the ownership of a reference counted data + * type without increasing the reference count, you can use g_steal_pointer(): + * + * |[ + * Person *p = g_arc_box_new (Person); + * + * fill_person_details (p); + * + * add_person_to_database (db, g_steal_pointer (&p)); + * ]| + * + * The reference counting operations on data allocated using g_arc_box_alloc(), + * g_arc_box_new(), and g_arc_box_dup() are guaranteed to be atomic, and thus + * can be safely be performed by different threads. It is important to note that + * only the reference acquisition and release are atomic; changes to the content + * of the data are your responsibility. + * + * Since: 2.58. + */ + +/** + * g_arc_box_alloc: + * @block_size: the size of the allocation + * + * Allocates @block_size bytes of memory, and adds atomic + * reference counting semantics to it. + * + * The data will be freed when its reference count drops to + * zero. + * + * Returns: a pointer to the allocated memory + * + * Since: 2.58 + */ +gpointer +g_arc_box_alloc (gsize block_size) +{ + g_return_val_if_fail (block_size > 0, NULL); + + return g_rc_box_alloc_full (block_size, TRUE, FALSE); +} + +/** + * g_arc_box_alloc0: + * @block_size: the size of the allocation + * + * Allocates @block_size bytes of memory, and adds atomic + * referenc counting semantics to it. + * + * The contents of the returned data is set to 0's. + * + * The data will be freed when its reference count drops to + * zero. + * + * Returns: a pointer to the allocated memory + * + * Since: 2.58 + */ +gpointer +g_arc_box_alloc0 (gsize block_size) +{ + g_return_val_if_fail (block_size > 0, NULL); + + return g_rc_box_alloc_full (block_size, TRUE, TRUE); +} + +/** + * g_arc_box_new: + * @type: the type to allocate, typically a structure name + * + * A convenience macro to allocate atomically reference counted + * data with the size of the given @type. + * + * This macro calls g_arc_box_alloc() with `sizeof (@type)` and + * casts the returned pointer to a pointer of the given @type, + * avoiding a type cast in the source code. + * + * This macro cannot return %NULL, as the minimum allocation + * size from `sizeof (@type)` is 1 byte. + * + * Returns: (not nullable): a pointer to the allocated memory, + * cast to a pointer for the given @type + * + * Since: 2.58 + */ + +/** + * g_arc_box_new0: + * @type: the type to allocate, typically a structure name + * + * A convenience macro to allocate atomically reference counted + * data with the size of the given @type, and set its contents + * to 0. + * + * This macro calls g_arc_box_alloc0() with `sizeof (@type)` and + * casts the returned pointer to a pointer of the given @type, + * avoiding a type cast in the source code. + * + * This macro cannot return %NULL, as the minimum allocation + * size from `sizeof (@type)` is 1 byte. + * + * Returns: (not nullable): a pointer to the allocated memory, + * cast to a pointer for the given @type + * + * Since: 2.58 + */ + +/** + * g_arc_box_dup: + * @mem_block: (not nullable): a pointer to reference counted data + * + * Allocates a new block of data with atomic reference counting + * semantics, and copies the contents of @mem_block into + * it. + * + * Returns: (not nullable): a pointer to the allocated memory + * + * Since: 2.58 + */ +gpointer +(g_arc_box_dup) (gpointer mem_block) +{ + GArcBox *real_box = G_ARC_BOX (mem_block); + gpointer res; + + g_return_val_if_fail (mem_block != NULL, NULL); +#ifndef G_DISABLE_ASSERT + g_return_val_if_fail (real_box->magic == G_BOX_MAGIC, NULL); +#endif + + res = g_rc_box_alloc_full (real_box->mem_size, TRUE, FALSE); + memcpy (res, mem_block, real_box->mem_size); + + return res; +} + +/** + * g_arc_box_acquire: + * @mem_block: (not nullable): a pointer to reference counted data + * + * Atomically acquires a reference on the data pointed by @mem_block. + * + * Returns: (not nullabl): a pointer to the data, with its reference + * count increased + * + * Since: 2.58 + */ +gpointer +(g_arc_box_acquire) (gpointer mem_block) +{ + GArcBox *real_box = G_ARC_BOX (mem_block); + + g_return_val_if_fail (mem_block != NULL, NULL); +#ifndef G_DISABLE_ASSERT + g_return_val_if_fail (real_box->magic == G_BOX_MAGIC, NULL); +#endif + + g_atomic_ref_count_inc (&real_box->ref_count); + + return mem_block; +} + +/** + * g_arc_box_release: + * @mem_block: (not nullable): a pointer to reference counted data + * + * Atomically releases a reference on the data pointed by @mem_block. + * + * If the reference was the last one, it will free the + * resources allocated for @mem_block. + * + * Since: 2.58 + */ +void +g_arc_box_release (gpointer mem_block) +{ + GArcBox *real_box = G_ARC_BOX (mem_block); + + g_return_if_fail (mem_block != NULL); +#ifndef G_DISABLE_ASSERT + g_return_if_fail (real_box->magic == G_BOX_MAGIC); +#endif + + if (g_atomic_ref_count_dec (&real_box->ref_count)) + g_free (real_box); +} + +/** + * g_arc_box_release_full: + * @mem_block: (not nullable): a pointer to reference counted data + * @clear_func: (not nullable): a function to call when clearing the data + * + * Atomically releases a reference on the data pointed by @mem_block. + * + * If the reference was the last one, it will call @clear_func + * to clear the contents of @mem_block, and then will free the + * resources allocated for @mem_block. + * + * Since: 2.58 + */ +void +g_arc_box_release_full (gpointer mem_block, + GDestroyNotify clear_func) +{ + GArcBox *real_box = G_ARC_BOX (mem_block); + + g_return_if_fail (mem_block != NULL); + g_return_if_fail (clear_func != NULL); +#ifndef G_DISABLE_ASSERT + g_return_if_fail (real_box->magic == G_BOX_MAGIC); +#endif + + if (g_atomic_ref_count_dec (&real_box->ref_count)) + { + clear_func (mem_block); + g_free (real_box); + } +} diff --git a/glib/grcbox.c b/glib/grcbox.c index 0860634be..d41a6def4 100644 --- a/glib/grcbox.c +++ b/glib/grcbox.c @@ -1,4 +1,4 @@ -/* grcbox.h: Reference counted data +/* grcbox.c: Reference counted data * * Copyright 2018 Emmanuele Bassi * @@ -21,6 +21,7 @@ #include "grcbox.h" #include "gmessages.h" +#include "grcboxprivate.h" #include "grefcount.h" #ifdef ENABLE_VALGRIND @@ -129,35 +130,21 @@ * Since: 2.58. */ -typedef struct { - grefcount ref_count; - - gsize mem_size; - -#ifndef G_DISABLE_ASSERT - /* A "magic" number, used to perform additional integrity - * checks on the allocated data - */ - guint32 magic; -#endif -} GRcBox; - -#define G_RC_BOX_MAGIC 0x44ae2bf0 -#define G_RC_BOX_SIZE sizeof (GRcBox) #define G_RC_BOX(p) (GRcBox *) (((char *) (p)) - G_RC_BOX_SIZE) /* We use the same alignment as GTypeInstance and GNU libc's malloc */ #define STRUCT_ALIGNMENT (2 * sizeof (gsize)) #define ALIGN_STRUCT(offset) ((offset + (STRUCT_ALIGNMENT - 1)) & -STRUCT_ALIGNMENT) -static gpointer +gpointer g_rc_box_alloc_full (gsize block_size, + gboolean atomic, gboolean clear) { - gsize private_size = G_RC_BOX_SIZE; + /* sizeof GArcBox == sizeof GRcBox */ + gsize private_size = G_ARC_BOX_SIZE; gsize real_size = private_size + block_size; char *allocated; - GRcBox *real_box; #ifdef ENABLE_VALGRIND if (RUNNING_ON_VALGRIND) @@ -189,12 +176,24 @@ g_rc_box_alloc_full (gsize block_size, allocated = g_malloc (real_size); } - real_box = (GRcBox *) allocated; - real_box->mem_size = block_size; + if (atomic) + { + GArcBox *real_box = (GArcBox *) allocated; + real_box->mem_size = block_size; #ifndef G_DISABLE_ASSERT - real_box->magic = G_RC_BOX_MAGIC; + real_box->magic = G_BOX_MAGIC; #endif - g_ref_count_init (&real_box->ref_count); + g_atomic_ref_count_init (&real_box->ref_count); + } + else + { + GRcBox *real_box = (GRcBox *) allocated; + real_box->mem_size = block_size; +#ifndef G_DISABLE_ASSERT + real_box->magic = G_BOX_MAGIC; +#endif + g_ref_count_init (&real_box->ref_count); + } return allocated + private_size; } @@ -218,7 +217,7 @@ g_rc_box_alloc (gsize block_size) { g_return_val_if_fail (block_size > 0, NULL); - return g_rc_box_alloc_full (block_size, FALSE); + return g_rc_box_alloc_full (block_size, FALSE, FALSE); } /** @@ -242,7 +241,7 @@ g_rc_box_alloc0 (gsize block_size) { g_return_val_if_fail (block_size > 0, NULL); - return g_rc_box_alloc_full (block_size, TRUE); + return g_rc_box_alloc_full (block_size, FALSE, TRUE); } /** @@ -305,10 +304,10 @@ gpointer g_return_val_if_fail (mem_block != NULL, NULL); #ifndef G_DISABLE_ASSERT - g_return_val_if_fail (real_box->magic == G_RC_BOX_MAGIC, NULL); + g_return_val_if_fail (real_box->magic == G_BOX_MAGIC, NULL); #endif - res = g_rc_box_alloc_full (real_box->mem_size, FALSE); + res = g_rc_box_alloc_full (real_box->mem_size, FALSE, FALSE); memcpy (res, mem_block, real_box->mem_size); return res; @@ -332,7 +331,7 @@ gpointer g_return_val_if_fail (mem_block != NULL, NULL); #ifndef G_DISABLE_ASSERT - g_return_val_if_fail (real_box->magic == G_RC_BOX_MAGIC, NULL); + g_return_val_if_fail (real_box->magic == G_BOX_MAGIC, NULL); #endif g_ref_count_inc (&real_box->ref_count); @@ -358,7 +357,7 @@ g_rc_box_release (gpointer mem_block) g_return_if_fail (mem_block != NULL); #ifndef G_DISABLE_ASSERT - g_return_if_fail (real_box->magic == G_RC_BOX_MAGIC); + g_return_if_fail (real_box->magic == G_BOX_MAGIC); #endif if (g_ref_count_dec (&real_box->ref_count)) @@ -387,7 +386,7 @@ g_rc_box_release_full (gpointer mem_block, g_return_if_fail (mem_block != NULL); g_return_if_fail (clear_func != NULL); #ifndef G_DISABLE_ASSERT - g_return_if_fail (real_box->magic == G_RC_BOX_MAGIC); + g_return_if_fail (real_box->magic == G_BOX_MAGIC); #endif if (g_ref_count_dec (&real_box->ref_count)) diff --git a/glib/grcbox.h b/glib/grcbox.h index 8cd44037a..2b38815d8 100644 --- a/glib/grcbox.h +++ b/glib/grcbox.h @@ -40,16 +40,41 @@ GLIB_AVAILABLE_IN_2_58 void g_rc_box_release_full (gpointer mem_block, GDestroyNotify clear_func); +GLIB_AVAILABLE_IN_2_58 +gpointer g_arc_box_alloc (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); +GLIB_AVAILABLE_IN_2_58 +gpointer g_arc_box_alloc0 (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); +GLIB_AVAILABLE_IN_2_58 +gpointer g_arc_box_dup (gpointer mem_block) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); +GLIB_AVAILABLE_IN_2_58 +gpointer g_arc_box_acquire (gpointer mem_block); +GLIB_AVAILABLE_IN_2_58 +void g_arc_box_release (gpointer mem_block); +GLIB_AVAILABLE_IN_2_58 +void g_arc_box_release_full (gpointer mem_block, + GDestroyNotify clear_func); + #define g_rc_box_new(type) \ ((type *) g_rc_box_alloc (sizeof (type))) #define g_rc_box_new0(type) \ ((type *) g_rc_box_alloc0 (sizeof (type))) +#define g_arc_box_new(type) \ + ((type *) g_arc_box_alloc (sizeof (type))) +#define g_arc_box_new0(type) \ + ((type *) g_arc_box_alloc0 (sizeof (type))) #if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) && !defined(_cplusplus) /* Type check to avoid assigning references to different types */ -# define g_rc_box_acquire(mem_block) ((__typeof__(mem_block)) (g_rc_box_acquire) (mem_block)) +# define g_rc_box_acquire(mem_block) \ + ((__typeof__(mem_block)) (g_rc_box_acquire) (mem_block)) +# define g_arc_box_acquire(mem_block) \ + ((__typeof__(mem_block)) (g_arc_box_acquire) (mem_block)) + /* Type check to avoid duplicating data to different types */ -# define g_rc_box_dup(mem_block) ((__typeof__(mem_block)) (g_rc_box_dup) (mem_block)) +# define g_rc_box_dup(mem_block) \ + ((__typeof__(mem_block)) (g_rc_box_dup) (mem_block)) +# define g_arc_box_dup(mem_block) \ + ((__typeof__(mem_block)) (g_arc_box_dup) (mem_block)) #endif G_END_DECLS diff --git a/glib/grcboxprivate.h b/glib/grcboxprivate.h new file mode 100644 index 000000000..6599e4d4a --- /dev/null +++ b/glib/grcboxprivate.h @@ -0,0 +1,42 @@ +#pragma once + +#include "gtypes.h" + +G_BEGIN_DECLS + +typedef struct { + grefcount ref_count; + + gsize mem_size; + +#ifndef G_DISABLE_ASSERT + /* A "magic" number, used to perform additional integrity + * checks on the allocated data + */ + guint32 magic; +#endif +} GRcBox; + +typedef struct { + gatomicrefcount ref_count; + + gsize mem_size; + +#ifndef G_DISABLE_ASSERT + guint32 magic; +#endif +} GArcBox; + +#define G_BOX_MAGIC 0x44ae2bf0 + +/* Keep the two refcounted boxes identical in size */ +G_STATIC_ASSERT (sizeof (GRcBox) == sizeof (GArcBox)); + +#define G_RC_BOX_SIZE sizeof (GRcBox) +#define G_ARC_BOX_SIZE sizeof (GArcBox) + +gpointer g_rc_box_alloc_full (gsize block_size, + gboolean atomic, + gboolean clear); + +G_END_DECLS diff --git a/glib/meson.build b/glib/meson.build index 73ab4928f..1a3f44797 100644 --- a/glib/meson.build +++ b/glib/meson.build @@ -119,6 +119,7 @@ deprecated_sources = files( ) glib_sources = files( + 'garcbox.c', 'garray.c', 'gasyncqueue.c', 'gatomic.c', From 8990c3c4d3a875e3360f9208d655d917669f54cf Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Mon, 11 Jun 2018 17:14:12 +0100 Subject: [PATCH 03/19] Make g_rc_box_dup()/g_arc_box_dup() more generic It's more useful to have a dup() function that copies any blob of memory into a reference counted allocation, than to have a dup() that only copies a reference counted allocation. --- glib/garcbox.c | 21 ++++++------ glib/grcbox.c | 19 +++++------ glib/grcbox.h | 14 ++++---- glib/tests/rcbox.c | 82 ++++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 103 insertions(+), 33 deletions(-) diff --git a/glib/garcbox.c b/glib/garcbox.c index ea6e742d8..aaa2c8b56 100644 --- a/glib/garcbox.c +++ b/glib/garcbox.c @@ -222,29 +222,28 @@ g_arc_box_alloc0 (gsize block_size) /** * g_arc_box_dup: - * @mem_block: (not nullable): a pointer to reference counted data + * @block_size: the number of bytes to copy + * @mem_block: (not nullable): the memory to copy * - * Allocates a new block of data with atomic reference counting - * semantics, and copies the contents of @mem_block into - * it. + * Allocates a new block of data with atomit reference counting + * semantics, and copies @block_size bytes of @mem_block + * into it. * * Returns: (not nullable): a pointer to the allocated memory * * Since: 2.58 */ gpointer -(g_arc_box_dup) (gpointer mem_block) +(g_arc_box_dup) (gsize block_size, + gconstpointer mem_block) { - GArcBox *real_box = G_ARC_BOX (mem_block); gpointer res; + g_return_val_if_fail (block_size > 0, NULL); g_return_val_if_fail (mem_block != NULL, NULL); -#ifndef G_DISABLE_ASSERT - g_return_val_if_fail (real_box->magic == G_BOX_MAGIC, NULL); -#endif - res = g_rc_box_alloc_full (real_box->mem_size, TRUE, FALSE); - memcpy (res, mem_block, real_box->mem_size); + res = g_rc_box_alloc_full (block_size, TRUE, FALSE); + memcpy (res, mem_block, block_size); return res; } diff --git a/glib/grcbox.c b/glib/grcbox.c index d41a6def4..d65f4c919 100644 --- a/glib/grcbox.c +++ b/glib/grcbox.c @@ -286,29 +286,28 @@ g_rc_box_alloc0 (gsize block_size) /** * g_rc_box_dup: - * @mem_block: (not nullable): a pointer to reference counted data + * @block_size: the number of bytes to copy + * @mem_block: (not nullable): the memory to copy * * Allocates a new block of data with reference counting - * semantics, and copies the contents of @mem_block into - * it. + * semantics, and copies @block_size bytes of @mem_block + * into it. * * Returns: (not nullable): a pointer to the allocated memory * * Since: 2.58 */ gpointer -(g_rc_box_dup) (gpointer mem_block) +(g_rc_box_dup) (gsize block_size, + gconstpointer mem_block) { - GRcBox *real_box = G_RC_BOX (mem_block); gpointer res; + g_return_val_if_fail (block_size > 0, NULL); g_return_val_if_fail (mem_block != NULL, NULL); -#ifndef G_DISABLE_ASSERT - g_return_val_if_fail (real_box->magic == G_BOX_MAGIC, NULL); -#endif - res = g_rc_box_alloc_full (real_box->mem_size, FALSE, FALSE); - memcpy (res, mem_block, real_box->mem_size); + res = g_rc_box_alloc_full (block_size, FALSE, FALSE); + memcpy (res, mem_block, block_size); return res; } diff --git a/glib/grcbox.h b/glib/grcbox.h index 2b38815d8..3f364d330 100644 --- a/glib/grcbox.h +++ b/glib/grcbox.h @@ -31,7 +31,8 @@ gpointer g_rc_box_alloc (gsize block_size) G_GNUC_MALL GLIB_AVAILABLE_IN_2_58 gpointer g_rc_box_alloc0 (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); GLIB_AVAILABLE_IN_2_58 -gpointer g_rc_box_dup (gpointer mem_block) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); +gpointer g_rc_box_dup (gsize block_size, + gconstpointer mem_block) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); GLIB_AVAILABLE_IN_2_58 gpointer g_rc_box_acquire (gpointer mem_block); GLIB_AVAILABLE_IN_2_58 @@ -45,7 +46,8 @@ gpointer g_arc_box_alloc (gsize block_size) G_GNUC_MALL GLIB_AVAILABLE_IN_2_58 gpointer g_arc_box_alloc0 (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); GLIB_AVAILABLE_IN_2_58 -gpointer g_arc_box_dup (gpointer mem_block) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); +gpointer g_arc_box_dup (gsize block_size, + gconstpointer mem_block) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); GLIB_AVAILABLE_IN_2_58 gpointer g_arc_box_acquire (gpointer mem_block); GLIB_AVAILABLE_IN_2_58 @@ -71,10 +73,10 @@ void g_arc_box_release_full (gpointer mem_block, ((__typeof__(mem_block)) (g_arc_box_acquire) (mem_block)) /* Type check to avoid duplicating data to different types */ -# define g_rc_box_dup(mem_block) \ - ((__typeof__(mem_block)) (g_rc_box_dup) (mem_block)) -# define g_arc_box_dup(mem_block) \ - ((__typeof__(mem_block)) (g_arc_box_dup) (mem_block)) +# define g_rc_box_dup(block_size,mem_block) \ + ((__typeof__(mem_block)) (g_rc_box_dup) (block_size,mem_block)) +# define g_arc_box_dup(block_size,mem_block) \ + ((__typeof__(mem_block)) (g_arc_box_dup) (block_size,mem_block)) #endif G_END_DECLS diff --git a/glib/tests/rcbox.c b/glib/tests/rcbox.c index 19b3842e5..4a33876bc 100644 --- a/glib/tests/rcbox.c +++ b/glib/tests/rcbox.c @@ -1,3 +1,21 @@ +/* rcbox.c: Reference counted data + * + * Copyright 2018 Emmanuele Bassi + * + * 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.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 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * 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, see . + */ + #include typedef struct { @@ -6,6 +24,7 @@ typedef struct { static Point *global_point; +/* test_rcbox_new: Test g_rc_box_new() */ static void test_rcbox_new (void) { @@ -27,14 +46,18 @@ static void point_clear (Point *p) { g_assert_nonnull (p); + g_assert_true (global_point == p); g_assert_cmpfloat (p->x, ==, 42.0f); g_assert_cmpfloat (p->y, ==, 47.0f); - g_assert_true (global_point == p); + g_test_message ("global_point = %p", p); global_point = NULL; } +/* test_rcbox_release_full: Verify that g_rc_box_release_full() calls + * the clear function only when the last reference is released + */ static void test_rcbox_release_full (void) { @@ -56,16 +79,63 @@ test_rcbox_release_full (void) g_assert_null (global_point); } +static Point *global_point_a; +static Point *global_point_b; + +static void +point_clear_dup_a (Point *a) +{ + g_assert_true (a == global_point_a); + + g_test_message ("global_point_a = %p", a); + global_point_a = NULL; +} + +static void +point_clear_dup_b (Point *b) +{ + g_assert_true (b == global_point_b); + + g_test_message ("global_point_b = %p", b); + global_point_b = NULL; +} + +/* test_rcbox_dup: Verify that g_rc_box_dup() copies only the + * data and does not change the reference count of the original + */ static void test_rcbox_dup (void) { - Point *a = g_rc_box_new (Point); - Point *b = g_rc_box_dup (a); + Point *a, *b; + a = g_rc_box_new (Point); + a->x = 10.f; + a->y = 5.f; + + b = g_rc_box_dup (sizeof (Point), a); g_assert_true (a != b); + g_assert_cmpfloat (a->x, ==, b->x); + g_assert_cmpfloat (a->y, ==, b->y); - g_rc_box_release (a); - g_rc_box_release (b); + global_point_a = a; + global_point_b = b; + + a->x = 1.f; + a->y = 1.f; + g_assert_cmpfloat (a->x, !=, b->x); + g_assert_cmpfloat (a->y, !=, b->y); + + b->x = 5.f; + b->y = 10.f; + g_assert_cmpfloat (a->x, !=, b->x); + g_assert_cmpfloat (a->y, !=, b->y); + + g_rc_box_release_full (a, (GDestroyNotify) point_clear_dup_a); + g_assert_null (global_point_a); + g_assert_nonnull (global_point_b); + + g_rc_box_release_full (b, (GDestroyNotify) point_clear_dup_b); + g_assert_null (global_point_b); } int @@ -75,8 +145,8 @@ main (int argc, g_test_init (&argc, &argv, NULL); g_test_add_func ("/rcbox/new", test_rcbox_new); - g_test_add_func ("/rcbox/dup", test_rcbox_dup); g_test_add_func ("/rcbox/release-full", test_rcbox_release_full); + g_test_add_func ("/rcbox/dup", test_rcbox_dup); return g_test_run (); } From 4b33b03dd3f3030e4a28116a3e5f6b4315a67b39 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 12 Jun 2018 09:23:32 +0100 Subject: [PATCH 04/19] Improve the RcBox and ArcBox documentation Use better examples, split up into sections, and mention use with g_autoptr(). --- glib/garcbox.c | 108 +++++++++++++++++++++++++++------------------ glib/grcbox.c | 116 +++++++++++++++++++++++++++++++------------------ 2 files changed, 139 insertions(+), 85 deletions(-) diff --git a/glib/garcbox.c b/glib/garcbox.c index aaa2c8b56..01e0f1c56 100644 --- a/glib/garcbox.c +++ b/glib/garcbox.c @@ -44,52 +44,14 @@ * * ArcBox is useful if you have a plain old data type, like a structure * typically placed on the stack, and you wish to provide additional API - * to use it on the heap, without necessarily implementing copy/free - * semantics, or your own reference counting. + * to use it on the heap; or if you want to implement a new type to be + * passed around by reference without necessarily implementing copy/free + * semantics or your own reference counting. * * The typical use is: * * |[ * typedef struct { - * float x, y; - * } Point; - * - * Point * - * point_new (float x, float y) - * { - * Point *res = g_arc_box_new (Point); - * - * res->x = x; - * res->y = y; - * - * return res; - * } - * ]| - * - * Every time you wish to acquire a reference on the memory, you should - * call g_arc_box_acquire(); similarly, when you wish to release a reference - * you should call g_arc_box_release(): - * - * |[ - * Point * - * point_ref (Point *p) - * { - * return g_arc_box_acquire (p); - * } - * - * void - * point_unref (Point *p) - * { - * g_arc_box_release (p); - * } - * ]| - * - * If you have additional memory allocated inside the structure, you can - * use g_arc_box_release_full(), which takes a function pointer, which - * will be called if the reference released was the last: - * - * |[ - * typedef struct { * char *name; * char *address; * char *city; @@ -97,6 +59,41 @@ * int age; * } Person; * + * Person * + * person_new (void) + * { + * return g_arc_box_new0 (Person); + * } + * ]| + * + * Every time you wish to acquire a reference on the memory, you should + * call g_arc_box_acquire(); similarly, when you wish to release a reference + * you should call g_arc_box_release(): + * + * |[ + * // Add a Person to the Database; the Database acquires ownership + * // of the Person instance + * void + * add_person_to_database (Database *db, Person *p) + * { + * db->persons = g_list_prepend (db->persons, g_arc_box_acquire (p)); + * } + * + * // Removes a Person from the Database; the reference acquired by + * // add_person_to_database() is released here + * void + * remove_person_from_database (Database *db, Person *p) + * { + * db->persons = g_list_remove (db->persons, p); + * g_arc_box_release (p); + * } + * ]| + * + * If you have additional memory allocated inside the structure, you can + * use g_arc_box_release_full(), which takes a function pointer, which + * will be called if the reference released was the last: + * + * |[ * void * person_clear (Person *p) * { @@ -107,8 +104,9 @@ * } * * void - * person_unref (Person *p) + * remove_person_from_database (Database *db, Person *p) * { + * db->persons = g_list_remove (db->persons, p); * g_arc_box_release_full (p, (GDestroyNotify) person_clear); * } * ]| @@ -124,12 +122,38 @@ * add_person_to_database (db, g_steal_pointer (&p)); * ]| * + * ## Thread safety + * * The reference counting operations on data allocated using g_arc_box_alloc(), * g_arc_box_new(), and g_arc_box_dup() are guaranteed to be atomic, and thus * can be safely be performed by different threads. It is important to note that * only the reference acquisition and release are atomic; changes to the content * of the data are your responsibility. * + * ## Automatic pointer clean up + * + * If you want to add g_autoptr() support to your plain old data type through + * reference counting, you can use the G_DEFINE_AUTOPTR_CLEANUP_FUNC() and + * g_arc_box_release(): + * + * |[ + * G_DEFINE_AUTOPTR_CLEANUP_FUNC (MyDataStruct, g_arc_box_release) + * ]| + * + * If you need to clear the contents of the data, you will need to use an + * ancillary function that calls g_rc_box_release_full(): + * + * |[ + * static void + * my_data_struct_release (MyDataStruct *data) + * { + * // my_data_struct_clear() is defined elsewhere + * g_arc_box_release_full (data, (GDestroyNotify) my_data_struct_clear); + * } + * + * G_DEFINE_AUTOPTR_CLEANUP_FUNC (MyDataStruct, my_data_struct_clear) + * ]| + * * Since: 2.58. */ diff --git a/glib/grcbox.c b/glib/grcbox.c index d65f4c919..d09c3b7fa 100644 --- a/glib/grcbox.c +++ b/glib/grcbox.c @@ -42,52 +42,14 @@ * * RcBox is useful if you have a plain old data type, like a structure * typically placed on the stack, and you wish to provide additional API - * to use it on the heap, without necessarily implementing copy/free - * semantics, or your own reference counting. + * to use it on the heap; or if you want to implement a new type to be + * passed around by reference without necessarily implementing copy/free + * semantics or your own reference counting. * * The typical use is: * * |[ * typedef struct { - * float x, y; - * } Point; - * - * Point * - * point_new (float x, float y) - * { - * Point *res = g_rc_box_new (Point); - * - * res->x = x; - * res->y = y; - * - * return res; - * } - * ]| - * - * Every time you wish to acquire a reference on the memory, you should - * call g_rc_box_acquire(); similarly, when you wish to release a reference - * you should call g_rc_box_release(): - * - * |[ - * Point * - * point_ref (Point *p) - * { - * return g_rc_box_acquire (p); - * } - * - * void - * point_unref (Point *p) - * { - * g_rc_box_release (p); - * } - * ]| - * - * If you have additional memory allocated inside the structure, you can - * use g_rc_box_release_full(), which takes a function pointer, which - * will be called if the reference released was the last: - * - * |[ - * typedef struct { * char *name; * char *address; * char *city; @@ -95,6 +57,41 @@ * int age; * } Person; * + * Person * + * person_new (void) + * { + * return g_rc_box_new0 (Person); + * } + * ]| + * + * Every time you wish to acquire a reference on the memory, you should + * call g_rc_box_acquire(); similarly, when you wish to release a reference + * you should call g_rc_box_release(): + * + * |[ + * // Add a Person to the Database; the Database acquires ownership + * // of the Person instance + * void + * add_person_to_database (Database *db, Person *p) + * { + * db->persons = g_list_prepend (db->persons, g_rc_box_acquire (p)); + * } + * + * // Removes a Person from the Database; the reference acquired by + * // add_person_to_database() is released here + * void + * remove_person_from_database (Database *db, Person *p) + * { + * db->persons = g_list_remove (db->persons, p); + * g_rc_box_release (p); + * } + * ]| + * + * If you have additional memory allocated inside the structure, you can + * use g_rc_box_release_full(), which takes a function pointer, which + * will be called if the reference released was the last: + * + * |[ * void * person_clear (Person *p) * { @@ -105,8 +102,9 @@ * } * * void - * person_unref (Person *p) + * remove_person_from_database (Database *db, Person *p) * { + * db->persons = g_list_remove (db->persons, p); * g_rc_box_release_full (p, (GDestroyNotify) person_clear); * } * ]| @@ -117,16 +115,48 @@ * |[ * Person *p = g_rc_box_new (Person); * + * // fill_person_details() is defined elsewhere * fill_person_details (p); * - * add_person_to_database (db, g_steal_pointer (&p)); + * // add_person_to_database_no_ref() is defined elsewhere; it adds + * // a Person to the Database without taking a reference + * add_person_to_database_no_ref (db, g_steal_pointer (&p)); * ]| * + * ## Thread safety + * * The reference counting operations on data allocated using g_rc_box_alloc(), * g_rc_box_new(), and g_rc_box_dup() are not thread safe; it is your code's * responsibility to ensure that references are acquired are released on the * same thread. * + * If you need thread safe reference counting, see the [atomic reference counted + * data][arcbox] API. + * + * ## Automatic pointer clean up + * + * If you want to add g_autoptr() support to your plain old data type through + * reference counting, you can use the G_DEFINE_AUTOPTR_CLEANUP_FUNC() and + * g_rc_box_release(): + * + * |[ + * G_DEFINE_AUTOPTR_CLEANUP_FUNC (MyDataStruct, g_rc_box_release) + * ]| + * + * If you need to clear the contents of the data, you will need to use an + * ancillary function that calls g_rc_box_release_full(): + * + * |[ + * static void + * my_data_struct_release (MyDataStruct *data) + * { + * // my_data_struct_clear() is defined elsewhere + * g_rc_box_release_full (data, (GDestroyNotify) my_data_struct_clear); + * } + * + * G_DEFINE_AUTOPTR_CLEANUP_FUNC (MyDataStruct, my_data_struct_clear) + * ]| + * * Since: 2.58. */ From 00a723f597da47182040dcf1a60df09ef350a0d2 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 12 Jun 2018 13:46:28 +0100 Subject: [PATCH 05/19] Add reference counted strings The last part of the reference counting saga. Now that we have: - reference counter types - reference counted allocations we can finally add reference counted strings using reference counted allocations to avoid creating a new String type, and reimplementing every single string-based API. --- docs/reference/glib/glib-docs.xml | 1 + docs/reference/glib/glib-sections.txt | 8 ++ glib/Makefile.am | 2 + glib/glib-autocleanups.h | 1 + glib/glib.h | 1 + glib/grefstring.c | 175 ++++++++++++++++++++++++++ glib/grefstring.h | 38 ++++++ glib/meson.build | 2 + glib/tests/Makefile.am | 1 + glib/tests/autoptr.c | 8 ++ glib/tests/meson.build | 1 + glib/tests/refstring.c | 70 +++++++++++ 12 files changed, 308 insertions(+) create mode 100644 glib/grefstring.c create mode 100644 glib/grefstring.h create mode 100644 glib/tests/refstring.c diff --git a/docs/reference/glib/glib-docs.xml b/docs/reference/glib/glib-docs.xml index 9d253ee5f..e0e8ba233 100644 --- a/docs/reference/glib/glib-docs.xml +++ b/docs/reference/glib/glib-docs.xml @@ -122,6 +122,7 @@ + diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt index 2a4d1f281..dd851bc9e 100644 --- a/docs/reference/glib/glib-sections.txt +++ b/docs/reference/glib/glib-sections.txt @@ -3490,3 +3490,11 @@ g_arc_box_acquire g_arc_box_release g_arc_box_release_full + +
+refstring +g_ref_string_new +g_ref_string_new_intern +g_ref_string_acquire +g_ref_string_release +
diff --git a/glib/Makefile.am b/glib/Makefile.am index 352ecb0ec..c7adc1f0e 100644 --- a/glib/Makefile.am +++ b/glib/Makefile.am @@ -153,6 +153,7 @@ libglib_2_0_la_SOURCES = \ grcbox.c \ grcboxprivate.h \ grefcount.c \ + grefstring.c \ gregex.c \ gscanner.c \ gscripttable.h \ @@ -291,6 +292,7 @@ glibsubinclude_HEADERS = \ grand.h \ grcbox.h \ grefcount.h \ + grefstring.h \ gregex.h \ gscanner.h \ gsequence.h \ diff --git a/glib/glib-autocleanups.h b/glib/glib-autocleanups.h index 9f86bd99c..ce1690b78 100644 --- a/glib/glib-autocleanups.h +++ b/glib/glib-autocleanups.h @@ -87,3 +87,4 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(GVariantDict, g_variant_dict_unref) G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(GVariantDict, g_variant_dict_clear) G_DEFINE_AUTOPTR_CLEANUP_FUNC(GVariantType, g_variant_type_free) G_DEFINE_AUTO_CLEANUP_FREE_FUNC(GStrv, g_strfreev, NULL) +G_DEFINE_AUTOPTR_CLEANUP_FUNC (GRefString, g_ref_string_release) diff --git a/glib/glib.h b/glib/glib.h index 309366281..94a11fb62 100644 --- a/glib/glib.h +++ b/glib/glib.h @@ -71,6 +71,7 @@ #include #include #include +#include #include #include #include diff --git a/glib/grefstring.c b/glib/grefstring.c new file mode 100644 index 000000000..cc62c8011 --- /dev/null +++ b/glib/grefstring.c @@ -0,0 +1,175 @@ +/* grefstring.c: Reference counted strings + * + * Copyright 2018 Emmanuele Bassi + * + * 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.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 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * 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, see . + */ + +/** + * SECTION:refstring + * @Title: Reference counted strings + * @Short_description: Strings with reference counted memory management + * + * Reference counted strings are normal C strings that have been augmented + * with a reference counter to manage their resources. You allocate a new + * reference counted string and acquire and release references as needed, + * instead of copying the string among callers; when the last reference on + * the string is released, the resources allocated for it are freed. + * + * Since: 2.58 + */ + +#include "config.h" + +#include "grefstring.h" + +#include "ghash.h" +#include "gmessages.h" +#include "grcbox.h" +#include "gthread.h" + +#include + +G_LOCK_DEFINE_STATIC (interned_ref_strings); +static GHashTable *interned_ref_strings; + +/** + * g_ref_string_new: + * @str: (not nullable): a NUL-terminated string + * + * Creates a new reference counted string and copies the contents of @str + * into it. + * + * Returns: (not nullable): the newly created reference counted string + * + * Since: 2.58 + */ +char * +g_ref_string_new (const char *str) +{ + char *res; + gsize len; + + g_return_val_if_fail (str != NULL && *str != '\0', NULL); + + len = strlen (str); + + res = (char *) g_arc_box_dup (sizeof (char) * len + 1, str); + res[len] = '\0'; + + return res; +} + +/** + * g_ref_string_new_intern: + * @str: (not nullable): a NUL-terminated string + * + * Creates a new reference counted string and copies the content of @str + * into it. + * + * If you call this function multiple times with the same @str, or with + * the same contents of @str, it will return a new reference, instead of + * creating a new string. + * + * Returns: (not nullable): the newly created reference counted string, or + * a new reference to an existing string + * + * Since: 2.58 + */ +char * +g_ref_string_new_intern (const char *str) +{ + gsize len; + char *res; + + g_return_val_if_fail (str != NULL && *str != '\0', NULL); + + G_LOCK (interned_ref_strings); + + if (G_UNLIKELY (interned_ref_strings == NULL)) + interned_ref_strings = g_hash_table_new_full (g_str_hash, g_str_equal, + (GDestroyNotify) g_arc_box_release, + NULL); + + res = g_hash_table_lookup (interned_ref_strings, str); + if (res != NULL) + { + G_UNLOCK (interned_ref_strings); + return g_arc_box_acquire (res); + } + + len = strlen (str); + res = (char *) g_arc_box_dup (sizeof (char) * len + 1, str); + res[len] = '\0'; + + g_hash_table_add (interned_ref_strings, g_arc_box_acquire (res)); + + G_UNLOCK (interned_ref_strings); + + return res; +} + +/** + * g_ref_string_acquire: + * @str: a reference counted string + * + * Acquires a reference on a string. + * + * Returns: the given string, with its reference count increased + * + * Since: 2.58 + */ +char * +g_ref_string_acquire (char *str) +{ + g_return_val_if_fail (str != NULL && *str != '\0', NULL); + + return g_arc_box_acquire (str); +} + +static void +remove_if_interned (gpointer data) +{ + char *str = data; + + G_LOCK (interned_ref_strings); + + if (G_LIKELY (interned_ref_strings != NULL)) + { + if (g_hash_table_contains (interned_ref_strings, str)) + g_hash_table_remove (interned_ref_strings, str); + + if (g_hash_table_size (interned_ref_strings) == 0) + g_clear_pointer (&interned_ref_strings, g_hash_table_destroy); + } + + G_UNLOCK (interned_ref_strings); +} + +/** + * g_ref_string_release: + * @str: a reference counted string + * + * Releases a reference on a string; if it was the last reference, the + * resources allocated by the string are freed as well. + * + * Since: 2.58 + */ +void +g_ref_string_release (char *str) +{ + g_return_if_fail (str != NULL && *str != '\0'); + + g_arc_box_release_full (str, remove_if_interned); +} diff --git a/glib/grefstring.h b/glib/grefstring.h new file mode 100644 index 000000000..f2156fef4 --- /dev/null +++ b/glib/grefstring.h @@ -0,0 +1,38 @@ +/* grefstring.h: Reference counted strings + * + * Copyright 2018 Emmanuele Bassi + * + * 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.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 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * 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, see . + */ + +#pragma once + +#include "gmem.h" +#include "gmacros.h" + +G_BEGIN_DECLS + +GLIB_AVAILABLE_IN_2_58 +char * g_ref_string_new (const char *str); +GLIB_AVAILABLE_IN_2_58 +char * g_ref_string_new_intern (const char *str); + +GLIB_AVAILABLE_IN_2_58 +char * g_ref_string_acquire (char *str); +GLIB_AVAILABLE_IN_2_58 +void g_ref_string_release (char *str); + +typedef char GRefString; + +G_END_DECLS diff --git a/glib/meson.build b/glib/meson.build index 1a3f44797..c05c69406 100644 --- a/glib/meson.build +++ b/glib/meson.build @@ -78,6 +78,7 @@ glib_sub_headers = files( 'grand.h', 'grcbox.h', 'grefcount.h', + 'grefstring.h', 'gregex.h', 'gscanner.h', 'gsequence.h', @@ -164,6 +165,7 @@ glib_sources = files( 'grand.c', 'grcbox.c', 'grefcount.c', + 'grefstring.c', 'gregex.c', 'gscanner.c', 'gsequence.c', diff --git a/glib/tests/Makefile.am b/glib/tests/Makefile.am index 0d524d57f..71ac15517 100644 --- a/glib/tests/Makefile.am +++ b/glib/tests/Makefile.am @@ -91,6 +91,7 @@ test_programs = \ rand \ rcbox \ rec-mutex \ + refstring \ regex \ rwlock \ scannerapi \ diff --git a/glib/tests/autoptr.c b/glib/tests/autoptr.c index c8c130d5c..5b3bce71c 100644 --- a/glib/tests/autoptr.c +++ b/glib/tests/autoptr.c @@ -422,6 +422,13 @@ test_strv (void) g_assert (val != NULL); } +static void +test_refstring (void) +{ + g_autoptr(GRefString) str = g_ref_string_new ("hello, world"); + g_assert_nonnull (str); +} + static void mark_freed (gpointer ptr) { @@ -539,6 +546,7 @@ main (int argc, gchar *argv[]) g_test_add_func ("/autoptr/g_variant_dict", test_g_variant_dict); g_test_add_func ("/autoptr/g_variant_type", test_g_variant_type); g_test_add_func ("/autoptr/strv", test_strv); + g_test_add_func ("/autoptr/refstring", test_refstring); g_test_add_func ("/autoptr/autolist", test_autolist); g_test_add_func ("/autoptr/autoslist", test_autoslist); diff --git a/glib/tests/meson.build b/glib/tests/meson.build index 3c20bad39..7c49d579c 100644 --- a/glib/tests/meson.build +++ b/glib/tests/meson.build @@ -50,6 +50,7 @@ glib_tests = [ 'rec-mutex', 'refcount', 'refcount-macro', + 'refstring', 'regex', 'rwlock', 'scannerapi', diff --git a/glib/tests/refstring.c b/glib/tests/refstring.c new file mode 100644 index 000000000..0181be945 --- /dev/null +++ b/glib/tests/refstring.c @@ -0,0 +1,70 @@ +/* refstring.c: Reference counted strings + * + * Copyright 2018 Emmanuele Bassi + * + * 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.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 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * 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, see . + */ + +#include +#include + +/* test_refstring_base: Test the base API of GRefString */ +static void +test_refstring_base (void) +{ + char *s = g_ref_string_new ("hello, world"); + + g_test_message ("s = '%s' (%p)", s, s); + g_assert_cmpint (strcmp (s, "hello, world"), ==, 0); + g_assert_cmpint (strlen (s), ==, strlen ("hello, world")); + + g_ref_string_release (s); +} + +/* test_refstring_intern: Test the interning API of GRefString */ +static void +test_refstring_intern (void) +{ + char *s = g_ref_string_new_intern ("hello, world"); + char *p; + + g_test_message ("s = '%s' (%p)", s, s); + g_assert_cmpstr (s, ==, "hello, world"); + + p = g_ref_string_new_intern ("hello, world"); + g_test_message ("p = s = '%s' (%p)", p, p); + g_assert_true (s == p); + + g_ref_string_release (p); + + p = g_ref_string_new_intern ("goodbye, world"); + g_test_message ("p = '%s' (%p)", p, p); + g_assert_false (s == p); + + g_ref_string_release (p); + g_ref_string_release (s); +} + +int +main (int argc, + char *argv[]) +{ + g_test_init (&argc, &argv, NULL); + + g_test_add_func ("/refstring/base", test_refstring_base); + g_test_add_func ("/refstring/intern", test_refstring_intern); + + return g_test_run (); +} + From 43b7a8f158343f1334a539d85a71dfb8a5e0328a Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 12 Jun 2018 13:56:29 +0100 Subject: [PATCH 06/19] Add size accessor to RcBox and ArcBox It may be useful to know how big a reference counted allocation is outside of internal checks. --- docs/reference/glib/glib-sections.txt | 2 ++ glib/garcbox.c | 23 +++++++++++++++++++++++ glib/grcbox.c | 23 +++++++++++++++++++++++ glib/grcbox.h | 6 ++++++ glib/tests/rcbox.c | 1 + 5 files changed, 55 insertions(+) diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt index dd851bc9e..b73ba1cc0 100644 --- a/docs/reference/glib/glib-sections.txt +++ b/docs/reference/glib/glib-sections.txt @@ -3477,6 +3477,7 @@ g_rc_box_dup g_rc_box_acquire g_rc_box_release g_rc_box_release_full +g_rc_box_get_size
@@ -3489,6 +3490,7 @@ g_arc_box_dup g_arc_box_acquire g_arc_box_release g_arc_box_release_full +g_arc_box_get_size
diff --git a/glib/garcbox.c b/glib/garcbox.c index 01e0f1c56..b1581134c 100644 --- a/glib/garcbox.c +++ b/glib/garcbox.c @@ -354,3 +354,26 @@ g_arc_box_release_full (gpointer mem_block, g_free (real_box); } } + +/** + * g_arc_box_get_size: + * @mem_block: (not nullable): a pointer to reference counted data + * + * Retrieves the size of the reference counted data pointed by @mem_block. + * + * Returns: the size of the data + * + * Since: 2.58 + */ +gsize +g_arc_box_get_size (gpointer mem_block) +{ + GArcBox *real_box = G_ARC_BOX (mem_block); + + g_return_val_if_fail (mem_block != NULL, 0); +#ifndef G_DISABLE_ASSERT + g_return_val_if_fail (real_box->magic == G_BOX_MAGIC, 0); +#endif + + return real_box->mem_size; +} diff --git a/glib/grcbox.c b/glib/grcbox.c index d09c3b7fa..5cceb87b7 100644 --- a/glib/grcbox.c +++ b/glib/grcbox.c @@ -424,3 +424,26 @@ g_rc_box_release_full (gpointer mem_block, g_free (real_box); } } + +/** + * g_rc_box_get_size: + * @mem_block: (not nullable): a pointer to reference counted data + * + * Retrieves the size of the reference counted data pointed by @mem_block. + * + * Returns: the size of the data + * + * Since: 2.58 + */ +gsize +g_rc_box_get_size (gpointer mem_block) +{ + GRcBox *real_box = G_RC_BOX (mem_block); + + g_return_val_if_fail (mem_block != NULL, 0); +#ifndef G_DISABLE_ASSERT + g_return_val_if_fail (real_box->magic == G_BOX_MAGIC, 0); +#endif + + return real_box->mem_size; +} diff --git a/glib/grcbox.h b/glib/grcbox.h index 3f364d330..a8ecce356 100644 --- a/glib/grcbox.h +++ b/glib/grcbox.h @@ -41,6 +41,9 @@ GLIB_AVAILABLE_IN_2_58 void g_rc_box_release_full (gpointer mem_block, GDestroyNotify clear_func); +GLIB_AVAILABLE_IN_2_58 +gsize g_rc_box_get_size (gpointer mem_block); + GLIB_AVAILABLE_IN_2_58 gpointer g_arc_box_alloc (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); GLIB_AVAILABLE_IN_2_58 @@ -56,6 +59,9 @@ GLIB_AVAILABLE_IN_2_58 void g_arc_box_release_full (gpointer mem_block, GDestroyNotify clear_func); +GLIB_AVAILABLE_IN_2_58 +gsize g_arc_box_get_size (gpointer mem_block); + #define g_rc_box_new(type) \ ((type *) g_rc_box_alloc (sizeof (type))) #define g_rc_box_new0(type) \ diff --git a/glib/tests/rcbox.c b/glib/tests/rcbox.c index 4a33876bc..48271a5c8 100644 --- a/glib/tests/rcbox.c +++ b/glib/tests/rcbox.c @@ -31,6 +31,7 @@ test_rcbox_new (void) Point *a = g_rc_box_new (Point); g_assert_nonnull (a); + g_assert_cmpuint (g_rc_box_get_size (a), ==, sizeof (Point)); g_rc_box_release (a); From 32ecb86f5bb2290d3481187514305c173f33fe6e Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 12 Jun 2018 14:05:02 +0100 Subject: [PATCH 07/19] Add length accessor for GRefString Since we store the size of the allocation in the underlying ArcBox, we can get a constant time getter for the length of the string. --- docs/reference/glib/glib-sections.txt | 1 + glib/grefstring.c | 18 ++++++++++++++++++ glib/grefstring.h | 3 +++ glib/tests/refstring.c | 1 + 4 files changed, 23 insertions(+) diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt index b73ba1cc0..1717ba37d 100644 --- a/docs/reference/glib/glib-sections.txt +++ b/docs/reference/glib/glib-sections.txt @@ -3499,4 +3499,5 @@ g_ref_string_new g_ref_string_new_intern g_ref_string_acquire g_ref_string_release +g_ref_string_length
diff --git a/glib/grefstring.c b/glib/grefstring.c index cc62c8011..9d64a968b 100644 --- a/glib/grefstring.c +++ b/glib/grefstring.c @@ -173,3 +173,21 @@ g_ref_string_release (char *str) g_arc_box_release_full (str, remove_if_interned); } + +/** + * g_ref_string_length: + * @str: a reference counted string + * + * Retrieves the length of @str. + * + * Returns: the length of the given string, in bytes + * + * Since: 2.58 + */ +gsize +g_ref_string_length (char *str) +{ + g_return_val_if_fail (str != NULL && *str != '\0', 0); + + return g_arc_box_get_size (str) - 1; +} diff --git a/glib/grefstring.h b/glib/grefstring.h index f2156fef4..2afe23a73 100644 --- a/glib/grefstring.h +++ b/glib/grefstring.h @@ -33,6 +33,9 @@ char * g_ref_string_acquire (char *str); GLIB_AVAILABLE_IN_2_58 void g_ref_string_release (char *str); +GLIB_AVAILABLE_IN_2_58 +gsize g_ref_string_length (char *str); + typedef char GRefString; G_END_DECLS diff --git a/glib/tests/refstring.c b/glib/tests/refstring.c index 0181be945..67b3ca302 100644 --- a/glib/tests/refstring.c +++ b/glib/tests/refstring.c @@ -28,6 +28,7 @@ test_refstring_base (void) g_test_message ("s = '%s' (%p)", s, s); g_assert_cmpint (strcmp (s, "hello, world"), ==, 0); g_assert_cmpint (strlen (s), ==, strlen ("hello, world")); + g_assert_cmpuint (g_ref_string_length (s), ==, strlen ("hello, world")); g_ref_string_release (s); } From 3bc0499eb24231ecfae221785e0232559f5269a3 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 3 Jul 2018 15:23:34 +0100 Subject: [PATCH 08/19] Rename g_arc_box to g_atomic_rc_box Makes the API more self-explanatory. --- glib/garcbox.c | 66 +++++++++++++++++++++++------------------------ glib/grcbox.h | 52 ++++++++++++++++++------------------- glib/grefstring.c | 16 ++++++------ 3 files changed, 67 insertions(+), 67 deletions(-) diff --git a/glib/garcbox.c b/glib/garcbox.c index b1581134c..bbf16db1b 100644 --- a/glib/garcbox.c +++ b/glib/garcbox.c @@ -62,13 +62,13 @@ * Person * * person_new (void) * { - * return g_arc_box_new0 (Person); + * return g_atomic_rc_box_new0 (Person); * } * ]| * * Every time you wish to acquire a reference on the memory, you should - * call g_arc_box_acquire(); similarly, when you wish to release a reference - * you should call g_arc_box_release(): + * call g_atomic_rc_box_acquire(); similarly, when you wish to release a reference + * you should call g_atomic_rc_box_release(): * * |[ * // Add a Person to the Database; the Database acquires ownership @@ -76,7 +76,7 @@ * void * add_person_to_database (Database *db, Person *p) * { - * db->persons = g_list_prepend (db->persons, g_arc_box_acquire (p)); + * db->persons = g_list_prepend (db->persons, g_atomic_rc_box_acquire (p)); * } * * // Removes a Person from the Database; the reference acquired by @@ -85,12 +85,12 @@ * remove_person_from_database (Database *db, Person *p) * { * db->persons = g_list_remove (db->persons, p); - * g_arc_box_release (p); + * g_atomic_rc_box_release (p); * } * ]| * * If you have additional memory allocated inside the structure, you can - * use g_arc_box_release_full(), which takes a function pointer, which + * use g_atomic_rc_box_release_full(), which takes a function pointer, which * will be called if the reference released was the last: * * |[ @@ -107,7 +107,7 @@ * remove_person_from_database (Database *db, Person *p) * { * db->persons = g_list_remove (db->persons, p); - * g_arc_box_release_full (p, (GDestroyNotify) person_clear); + * g_atomic_rc_box_release_full (p, (GDestroyNotify) person_clear); * } * ]| * @@ -115,7 +115,7 @@ * type without increasing the reference count, you can use g_steal_pointer(): * * |[ - * Person *p = g_arc_box_new (Person); + * Person *p = g_atomic_rc_box_new (Person); * * fill_person_details (p); * @@ -124,8 +124,8 @@ * * ## Thread safety * - * The reference counting operations on data allocated using g_arc_box_alloc(), - * g_arc_box_new(), and g_arc_box_dup() are guaranteed to be atomic, and thus + * The reference counting operations on data allocated using g_atomic_rc_box_alloc(), + * g_atomic_rc_box_new(), and g_atomic_rc_box_dup() are guaranteed to be atomic, and thus * can be safely be performed by different threads. It is important to note that * only the reference acquisition and release are atomic; changes to the content * of the data are your responsibility. @@ -134,10 +134,10 @@ * * If you want to add g_autoptr() support to your plain old data type through * reference counting, you can use the G_DEFINE_AUTOPTR_CLEANUP_FUNC() and - * g_arc_box_release(): + * g_atomic_rc_box_release(): * * |[ - * G_DEFINE_AUTOPTR_CLEANUP_FUNC (MyDataStruct, g_arc_box_release) + * G_DEFINE_AUTOPTR_CLEANUP_FUNC (MyDataStruct, g_atomic_rc_box_release) * ]| * * If you need to clear the contents of the data, you will need to use an @@ -148,7 +148,7 @@ * my_data_struct_release (MyDataStruct *data) * { * // my_data_struct_clear() is defined elsewhere - * g_arc_box_release_full (data, (GDestroyNotify) my_data_struct_clear); + * g_atomic_rc_box_release_full (data, (GDestroyNotify) my_data_struct_clear); * } * * G_DEFINE_AUTOPTR_CLEANUP_FUNC (MyDataStruct, my_data_struct_clear) @@ -158,7 +158,7 @@ */ /** - * g_arc_box_alloc: + * g_atomic_rc_box_alloc: * @block_size: the size of the allocation * * Allocates @block_size bytes of memory, and adds atomic @@ -172,7 +172,7 @@ * Since: 2.58 */ gpointer -g_arc_box_alloc (gsize block_size) +g_atomic_rc_box_alloc (gsize block_size) { g_return_val_if_fail (block_size > 0, NULL); @@ -180,7 +180,7 @@ g_arc_box_alloc (gsize block_size) } /** - * g_arc_box_alloc0: + * g_atomic_rc_box_alloc0: * @block_size: the size of the allocation * * Allocates @block_size bytes of memory, and adds atomic @@ -196,7 +196,7 @@ g_arc_box_alloc (gsize block_size) * Since: 2.58 */ gpointer -g_arc_box_alloc0 (gsize block_size) +g_atomic_rc_box_alloc0 (gsize block_size) { g_return_val_if_fail (block_size > 0, NULL); @@ -204,13 +204,13 @@ g_arc_box_alloc0 (gsize block_size) } /** - * g_arc_box_new: + * g_atomic_rc_box_new: * @type: the type to allocate, typically a structure name * * A convenience macro to allocate atomically reference counted * data with the size of the given @type. * - * This macro calls g_arc_box_alloc() with `sizeof (@type)` and + * This macro calls g_atomic_rc_box_alloc() with `sizeof (@type)` and * casts the returned pointer to a pointer of the given @type, * avoiding a type cast in the source code. * @@ -224,14 +224,14 @@ g_arc_box_alloc0 (gsize block_size) */ /** - * g_arc_box_new0: + * g_atomic_rc_box_new0: * @type: the type to allocate, typically a structure name * * A convenience macro to allocate atomically reference counted * data with the size of the given @type, and set its contents * to 0. * - * This macro calls g_arc_box_alloc0() with `sizeof (@type)` and + * This macro calls g_atomic_rc_box_alloc0() with `sizeof (@type)` and * casts the returned pointer to a pointer of the given @type, * avoiding a type cast in the source code. * @@ -245,7 +245,7 @@ g_arc_box_alloc0 (gsize block_size) */ /** - * g_arc_box_dup: + * g_atomic_rc_box_dup: * @block_size: the number of bytes to copy * @mem_block: (not nullable): the memory to copy * @@ -258,8 +258,8 @@ g_arc_box_alloc0 (gsize block_size) * Since: 2.58 */ gpointer -(g_arc_box_dup) (gsize block_size, - gconstpointer mem_block) +(g_atomic_rc_box_dup) (gsize block_size, + gconstpointer mem_block) { gpointer res; @@ -273,7 +273,7 @@ gpointer } /** - * g_arc_box_acquire: + * g_atomic_rc_box_acquire: * @mem_block: (not nullable): a pointer to reference counted data * * Atomically acquires a reference on the data pointed by @mem_block. @@ -284,7 +284,7 @@ gpointer * Since: 2.58 */ gpointer -(g_arc_box_acquire) (gpointer mem_block) +(g_atomic_rc_box_acquire) (gpointer mem_block) { GArcBox *real_box = G_ARC_BOX (mem_block); @@ -299,7 +299,7 @@ gpointer } /** - * g_arc_box_release: + * g_atomic_rc_box_release: * @mem_block: (not nullable): a pointer to reference counted data * * Atomically releases a reference on the data pointed by @mem_block. @@ -310,7 +310,7 @@ gpointer * Since: 2.58 */ void -g_arc_box_release (gpointer mem_block) +g_atomic_rc_box_release (gpointer mem_block) { GArcBox *real_box = G_ARC_BOX (mem_block); @@ -324,7 +324,7 @@ g_arc_box_release (gpointer mem_block) } /** - * g_arc_box_release_full: + * g_atomic_rc_box_release_full: * @mem_block: (not nullable): a pointer to reference counted data * @clear_func: (not nullable): a function to call when clearing the data * @@ -337,8 +337,8 @@ g_arc_box_release (gpointer mem_block) * Since: 2.58 */ void -g_arc_box_release_full (gpointer mem_block, - GDestroyNotify clear_func) +g_atomic_rc_box_release_full (gpointer mem_block, + GDestroyNotify clear_func) { GArcBox *real_box = G_ARC_BOX (mem_block); @@ -356,7 +356,7 @@ g_arc_box_release_full (gpointer mem_block, } /** - * g_arc_box_get_size: + * g_atomic_rc_box_get_size: * @mem_block: (not nullable): a pointer to reference counted data * * Retrieves the size of the reference counted data pointed by @mem_block. @@ -366,7 +366,7 @@ g_arc_box_release_full (gpointer mem_block, * Since: 2.58 */ gsize -g_arc_box_get_size (gpointer mem_block) +g_atomic_rc_box_get_size (gpointer mem_block) { GArcBox *real_box = G_ARC_BOX (mem_block); diff --git a/glib/grcbox.h b/glib/grcbox.h index a8ecce356..23b417f09 100644 --- a/glib/grcbox.h +++ b/glib/grcbox.h @@ -27,62 +27,62 @@ G_BEGIN_DECLS GLIB_AVAILABLE_IN_2_58 -gpointer g_rc_box_alloc (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); +gpointer g_rc_box_alloc (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); GLIB_AVAILABLE_IN_2_58 -gpointer g_rc_box_alloc0 (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); +gpointer g_rc_box_alloc0 (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); GLIB_AVAILABLE_IN_2_58 -gpointer g_rc_box_dup (gsize block_size, - gconstpointer mem_block) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); +gpointer g_rc_box_dup (gsize block_size, + gconstpointer mem_block) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); GLIB_AVAILABLE_IN_2_58 -gpointer g_rc_box_acquire (gpointer mem_block); +gpointer g_rc_box_acquire (gpointer mem_block); GLIB_AVAILABLE_IN_2_58 -void g_rc_box_release (gpointer mem_block); +void g_rc_box_release (gpointer mem_block); GLIB_AVAILABLE_IN_2_58 -void g_rc_box_release_full (gpointer mem_block, - GDestroyNotify clear_func); +void g_rc_box_release_full (gpointer mem_block, + GDestroyNotify clear_func); GLIB_AVAILABLE_IN_2_58 -gsize g_rc_box_get_size (gpointer mem_block); +gsize g_rc_box_get_size (gpointer mem_block); GLIB_AVAILABLE_IN_2_58 -gpointer g_arc_box_alloc (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); +gpointer g_atomic_rc_box_alloc (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); GLIB_AVAILABLE_IN_2_58 -gpointer g_arc_box_alloc0 (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); +gpointer g_atomic_rc_box_alloc0 (gsize block_size) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); GLIB_AVAILABLE_IN_2_58 -gpointer g_arc_box_dup (gsize block_size, - gconstpointer mem_block) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); +gpointer g_atomic_rc_box_dup (gsize block_size, + gconstpointer mem_block) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1); GLIB_AVAILABLE_IN_2_58 -gpointer g_arc_box_acquire (gpointer mem_block); +gpointer g_atomic_rc_box_acquire (gpointer mem_block); GLIB_AVAILABLE_IN_2_58 -void g_arc_box_release (gpointer mem_block); +void g_atomic_rc_box_release (gpointer mem_block); GLIB_AVAILABLE_IN_2_58 -void g_arc_box_release_full (gpointer mem_block, - GDestroyNotify clear_func); +void g_atomic_rc_box_release_full (gpointer mem_block, + GDestroyNotify clear_func); GLIB_AVAILABLE_IN_2_58 -gsize g_arc_box_get_size (gpointer mem_block); +gsize g_atomic_rc_box_get_size (gpointer mem_block); #define g_rc_box_new(type) \ ((type *) g_rc_box_alloc (sizeof (type))) #define g_rc_box_new0(type) \ ((type *) g_rc_box_alloc0 (sizeof (type))) -#define g_arc_box_new(type) \ - ((type *) g_arc_box_alloc (sizeof (type))) -#define g_arc_box_new0(type) \ - ((type *) g_arc_box_alloc0 (sizeof (type))) +#define g_atomic_rc_box_new(type) \ + ((type *) g_atomic_rc_box_alloc (sizeof (type))) +#define g_atomic_rc_box_new0(type) \ + ((type *) g_atomic_rc_box_alloc0 (sizeof (type))) #if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) && !defined(_cplusplus) /* Type check to avoid assigning references to different types */ # define g_rc_box_acquire(mem_block) \ ((__typeof__(mem_block)) (g_rc_box_acquire) (mem_block)) -# define g_arc_box_acquire(mem_block) \ - ((__typeof__(mem_block)) (g_arc_box_acquire) (mem_block)) +# define g_atomic_rc_box_acquire(mem_block) \ + ((__typeof__(mem_block)) (g_atomic_rc_box_acquire) (mem_block)) /* Type check to avoid duplicating data to different types */ # define g_rc_box_dup(block_size,mem_block) \ ((__typeof__(mem_block)) (g_rc_box_dup) (block_size,mem_block)) -# define g_arc_box_dup(block_size,mem_block) \ - ((__typeof__(mem_block)) (g_arc_box_dup) (block_size,mem_block)) +# define g_atomic_rc_box_dup(block_size,mem_block) \ + ((__typeof__(mem_block)) (g_atomic_rc_box_dup) (block_size,mem_block)) #endif G_END_DECLS diff --git a/glib/grefstring.c b/glib/grefstring.c index 9d64a968b..8d46a7f4c 100644 --- a/glib/grefstring.c +++ b/glib/grefstring.c @@ -65,7 +65,7 @@ g_ref_string_new (const char *str) len = strlen (str); - res = (char *) g_arc_box_dup (sizeof (char) * len + 1, str); + res = (char *) g_atomic_rc_box_dup (sizeof (char) * len + 1, str); res[len] = '\0'; return res; @@ -99,21 +99,21 @@ g_ref_string_new_intern (const char *str) if (G_UNLIKELY (interned_ref_strings == NULL)) interned_ref_strings = g_hash_table_new_full (g_str_hash, g_str_equal, - (GDestroyNotify) g_arc_box_release, + (GDestroyNotify) g_atomic_rc_box_release, NULL); res = g_hash_table_lookup (interned_ref_strings, str); if (res != NULL) { G_UNLOCK (interned_ref_strings); - return g_arc_box_acquire (res); + return g_atomic_rc_box_acquire (res); } len = strlen (str); - res = (char *) g_arc_box_dup (sizeof (char) * len + 1, str); + res = (char *) g_atomic_rc_box_dup (sizeof (char) * len + 1, str); res[len] = '\0'; - g_hash_table_add (interned_ref_strings, g_arc_box_acquire (res)); + g_hash_table_add (interned_ref_strings, g_atomic_rc_box_acquire (res)); G_UNLOCK (interned_ref_strings); @@ -135,7 +135,7 @@ g_ref_string_acquire (char *str) { g_return_val_if_fail (str != NULL && *str != '\0', NULL); - return g_arc_box_acquire (str); + return g_atomic_rc_box_acquire (str); } static void @@ -171,7 +171,7 @@ g_ref_string_release (char *str) { g_return_if_fail (str != NULL && *str != '\0'); - g_arc_box_release_full (str, remove_if_interned); + g_atomic_rc_box_release_full (str, remove_if_interned); } /** @@ -189,5 +189,5 @@ g_ref_string_length (char *str) { g_return_val_if_fail (str != NULL && *str != '\0', 0); - return g_arc_box_get_size (str) - 1; + return g_atomic_rc_box_get_size (str) - 1; } From 68304ae583b7f633742420a0fa014accc8eec379 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 3 Jul 2018 15:43:45 +0100 Subject: [PATCH 09/19] Improve docs for g_rc_box/g_atomic_rc_box Especially the preconditions and the annotations for the returned values. --- glib/garcbox.c | 37 ++++++++++++++++--------------------- glib/grcbox.c | 39 +++++++++++++++++---------------------- 2 files changed, 33 insertions(+), 43 deletions(-) diff --git a/glib/garcbox.c b/glib/garcbox.c index bbf16db1b..b423248da 100644 --- a/glib/garcbox.c +++ b/glib/garcbox.c @@ -159,7 +159,7 @@ /** * g_atomic_rc_box_alloc: - * @block_size: the size of the allocation + * @block_size: the size of the allocation, must be greater than 0 * * Allocates @block_size bytes of memory, and adds atomic * reference counting semantics to it. @@ -167,7 +167,7 @@ * The data will be freed when its reference count drops to * zero. * - * Returns: a pointer to the allocated memory + * Returns: (transfer full) (not nullable): a pointer to the allocated memory * * Since: 2.58 */ @@ -181,17 +181,17 @@ g_atomic_rc_box_alloc (gsize block_size) /** * g_atomic_rc_box_alloc0: - * @block_size: the size of the allocation + * @block_size: the size of the allocation, must be greater than 0 * * Allocates @block_size bytes of memory, and adds atomic * referenc counting semantics to it. * - * The contents of the returned data is set to 0's. + * The contents of the returned data is set to zero. * * The data will be freed when its reference count drops to * zero. * - * Returns: a pointer to the allocated memory + * Returns: (transfer full) (not nullable): a pointer to the allocated memory * * Since: 2.58 */ @@ -214,11 +214,8 @@ g_atomic_rc_box_alloc0 (gsize block_size) * casts the returned pointer to a pointer of the given @type, * avoiding a type cast in the source code. * - * This macro cannot return %NULL, as the minimum allocation - * size from `sizeof (@type)` is 1 byte. - * - * Returns: (not nullable): a pointer to the allocated memory, - * cast to a pointer for the given @type + * Returns: (transfer full) (not nullable): a pointer to the allocated + * memory, cast to a pointer for the given @type * * Since: 2.58 */ @@ -229,31 +226,29 @@ g_atomic_rc_box_alloc0 (gsize block_size) * * A convenience macro to allocate atomically reference counted * data with the size of the given @type, and set its contents - * to 0. + * to zero. * * This macro calls g_atomic_rc_box_alloc0() with `sizeof (@type)` and * casts the returned pointer to a pointer of the given @type, * avoiding a type cast in the source code. * - * This macro cannot return %NULL, as the minimum allocation - * size from `sizeof (@type)` is 1 byte. - * - * Returns: (not nullable): a pointer to the allocated memory, - * cast to a pointer for the given @type + * Returns: (transfer full) (not nullable): a pointer to the allocated + * memory, cast to a pointer for the given @type * * Since: 2.58 */ /** * g_atomic_rc_box_dup: - * @block_size: the number of bytes to copy + * @block_size: the number of bytes to copy, must be greater than 0 * @mem_block: (not nullable): the memory to copy * * Allocates a new block of data with atomit reference counting * semantics, and copies @block_size bytes of @mem_block * into it. * - * Returns: (not nullable): a pointer to the allocated memory + * Returns: (transfer full) (not nullable): a pointer to the allocated + * memory * * Since: 2.58 */ @@ -278,8 +273,8 @@ gpointer * * Atomically acquires a reference on the data pointed by @mem_block. * - * Returns: (not nullabl): a pointer to the data, with its reference - * count increased + * Returns: (transfer none) (not nullable): a pointer to the data, + * with its reference count increased * * Since: 2.58 */ @@ -361,7 +356,7 @@ g_atomic_rc_box_release_full (gpointer mem_block, * * Retrieves the size of the reference counted data pointed by @mem_block. * - * Returns: the size of the data + * Returns: the size of the data, in bytes * * Since: 2.58 */ diff --git a/glib/grcbox.c b/glib/grcbox.c index 5cceb87b7..16ff06d2f 100644 --- a/glib/grcbox.c +++ b/glib/grcbox.c @@ -230,7 +230,7 @@ g_rc_box_alloc_full (gsize block_size, /** * g_rc_box_alloc: - * @block_size: the size of the allocation + * @block_size: the size of the allocation, must be greater than 0 * * Allocates @block_size bytes of memory, and adds reference * counting semantics to it. @@ -238,7 +238,7 @@ g_rc_box_alloc_full (gsize block_size, * The data will be freed when its reference count drops to * zero. * - * Returns: a pointer to the allocated memory + * Returns: (transfer full) (not nullable): a pointer to the allocated memory * * Since: 2.58 */ @@ -252,17 +252,17 @@ g_rc_box_alloc (gsize block_size) /** * g_rc_box_alloc0: - * @block_size: the size of the allocation + * @block_size: the size of the allocation, must be greater than 0 * * Allocates @block_size bytes of memory, and adds reference * counting semantics to it. * - * The contents of the returned data is set to 0's. + * The contents of the returned data is set to zero. * * The data will be freed when its reference count drops to * zero. * - * Returns: a pointer to the allocated memory + * Returns: (transfer full) (not nullable): a pointer to the allocated memory * * Since: 2.58 */ @@ -285,11 +285,8 @@ g_rc_box_alloc0 (gsize block_size) * casts the returned pointer to a pointer of the given @type, * avoiding a type cast in the source code. * - * This macro cannot return %NULL, as the minimum allocation - * size from `sizeof (@type)` is 1 byte. - * - * Returns: (not nullable): a pointer to the allocated memory, - * cast to a pointer for the given @type + * Returns: (transfer full) (not nullable): a pointer to the + * allocated memory, cast to a pointer for the given @type * * Since: 2.58 */ @@ -299,31 +296,29 @@ g_rc_box_alloc0 (gsize block_size) * @type: the type to allocate, typically a structure name * * A convenience macro to allocate reference counted data with - * the size of the given @type, and set its contents to 0. + * the size of the given @type, and set its contents to zero. * * This macro calls g_rc_box_alloc0() with `sizeof (@type)` and * casts the returned pointer to a pointer of the given @type, * avoiding a type cast in the source code. * - * This macro cannot return %NULL, as the minimum allocation - * size from `sizeof (@type)` is 1 byte. - * - * Returns: (not nullable): a pointer to the allocated memory, - * cast to a pointer for the given @type + * Returns: (transfer full) (not nullable): a pointer to the + * allocated memory, cast to a pointer for the given @type * * Since: 2.58 */ /** * g_rc_box_dup: - * @block_size: the number of bytes to copy + * @block_size: the number of bytes to copy, must be greater than 0 * @mem_block: (not nullable): the memory to copy * * Allocates a new block of data with reference counting * semantics, and copies @block_size bytes of @mem_block * into it. * - * Returns: (not nullable): a pointer to the allocated memory + * Returns: (transfer full) (not nullable): a pointer to the allocated + * memory * * Since: 2.58 */ @@ -348,8 +343,8 @@ gpointer * * Acquires a reference on the data pointed by @mem_block. * - * Returns: (not nullabl): a pointer to the data, with its reference - * count increased + * Returns: (transfer none) (not nullable): a pointer to the data, + * with its reference count increased * * Since: 2.58 */ @@ -395,7 +390,7 @@ g_rc_box_release (gpointer mem_block) /** * g_rc_box_release_full: - * @mem_block: (not nullabl): a pointer to reference counted data + * @mem_block: (not nullable): a pointer to reference counted data * @clear_func: (not nullable): a function to call when clearing the data * * Releases a reference on the data pointed by @mem_block. @@ -431,7 +426,7 @@ g_rc_box_release_full (gpointer mem_block, * * Retrieves the size of the reference counted data pointed by @mem_block. * - * Returns: the size of the data + * Returns: the size of the data, in bytes * * Since: 2.58 */ From 7c4ac58938187a431db0651e383dda6e098cff07 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 3 Jul 2018 15:46:11 +0100 Subject: [PATCH 10/19] Allow NULL clear function when releasing references Both g_rc_box_release_full() and g_atomic_rc_box_release_full() should allow passing NULL as the clear function, to conform to the existing coding practices in GLib. Additionally, this allows us to reimplement release() in terms of release_full(), and improve test coverage. --- glib/garcbox.c | 14 +++----------- glib/grcbox.c | 14 +++----------- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/glib/garcbox.c b/glib/garcbox.c index b423248da..4182e986e 100644 --- a/glib/garcbox.c +++ b/glib/garcbox.c @@ -307,15 +307,7 @@ gpointer void g_atomic_rc_box_release (gpointer mem_block) { - GArcBox *real_box = G_ARC_BOX (mem_block); - - g_return_if_fail (mem_block != NULL); -#ifndef G_DISABLE_ASSERT - g_return_if_fail (real_box->magic == G_BOX_MAGIC); -#endif - - if (g_atomic_ref_count_dec (&real_box->ref_count)) - g_free (real_box); + g_atomic_rc_box_release_full (mem_block, NULL); } /** @@ -338,14 +330,14 @@ g_atomic_rc_box_release_full (gpointer mem_block, GArcBox *real_box = G_ARC_BOX (mem_block); g_return_if_fail (mem_block != NULL); - g_return_if_fail (clear_func != NULL); #ifndef G_DISABLE_ASSERT g_return_if_fail (real_box->magic == G_BOX_MAGIC); #endif if (g_atomic_ref_count_dec (&real_box->ref_count)) { - clear_func (mem_block); + if (clear_func != NULL) + clear_func (mem_block); g_free (real_box); } } diff --git a/glib/grcbox.c b/glib/grcbox.c index 16ff06d2f..5a4d87424 100644 --- a/glib/grcbox.c +++ b/glib/grcbox.c @@ -377,15 +377,7 @@ gpointer void g_rc_box_release (gpointer mem_block) { - GRcBox *real_box = G_RC_BOX (mem_block); - - g_return_if_fail (mem_block != NULL); -#ifndef G_DISABLE_ASSERT - g_return_if_fail (real_box->magic == G_BOX_MAGIC); -#endif - - if (g_ref_count_dec (&real_box->ref_count)) - g_free (real_box); + g_rc_box_release_full (mem_block, NULL); } /** @@ -408,14 +400,14 @@ g_rc_box_release_full (gpointer mem_block, GRcBox *real_box = G_RC_BOX (mem_block); g_return_if_fail (mem_block != NULL); - g_return_if_fail (clear_func != NULL); #ifndef G_DISABLE_ASSERT g_return_if_fail (real_box->magic == G_BOX_MAGIC); #endif if (g_ref_count_dec (&real_box->ref_count)) { - clear_func (mem_block); + if (clear_func != NULL) + clear_func (mem_block); g_free (real_box); } } From 61ca2e4c8503762006eea18a6316e3b151d0bf92 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 3 Jul 2018 15:55:24 +0100 Subject: [PATCH 11/19] Check for overflow when allocating RcBox Since we're over-allocating the passed block size, we need to check that we're not overflowing gsize when computing the actual allocation size. --- glib/garcbox.c | 3 +-- glib/grcbox.c | 10 +++++++--- glib/grcboxprivate.h | 1 + 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/glib/garcbox.c b/glib/garcbox.c index 4182e986e..25c806ef7 100644 --- a/glib/garcbox.c +++ b/glib/garcbox.c @@ -18,10 +18,9 @@ #include "config.h" -#include "grcbox.h" +#include "grcboxprivate.h" #include "gmessages.h" -#include "grcboxprivate.h" #include "grefcount.h" #ifdef ENABLE_VALGRIND diff --git a/glib/grcbox.c b/glib/grcbox.c index 5a4d87424..0629c1279 100644 --- a/glib/grcbox.c +++ b/glib/grcbox.c @@ -18,11 +18,11 @@ #include "config.h" -#include "grcbox.h" +#include "grcboxprivate.h" #include "gmessages.h" -#include "grcboxprivate.h" #include "grefcount.h" +#include "gtestutils.h" #ifdef ENABLE_VALGRIND #include "valgrind.h" @@ -173,9 +173,12 @@ g_rc_box_alloc_full (gsize block_size, { /* sizeof GArcBox == sizeof GRcBox */ gsize private_size = G_ARC_BOX_SIZE; - gsize real_size = private_size + block_size; + gsize real_size; char *allocated; + g_assert (block_size < (G_MAXSIZE - G_ARC_BOX_SIZE)); + real_size = private_size + block_size; + #ifdef ENABLE_VALGRIND if (RUNNING_ON_VALGRIND) { @@ -185,6 +188,7 @@ g_rc_box_alloc_full (gsize block_size, * Valgrind to keep track of the over-allocation and not be * confused when passing the pointer around */ + g_assert (private_size < (G_MAXSIZE - ALIGN_STRUCT (1))); private_size += ALIGN_STRUCT (1); if (clear) diff --git a/glib/grcboxprivate.h b/glib/grcboxprivate.h index 6599e4d4a..7504d9d95 100644 --- a/glib/grcboxprivate.h +++ b/glib/grcboxprivate.h @@ -1,6 +1,7 @@ #pragma once #include "gtypes.h" +#include "grcbox.h" G_BEGIN_DECLS From 501a8e96e8eeda33b2277f934f29c0dc7af7c538 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 3 Jul 2018 15:57:54 +0100 Subject: [PATCH 12/19] Add missing copyright notice --- glib/grcboxprivate.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/glib/grcboxprivate.h b/glib/grcboxprivate.h index 7504d9d95..8b0d8dd4e 100644 --- a/glib/grcboxprivate.h +++ b/glib/grcboxprivate.h @@ -1,3 +1,21 @@ +/* grcboxprivate.h: Reference counted data + * + * Copyright 2018 Emmanuele Bassi + * + * 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.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 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * 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, see . + */ + #pragma once #include "gtypes.h" From 4248b4b300f54e2c1897d68ff42f97062747c7c4 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 3 Jul 2018 17:21:38 +0100 Subject: [PATCH 13/19] Fix the implementation of interned refstrings The global hash table we use for interned strings should not own a reference on the strings themselves, as otherwise we'd leak them all over the place. Instead, it should keep a "weak" reference to them; once the last strong reference goes away, we drop remove the weak reference from the hash table. --- glib/grefstring.c | 107 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 92 insertions(+), 15 deletions(-) diff --git a/glib/grefstring.c b/glib/grefstring.c index 8d46a7f4c..ea15d544f 100644 --- a/glib/grefstring.c +++ b/glib/grefstring.c @@ -27,6 +27,60 @@ * instead of copying the string among callers; when the last reference on * the string is released, the resources allocated for it are freed. * + * Typically, reference counted strings can be used when parsing data from + * files and storing them into data structures that are passed to various + * callers: + * + * |[ + * PersonDetails * + * person_details_from_data (const char *data) + * { + * // Use g_autoptr() to simplify error cases + * g_autoptr(GRefString) full_name = NULL; + * g_autoptr(GRefString) address = NULL; + * g_autoptr(GRefString) city = NULL; + * g_autoptr(GRefString) state = NULL; + * g_autoptr(GRefString) zip_code = NULL; + * + * // parse_person_details() is defined elsewhere; returns refcounted strings + * if (!parse_person_details (data, &full_name, &address, &city, &state, &zip_code)) + * return NULL; + * + * if (!validate_zip_code (zip_code)) + * return NULL; + * + * // add_address_to_cache() and add_full_name_to_cache() are defined + * // elsewhere; they add strings to various caches, using refcounted + * // strings to avoid copying data over and over again + * add_address_to_cache (address, city, state, zip_code); + * add_full_name_to_cache (full_name); + * + * // person_details_new() is defined elsewhere; it takes a reference + * // on each string + * PersonDetails *res = person_details_new (full_name, + * address, + * city, + * state, + * zip_code); + * + * return res; + * } + * ]| + * + * In the example above, we have multiple functions taking the same strings + * for different uses; with typical C strings, we'd have to copy the strings + * every time the life time rules of the data differ from the life time of + * the string parsed from the original buffer. With reference counted strings, + * each caller can take a reference on the data, and keep it as long as it + * needs to own the string. + * + * Reference counted strings can also be "interned" inside a global table + * owned by GLib; while an interned string has at least a reference, creating + * a new interned reference counted string with the same contents will return + * a reference to the existing string instead of creating a new reference + * counted string instance. Once the string loses its last reference, it will + * be automatically removed from the global interned strings table. + * * Since: 2.58 */ @@ -41,6 +95,11 @@ #include +/* A global table of refcounted strings; the hash table does not own + * the strings, just a pointer to them. Strings are interned as long + * as they are alive; once their reference count drops to zero, they + * are removed from the table + */ G_LOCK_DEFINE_STATIC (interned_ref_strings); static GHashTable *interned_ref_strings; @@ -71,6 +130,26 @@ g_ref_string_new (const char *str) return res; } +/* interned_str_equal: variant of g_str_equal() that compares + * pointers as well as contents; this avoids running strcmp() + * on arbitrarily long strings, as it's more likely to have + * g_ref_string_new_intern() being called on the same refcounted + * string instance, than on a different string with the same + * contents + */ +static gboolean +interned_str_equal (gconstpointer v1, + gconstpointer v2) +{ + const char *str1 = v1; + const char *str2 = v2; + + if (v1 == v2) + return TRUE; + + return strcmp (str1, str2) == 0; +} + /** * g_ref_string_new_intern: * @str: (not nullable): a NUL-terminated string @@ -82,15 +161,14 @@ g_ref_string_new (const char *str) * the same contents of @str, it will return a new reference, instead of * creating a new string. * - * Returns: (not nullable): the newly created reference counted string, or - * a new reference to an existing string + * Returns: (transfer full) (not nullable): the newly created reference + * counted string, or a new reference to an existing string * * Since: 2.58 */ char * g_ref_string_new_intern (const char *str) { - gsize len; char *res; g_return_val_if_fail (str != NULL && *str != '\0', NULL); @@ -98,23 +176,23 @@ g_ref_string_new_intern (const char *str) G_LOCK (interned_ref_strings); if (G_UNLIKELY (interned_ref_strings == NULL)) - interned_ref_strings = g_hash_table_new_full (g_str_hash, g_str_equal, - (GDestroyNotify) g_atomic_rc_box_release, - NULL); + interned_ref_strings = g_hash_table_new (g_str_hash, interned_str_equal); res = g_hash_table_lookup (interned_ref_strings, str); if (res != NULL) { + /* We acquire the reference while holding the lock, to + * avoid a potential race between releasing the lock on + * the hash table and another thread releasing the reference + * on the same string + */ + g_atomic_rc_box_acquire (res); G_UNLOCK (interned_ref_strings); - return g_atomic_rc_box_acquire (res); + return res; } - len = strlen (str); - res = (char *) g_atomic_rc_box_dup (sizeof (char) * len + 1, str); - res[len] = '\0'; - - g_hash_table_add (interned_ref_strings, g_atomic_rc_box_acquire (res)); - + res = g_ref_string_new (str); + g_hash_table_add (interned_ref_strings, res); G_UNLOCK (interned_ref_strings); return res; @@ -147,8 +225,7 @@ remove_if_interned (gpointer data) if (G_LIKELY (interned_ref_strings != NULL)) { - if (g_hash_table_contains (interned_ref_strings, str)) - g_hash_table_remove (interned_ref_strings, str); + g_hash_table_remove (interned_ref_strings, str); if (g_hash_table_size (interned_ref_strings) == 0) g_clear_pointer (&interned_ref_strings, g_hash_table_destroy); From c342105e7648121c604371bcc1edf7e3a65e9ba7 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 3 Jul 2018 17:45:49 +0100 Subject: [PATCH 14/19] Add systemtap probes to refcounted data Probes allow us to debug refcounting bugs. --- glib/garcbox.c | 8 ++++++++ glib/glib.stp.in | 45 +++++++++++++++++++++++++++++++++++++++++++++ glib/glib_probes.d | 4 ++++ glib/grcbox.c | 10 ++++++++++ 4 files changed, 67 insertions(+) diff --git a/glib/garcbox.c b/glib/garcbox.c index 25c806ef7..ddbf04c83 100644 --- a/glib/garcbox.c +++ b/glib/garcbox.c @@ -27,6 +27,8 @@ #include "valgrind.h" #endif +#include "glib_trace.h" + #include #define G_ARC_BOX(p) (GArcBox *) (((char *) (p)) - G_ARC_BOX_SIZE) @@ -289,6 +291,8 @@ gpointer g_atomic_ref_count_inc (&real_box->ref_count); + TRACE (GLIB_RCBOX_ACQUIRE (mem_block, 1)); + return mem_block; } @@ -335,8 +339,12 @@ g_atomic_rc_box_release_full (gpointer mem_block, if (g_atomic_ref_count_dec (&real_box->ref_count)) { + TRACE (GLIB_RCBOX_RELEASE (mem_block, 1)); + if (clear_func != NULL) clear_func (mem_block); + + TRACE (GLIB_RCBOX_FREE (mem_block)); g_free (real_box); } } diff --git a/glib/glib.stp.in b/glib/glib.stp.in index dc80e708d..0577d8ca2 100644 --- a/glib/glib.stp.in +++ b/glib/glib.stp.in @@ -598,3 +598,48 @@ probe glib.thread_spawned = process("@ABS_GLIB_RUNTIME_LIBDIR@/libglib-2.0.so.0. name = user_string($arg3); probestr = sprintf("glib.thread_spawned(%p, %p, %s)", func, data, name); } + +/** + * probe glib.rcbox_alloc - Called when a refcounted block is initially requested + * @mem: Raw memory pointer returned + * @n_bytes: number of bytes + * @atomic: Boolean value, %TRUE if this block is atomically refcounted + * @zeroed: Boolean value, %TRUE if this block was filled with NUL bytes + */ +probe glib.rcbox_alloc = process("@ABS_GLIB_RUNTIME_LIBDIR@/libglib-2.0.so.0.@LT_CURRENT@.@LT_REVISION@").mark("rcbox__alloc") +{ + mem = $arg1; + n_bytes = $arg2; + atomic = $arg3; + zeroed = $arg4; + probestr = sprintf("glib.rcbox_alloc(n_bytes=%d) -> %p", n_bytes, mem); +} + +/** + * probe glib.rcbox_acquire - Called when a refcounted block acquires a ref + */ +probe glib.rcbox_acquire = process("@ABS_GLIB_RUNTIME_LIBDIR@/libglib-2.0.so.0.@LT_CURRENT@.@LT_REVISION@").mark("rcbox__acquire") +{ + mem = $arg1; /* ARG: @mem: Raw memory pointer */ + atomic = $arg2; /* ARG: @atomic: Boolean value, %TRUE if the reference was acquired atomically */ + probestr = sprintf("glib.rcbox_acquire(mem=%p)", mem); +} + +/** + * probe glib.rcbox_release - Called when a refcounted block acquires a ref + */ +probe glib.rcbox_acquire = process("@ABS_GLIB_RUNTIME_LIBDIR@/libglib-2.0.so.0.@LT_CURRENT@.@LT_REVISION@").mark("rcbox__release") +{ + mem = $arg1; /* ARG: @mem: Raw memory pointer */ + atomic = $arg2; /* ARG: @atomic: Boolean value, %TRUE if the reference was released atomically */ + probestr = sprintf("glib.rcbox_release(mem=%p)", mem); +} + +/** + * probe glib.rcbox_free - Called when a refcounted block is freed + */ +probe glib.rcbox_free = process("@ABS_GLIB_RUNTIME_LIBDIR@/libglib-2.0.so.0.@LT_CURRENT@.@LT_REVISION@").mark("rcbox__free") +{ + mem = $arg1; /* ARG: @mem: Raw memory pointer */ + probestr = sprintf("glib.rcbox_free(mem=%p)", mem); +} diff --git a/glib/glib_probes.d b/glib/glib_probes.d index 29f7ff12e..d6b1f8d15 100644 --- a/glib/glib_probes.d +++ b/glib/glib_probes.d @@ -43,4 +43,8 @@ provider glib { probe source__set_name(void*, const char*); probe source__before_free(void*, void*, void*); probe thread__spawned(void*, void*, char*); + probe rcbox__alloc(void*, unsigned int, unsigned int, unsigned int); + probe rcbox__acquire(void*, unsigned int); + probe rcbox__release(void*, unsigned int); + probe rcbox__free(void*); }; diff --git a/glib/grcbox.c b/glib/grcbox.c index 0629c1279..448bc0c6a 100644 --- a/glib/grcbox.c +++ b/glib/grcbox.c @@ -28,6 +28,8 @@ #include "valgrind.h" #endif +#include "glib_trace.h" + #include /** @@ -229,6 +231,8 @@ g_rc_box_alloc_full (gsize block_size, g_ref_count_init (&real_box->ref_count); } + TRACE (GLIB_RCBOX_ALLOC (allocated, block_size, atomic, clear)); + return allocated + private_size; } @@ -364,6 +368,8 @@ gpointer g_ref_count_inc (&real_box->ref_count); + TRACE (GLIB_RCBOX_ACQUIRE (mem_block, 0)); + return mem_block; } @@ -410,8 +416,12 @@ g_rc_box_release_full (gpointer mem_block, if (g_ref_count_dec (&real_box->ref_count)) { + TRACE (GLIB_RCBOX_RELEASE (mem_block, 0)); + if (clear_func != NULL) clear_func (mem_block); + + TRACE (GLIB_RCBOX_FREE (mem_block)); g_free (real_box); } } From 18605db3acb2f6e4b7021ba60a6803c70247cb2a Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 3 Jul 2018 17:49:57 +0100 Subject: [PATCH 15/19] Allow empty strings to be refcounted --- glib/grefstring.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/glib/grefstring.c b/glib/grefstring.c index ea15d544f..454b38dcb 100644 --- a/glib/grefstring.c +++ b/glib/grefstring.c @@ -120,7 +120,7 @@ g_ref_string_new (const char *str) char *res; gsize len; - g_return_val_if_fail (str != NULL && *str != '\0', NULL); + g_return_val_if_fail (str != NULL, NULL); len = strlen (str); @@ -171,7 +171,7 @@ g_ref_string_new_intern (const char *str) { char *res; - g_return_val_if_fail (str != NULL && *str != '\0', NULL); + g_return_val_if_fail (str != NULL, NULL); G_LOCK (interned_ref_strings); @@ -211,7 +211,7 @@ g_ref_string_new_intern (const char *str) char * g_ref_string_acquire (char *str) { - g_return_val_if_fail (str != NULL && *str != '\0', NULL); + g_return_val_if_fail (str != NULL, NULL); return g_atomic_rc_box_acquire (str); } @@ -246,7 +246,7 @@ remove_if_interned (gpointer data) void g_ref_string_release (char *str) { - g_return_if_fail (str != NULL && *str != '\0'); + g_return_if_fail (str != NULL); g_atomic_rc_box_release_full (str, remove_if_interned); } @@ -264,7 +264,7 @@ g_ref_string_release (char *str) gsize g_ref_string_length (char *str) { - g_return_val_if_fail (str != NULL && *str != '\0', 0); + g_return_val_if_fail (str != NULL, 0); return g_atomic_rc_box_get_size (str) - 1; } From cfe962a5dcd47b8e73a8700a0e73f1c41012db15 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 3 Jul 2018 17:50:56 +0100 Subject: [PATCH 16/19] Add missing trasfer annotation for g_ref_string_new() We don't have one, but we ought to. --- glib/grefstring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glib/grefstring.c b/glib/grefstring.c index 454b38dcb..91fe8b5f8 100644 --- a/glib/grefstring.c +++ b/glib/grefstring.c @@ -110,7 +110,7 @@ static GHashTable *interned_ref_strings; * Creates a new reference counted string and copies the contents of @str * into it. * - * Returns: (not nullable): the newly created reference counted string + * Returns: (transfer full) (not nullable): the newly created reference counted string * * Since: 2.58 */ From 37687941ebcf644f11a85c7dd2e5e3ea4af0b1bb Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 3 Jul 2018 18:09:04 +0100 Subject: [PATCH 17/19] Increase coverage for GRefString We still have some holes in the code coverage of the GRefString test suite. --- glib/tests/refstring.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/glib/tests/refstring.c b/glib/tests/refstring.c index 67b3ca302..a4d8f1a8d 100644 --- a/glib/tests/refstring.c +++ b/glib/tests/refstring.c @@ -30,6 +30,9 @@ test_refstring_base (void) g_assert_cmpint (strlen (s), ==, strlen ("hello, world")); g_assert_cmpuint (g_ref_string_length (s), ==, strlen ("hello, world")); + g_assert_true (g_ref_string_acquire (s) == s); + g_ref_string_release (s); + g_ref_string_release (s); } @@ -47,14 +50,23 @@ test_refstring_intern (void) g_test_message ("p = s = '%s' (%p)", p, p); g_assert_true (s == p); + g_test_message ("releasing p[%p] ('%s')", p, p); g_ref_string_release (p); p = g_ref_string_new_intern ("goodbye, world"); g_test_message ("p = '%s' (%p)", p, p); g_assert_false (s == p); + g_test_message ("releasing p[%p] ('%s')", p, p); g_ref_string_release (p); + + g_test_message ("releasing s[%p] ('%s')", s, s); g_ref_string_release (s); + + p = g_ref_string_new_intern ("hello, world"); + g_test_message ("p[%p] ('%s') != s[%p]", p, p, s); + g_assert_false (s == p); + g_ref_string_release (p); } int From 0d00667d01169b766dd0fd2fa6c06631bc88e340 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 3 Jul 2018 18:19:56 +0100 Subject: [PATCH 18/19] Increase coverage of atomic refcounted data We are not testing the API directly, and this leads to holes in the code coverage. --- glib/tests/rcbox.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/glib/tests/rcbox.c b/glib/tests/rcbox.c index 48271a5c8..b1a1342bb 100644 --- a/glib/tests/rcbox.c +++ b/glib/tests/rcbox.c @@ -43,6 +43,25 @@ test_rcbox_new (void) g_rc_box_release (a); } +/* test_atomic_rcbox_new: Test g_atomic_rc_box_new() */ +static void +test_atomic_rcbox_new (void) +{ + Point *a = g_atomic_rc_box_new (Point); + + g_assert_nonnull (a); + g_assert_cmpuint (g_atomic_rc_box_get_size (a), ==, sizeof (Point)); + + g_atomic_rc_box_release (a); + + a = g_atomic_rc_box_new0 (Point); + g_assert_nonnull (a); + g_assert_cmpfloat (a->x, ==, 0.f); + g_assert_cmpfloat (a->y, ==, 0.f); + + g_atomic_rc_box_release (a); +} + static void point_clear (Point *p) { @@ -80,6 +99,30 @@ test_rcbox_release_full (void) g_assert_null (global_point); } +/* test_atomic_rcbox_release_full: Verify that g_atomic_rc_box_release_full() + * calls the clear function only when the last reference is released + */ +static void +test_atomic_rcbox_release_full (void) +{ + Point *p = g_atomic_rc_box_new (Point); + + g_assert_nonnull (p); + global_point = p; + + p->x = 42.0f; + p->y = 47.0f; + + g_assert_true (g_atomic_rc_box_acquire (p) == p); + + g_atomic_rc_box_release_full (p, (GDestroyNotify) point_clear); + g_assert_nonnull (global_point); + g_assert_true (p == global_point); + + g_atomic_rc_box_release_full (p, (GDestroyNotify) point_clear); + g_assert_null (global_point); +} + static Point *global_point_a; static Point *global_point_b; @@ -139,6 +182,44 @@ test_rcbox_dup (void) g_assert_null (global_point_b); } +/* test_atomic_rcbox_dup: Verify that g_atomic_rc_box_dup() copies + * only the data and does not change the reference count of the original + */ +static void +test_atomic_rcbox_dup (void) +{ + Point *a, *b; + + a = g_atomic_rc_box_new (Point); + a->x = 10.f; + a->y = 5.f; + + b = g_atomic_rc_box_dup (sizeof (Point), a); + g_assert_true (a != b); + g_assert_cmpfloat (a->x, ==, b->x); + g_assert_cmpfloat (a->y, ==, b->y); + + global_point_a = a; + global_point_b = b; + + a->x = 1.f; + a->y = 1.f; + g_assert_cmpfloat (a->x, !=, b->x); + g_assert_cmpfloat (a->y, !=, b->y); + + b->x = 5.f; + b->y = 10.f; + g_assert_cmpfloat (a->x, !=, b->x); + g_assert_cmpfloat (a->y, !=, b->y); + + g_atomic_rc_box_release_full (a, (GDestroyNotify) point_clear_dup_a); + g_assert_null (global_point_a); + g_assert_nonnull (global_point_b); + + g_atomic_rc_box_release_full (b, (GDestroyNotify) point_clear_dup_b); + g_assert_null (global_point_b); +} + int main (int argc, char *argv[]) @@ -149,5 +230,9 @@ main (int argc, g_test_add_func ("/rcbox/release-full", test_rcbox_release_full); g_test_add_func ("/rcbox/dup", test_rcbox_dup); + g_test_add_func ("/atomic-rcbox/new", test_atomic_rcbox_new); + g_test_add_func ("/atomic-rcbox/release-full", test_atomic_rcbox_release_full); + g_test_add_func ("/atomic-rcbox/dup", test_atomic_rcbox_dup); + return g_test_run (); } From 822b511cb660feca4bb73598dd49d418d42f97c5 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Mon, 9 Jul 2018 10:07:07 +0100 Subject: [PATCH 19/19] Update rcbox annotations for acquire/release functions The accepted behaviour for reference counting functions can be described as such: - acquire: takes a pointer to a memory area and returns the same pointer with its reference count increased; this means that the returned value's ownership is fully transfered from the callee to the caller - release: takes a pointer to a memory area and drops the reference count; this means that the caller transfers the ownership of the argument to the callee These annotations are mostly meant for documentation purposes: high level language bindings are unlikely to use them, as they have their own reference counting semantics on top of GLib's own, and they should not expose this API to their own consumers. --- glib/garcbox.c | 6 +++--- glib/grcbox.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/glib/garcbox.c b/glib/garcbox.c index ddbf04c83..9c1bd8fe5 100644 --- a/glib/garcbox.c +++ b/glib/garcbox.c @@ -274,7 +274,7 @@ gpointer * * Atomically acquires a reference on the data pointed by @mem_block. * - * Returns: (transfer none) (not nullable): a pointer to the data, + * Returns: (transfer full) (not nullable): a pointer to the data, * with its reference count increased * * Since: 2.58 @@ -298,7 +298,7 @@ gpointer /** * g_atomic_rc_box_release: - * @mem_block: (not nullable): a pointer to reference counted data + * @mem_block: (transfer full) (not nullable): a pointer to reference counted data * * Atomically releases a reference on the data pointed by @mem_block. * @@ -315,7 +315,7 @@ g_atomic_rc_box_release (gpointer mem_block) /** * g_atomic_rc_box_release_full: - * @mem_block: (not nullable): a pointer to reference counted data + * @mem_block: (transfer full) (not nullable): a pointer to reference counted data * @clear_func: (not nullable): a function to call when clearing the data * * Atomically releases a reference on the data pointed by @mem_block. diff --git a/glib/grcbox.c b/glib/grcbox.c index 448bc0c6a..204f4bc9e 100644 --- a/glib/grcbox.c +++ b/glib/grcbox.c @@ -351,7 +351,7 @@ gpointer * * Acquires a reference on the data pointed by @mem_block. * - * Returns: (transfer none) (not nullable): a pointer to the data, + * Returns: (transfer full) (not nullable): a pointer to the data, * with its reference count increased * * Since: 2.58 @@ -375,7 +375,7 @@ gpointer /** * g_rc_box_release: - * @mem_block: (not nullable): a pointer to reference counted data + * @mem_block: (transfer full) (not nullable): a pointer to reference counted data * * Releases a reference on the data pointed by @mem_block. * @@ -392,7 +392,7 @@ g_rc_box_release (gpointer mem_block) /** * g_rc_box_release_full: - * @mem_block: (not nullable): a pointer to reference counted data + * @mem_block: (transfer full) (not nullable): a pointer to reference counted data * @clear_func: (not nullable): a function to call when clearing the data * * Releases a reference on the data pointed by @mem_block.