From b8a068016f0953440df334b147db75e861e7f2ac Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Sat, 15 Aug 2015 11:18:18 -0400 Subject: [PATCH] Adapt plugin system to use unified reference counting MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jérémie Galarneau --- .../plugin/component-class-internal.h | 10 +- .../plugin/component-factory-internal.h | 5 +- include/babeltrace/plugin/component-factory.h | 19 +-- .../babeltrace/plugin/component-internal.h | 9 +- .../plugin/notification/iterator-internal.h | 2 +- .../babeltrace/plugin/notification/iterator.h | 21 --- .../notification/notification-internal.h | 3 +- include/babeltrace/plugin/plugin-internal.h | 11 +- plugins/component-class.c | 38 ++---- plugins/component-factory.c | 69 +++++----- plugins/component.c | 125 ++++++++---------- plugins/iterator.c | 32 +---- plugins/plugin.c | 34 ++--- plugins/source.c | 5 +- 14 files changed, 136 insertions(+), 247 deletions(-) diff --git a/include/babeltrace/plugin/component-class-internal.h b/include/babeltrace/plugin/component-class-internal.h index e516478b..500d7cf3 100644 --- a/include/babeltrace/plugin/component-class-internal.h +++ b/include/babeltrace/plugin/component-class-internal.h @@ -28,12 +28,12 @@ */ #include -#include #include #include +#include struct bt_component_class { - struct bt_ref ref; + struct bt_object base; enum bt_component_type type; GString *name; struct bt_plugin *plugin; @@ -44,10 +44,4 @@ struct bt_component_class *bt_component_class_create( enum bt_component_type type, const char *name, struct bt_plugin *plugin); -BT_HIDDEN -void bt_component_class_get(struct bt_component_class *class); - -BT_HIDDEN -void bt_component_class_put(struct bt_component_class *class); - #endif /* BABELTRACE_PLUGIN_COMPONENT_CLASS_INTERNAL_H */ diff --git a/include/babeltrace/plugin/component-factory-internal.h b/include/babeltrace/plugin/component-factory-internal.h index 257ff0f5..3ee24f85 100644 --- a/include/babeltrace/plugin/component-factory-internal.h +++ b/include/babeltrace/plugin/component-factory-internal.h @@ -28,7 +28,7 @@ */ #include -#include +#include #include #include #include @@ -37,7 +37,8 @@ #include struct bt_component_factory { - /** Array of pointers to struct plugin */ + struct bt_object base; + /** Array of pointers to struct bt_plugin */ GPtrArray *plugins; /** Array of pointers to struct bt_component_class */ GPtrArray *components; diff --git a/include/babeltrace/plugin/component-factory.h b/include/babeltrace/plugin/component-factory.h index 5915a5b1..4513035c 100644 --- a/include/babeltrace/plugin/component-factory.h +++ b/include/babeltrace/plugin/component-factory.h @@ -71,14 +71,12 @@ struct bt_component_factory; * * @returns An instance of component factory */ -extern -struct bt_component_factory *bt_component_factory_create(void); +extern struct bt_component_factory *bt_component_factory_create(void); /** * Get the list of components registered to this factory. */ -extern -struct bt_object *bt_component_factory_get_components( +extern struct bt_object *bt_component_factory_get_components( struct bt_component_factory *factory); /** @@ -91,24 +89,19 @@ struct bt_object *bt_component_factory_get_components( * @param path A path to a file or directory * @returns One of #bt_component_factory_status values */ -extern -enum bt_component_factory_status bt_component_factory_load( +extern enum bt_component_factory_status bt_component_factory_load( struct bt_component_factory *factory, const char *path); -extern -enum bt_component_factory_status +extern enum bt_component_factory_status bt_component_factory_register_source_component_class( struct bt_component_factory *factory, const char *name, bt_component_source_init_cb init); -extern -enum bt_component_factory_status bt_component_factory_register_sink_component_class( +extern enum bt_component_factory_status +bt_component_factory_register_sink_component_class( struct bt_component_factory *factory, const char *name, bt_component_sink_init_cb init); -extern -void bt_component_factory_destroy(struct bt_component_factory *factory); - #ifdef __cplusplus } #endif diff --git a/include/babeltrace/plugin/component-internal.h b/include/babeltrace/plugin/component-internal.h index 7a9361fe..c6443e54 100644 --- a/include/babeltrace/plugin/component-internal.h +++ b/include/babeltrace/plugin/component-internal.h @@ -31,19 +31,20 @@ #include #include #include -#include +#include #include #include struct bt_component { - struct bt_ref ref; + struct bt_object base; struct bt_component_class *class; GString *name; - /** No ownership taken */ + /** No ownership of stream taken */ FILE *error_stream; - /** source, sink or filter destroy */ + /** Source, Sink or Filter destroy */ bt_component_destroy_cb destroy; + /** User-defined data and its destruction callback */ void *user_data; bt_component_destroy_cb user_destroy; }; diff --git a/include/babeltrace/plugin/notification/iterator-internal.h b/include/babeltrace/plugin/notification/iterator-internal.h index e2ef852e..dd6ba450 100644 --- a/include/babeltrace/plugin/notification/iterator-internal.h +++ b/include/babeltrace/plugin/notification/iterator-internal.h @@ -32,7 +32,7 @@ #include struct bt_notification_iterator { - struct bt_ref ref; + struct bt_object base; bt_notification_iterator_get_cb get; bt_notification_iterator_next_cb next; void *user_data; diff --git a/include/babeltrace/plugin/notification/iterator.h b/include/babeltrace/plugin/notification/iterator.h index 480f50c7..c958c582 100644 --- a/include/babeltrace/plugin/notification/iterator.h +++ b/include/babeltrace/plugin/notification/iterator.h @@ -119,27 +119,6 @@ extern enum bt_notification_iterator_status *bt_notification_iterator_seek( struct bt_notification_iterator *iterator, int whence, int64_t time); -/** - * Increments the reference count of \p iterator. - * - * @param iterator Iterator of which to increment the reference count - * - * @see bt_notification_iterator_put() - */ -extern void bt_notification_iterator_get( - struct bt_notification_iterator *iterator); - -/** - * Decrements the reference count of \p iterator, destroying it when this - * count reaches 0. - * - * @param iterator Iterator of which to decrement the reference count - * - * @see bt_notification_iterator_get() - */ -extern void bt_notification_iterator_put( - struct bt_notification_iterator *iterator); - #ifdef __cplusplus } #endif diff --git a/include/babeltrace/plugin/notification/notification-internal.h b/include/babeltrace/plugin/notification/notification-internal.h index 0d909db4..a59d5035 100644 --- a/include/babeltrace/plugin/notification/notification-internal.h +++ b/include/babeltrace/plugin/notification/notification-internal.h @@ -29,6 +29,7 @@ #include #include +#include #include #ifdef __cplusplus @@ -36,7 +37,7 @@ extern "C" { #endif struct bt_notification { - struct bt_ctf_ref ref; + struct bt_object base; enum bt_notification_type type; }; diff --git a/include/babeltrace/plugin/plugin-internal.h b/include/babeltrace/plugin/plugin-internal.h index 96aa692a..3b60ea2f 100644 --- a/include/babeltrace/plugin/plugin-internal.h +++ b/include/babeltrace/plugin/plugin-internal.h @@ -31,6 +31,7 @@ #include #include #include +#include #include /** @@ -41,10 +42,10 @@ * reference to their plugin. * * This ensures that a plugin's library is not closed while it is being used - * even if the bt_component_factory which created its components is destroyed. + * even if the bt_component_factory, which created its components, is destroyed. */ struct bt_plugin { - struct bt_ref ref; + struct bt_object base; const char *name; const char *author; const char *license; @@ -60,10 +61,4 @@ BT_HIDDEN enum bt_component_status bt_plugin_register_component_classes( struct bt_plugin *plugin, struct bt_component_factory *factory); -BT_HIDDEN -void bt_plugin_get(struct bt_plugin *plugin); - -BT_HIDDEN -void bt_plugin_put(struct bt_plugin *plugin); - #endif /* BABELTRACE_PLUGIN_COMPONENT_CLASS_INTERNAL_H */ diff --git a/plugins/component-class.c b/plugins/component-class.c index 191efdcb..c13358f8 100644 --- a/plugins/component-class.c +++ b/plugins/component-class.c @@ -28,19 +28,21 @@ #include #include +#include #include static -void bt_component_class_destroy(struct bt_ref *ref) +void bt_component_class_destroy(struct bt_object *obj) { struct bt_component_class *class; - assert(ref); - class = container_of(ref, struct bt_component_class, ref); + assert(obj); + class = container_of(obj, struct bt_component_class, base); if (class->name) { g_string_free(class->name, TRUE); } - bt_plugin_put(class->plugin); + + bt_put(class->plugin); g_free(class); } @@ -56,36 +58,16 @@ struct bt_component_class *bt_component_class_create( goto end; } - bt_ref_init(&class->ref, bt_component_class_destroy); + bt_object_init(class, bt_component_class_destroy); class->type = type; class->name = g_string_new(name); if (!class->name) { - bt_component_class_put(class); - class = NULL; + BT_PUT(class); goto end; } - bt_plugin_get(plugin); + + bt_get(plugin); class->plugin = plugin; end: return class; } - -BT_HIDDEN -void bt_component_class_get(struct bt_component_class *class) -{ - if (!class) { - return; - } - - bt_ref_get(&class->ref); -} - -BT_HIDDEN -void bt_component_class_put(struct bt_component_class *class) -{ - if (!class) { - return; - } - - bt_ref_put(&class->ref); -} diff --git a/plugins/component-factory.c b/plugins/component-factory.c index a91b63a0..03e01485 100644 --- a/plugins/component-factory.c +++ b/plugins/component-factory.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -87,11 +88,11 @@ bt_component_factory_load_file(struct bt_component_factory *factory, * or .la on Linux). */ if (strncmp(NATIVE_PLUGIN_SUFFIX, - path + path_len - NATIVE_PLUGIN_SUFFIX_LEN, - NATIVE_PLUGIN_SUFFIX_LEN) && - strncmp(LIBTOOL_PLUGIN_SUFFIX, - path + path_len - LIBTOOL_PLUGIN_SUFFIX_LEN, - LIBTOOL_PLUGIN_SUFFIX_LEN)) { + path + path_len - NATIVE_PLUGIN_SUFFIX_LEN, + NATIVE_PLUGIN_SUFFIX_LEN) && + strncmp(LIBTOOL_PLUGIN_SUFFIX, + path + path_len - LIBTOOL_PLUGIN_SUFFIX_LEN, + LIBTOOL_PLUGIN_SUFFIX_LEN)) { /* Name indicates that this is not a plugin file. */ ret = BT_COMPONENT_FACTORY_STATUS_INVAL; goto end; @@ -116,7 +117,7 @@ bt_component_factory_load_file(struct bt_component_factory *factory, } component_status = bt_plugin_register_component_classes(plugin, - factory); + factory); if (component_status != BT_COMPONENT_STATUS_OK) { switch (component_status) { case BT_COMPONENT_STATUS_NOMEM: @@ -126,8 +127,8 @@ bt_component_factory_load_file(struct bt_component_factory *factory, ret = BT_COMPONENT_FACTORY_STATUS_ERROR; break; } - bt_plugin_put(plugin); - plugin = NULL; + + BT_PUT(plugin); goto end; } g_ptr_array_add(factory->plugins, plugin); @@ -191,7 +192,7 @@ bt_component_factory_load_dir_recursive(struct bt_component_factory *factory, if (S_ISDIR(st.st_mode)) { ret = bt_component_factory_load_dir_recursive(factory, - file_path); + file_path); if (ret != BT_COMPONENT_FACTORY_STATUS_OK) { goto end; } @@ -205,11 +206,13 @@ end: return ret; } -void bt_component_factory_destroy(struct bt_component_factory *factory) +static +void bt_component_factory_destroy(struct bt_object *obj) { - if (!factory) { - return; - } + struct bt_component_factory *factory = NULL; + + assert(obj); + factory = container_of(obj, struct bt_component_factory, base); if (factory->plugins) { g_ptr_array_free(factory->plugins, TRUE); @@ -220,8 +223,7 @@ void bt_component_factory_destroy(struct bt_component_factory *factory) g_free(factory); } -struct bt_component_factory * -bt_component_factory_create(void) +struct bt_component_factory *bt_component_factory_create(void) { struct bt_component_factory *factory; @@ -230,57 +232,48 @@ bt_component_factory_create(void) goto end; } + bt_object_init(factory, bt_component_factory_destroy); factory->plugins = g_ptr_array_new_with_free_func( - (GDestroyNotify) bt_plugin_put); + (GDestroyNotify) bt_put); if (!factory->plugins) { goto error; } factory->components = g_ptr_array_new_with_free_func( - (GDestroyNotify) bt_component_put); + (GDestroyNotify) bt_put); if (!factory->components) { goto error; } end: return factory; error: - bt_component_factory_destroy(factory); - return NULL; + BT_PUT(factory); + return factory; } enum bt_component_factory_status bt_component_factory_load( struct bt_component_factory *factory, const char *path) { enum bt_component_factory_status ret = BT_COMPONENT_FACTORY_STATUS_OK; - DIR *directory = NULL; if (!factory || !path) { ret = BT_COMPONENT_FACTORY_STATUS_INVAL; goto end; } - directory = opendir(path) ; - if (!directory) { - switch (errno) { - case ENOTDIR: - /* Try loading as a file. */ - break; - case ENOENT: - ret = BT_COMPONENT_FACTORY_STATUS_NOENT; - goto end; - default: - ret = BT_COMPONENT_FACTORY_STATUS_IO; - goto end; - } + if (!g_file_test(path, G_FILE_TEST_EXISTS)) { + ret = BT_COMPONENT_FACTORY_STATUS_NOENT; + goto end; } - if (directory) { + if (g_file_test(path, G_FILE_TEST_IS_DIR)) { ret = bt_component_factory_load_dir_recursive(factory, path); - } else { + } else if (g_file_test(path, G_FILE_TEST_IS_REGULAR) || + g_file_test(path, G_FILE_TEST_IS_SYMLINK)) { ret = bt_component_factory_load_file(factory, path); + } else { + ret = BT_COMPONENT_FACTORY_STATUS_INVAL; + goto end; } end: - if (directory) { - closedir(directory); - } return ret; } diff --git a/plugins/component.c b/plugins/component.c index 86e86ade..c931ddf6 100644 --- a/plugins/component.c +++ b/plugins/component.c @@ -30,8 +30,63 @@ #include #include #include +#include -static void bt_component_destroy(struct bt_ref *ref); +static +void bt_component_destroy(struct bt_object *obj) +{ + struct bt_component *component = NULL; + struct bt_component_class *component_class = NULL; + + if (!obj) { + return; + } + + component = container_of(obj, struct bt_component, base); + + /** + * User data is destroyed first, followed by the concrete component + * instance. + */ + assert(!component->user_data || component->user_destroy); + component->user_destroy(component->user_data); + + g_string_free(component->name, TRUE); + + assert(component->destroy); + component_class = component->class; + + /* Frees the component, which becomes invalid */ + component->destroy(component); + component = NULL; + + bt_put(component_class); +} + +BT_HIDDEN +enum bt_component_status bt_component_init(struct bt_component *component, + struct bt_component_class *class, const char *name, + bt_component_destroy_cb destroy) +{ + enum bt_component_status ret = BT_COMPONENT_STATUS_OK; + + if (!component || !class || !name || name[0] == '\0' || !destroy) { + ret = BT_COMPONENT_STATUS_INVAL; + goto end; + } + + bt_object_init(component, bt_component_destroy); + bt_get(class); + component->class = class; + component->name = g_string_new(name); + if (!component->name) { + ret = BT_COMPONENT_STATUS_NOMEM; + goto end; + } + component->destroy = destroy; +end: + return ret; +} const char *bt_component_get_name(struct bt_component *component) { @@ -89,49 +144,6 @@ end: return ret; } -void bt_component_get(struct bt_component *component) -{ - if (!component) { - return; - } - - bt_ref_get(&component->ref); -} - -void bt_component_put(struct bt_component *component) -{ - if (!component) { - return; - } - - bt_ref_put(&component->ref); -} - -BT_HIDDEN -enum bt_component_status bt_component_init(struct bt_component *component, - struct bt_component_class *class, const char *name, - bt_component_destroy_cb destroy) -{ - enum bt_component_status ret = BT_COMPONENT_STATUS_OK; - - if (!component || !class || !name || name[0] == '\0' || destroy) { - ret = BT_COMPONENT_STATUS_INVAL; - goto end; - } - - bt_ref_init(&component->ref, bt_component_destroy); - bt_component_class_get(class); - component->class = class; - component->name = g_string_new(name); - if (!component->name) { - ret = BT_COMPONENT_STATUS_NOMEM; - goto end; - } - component->destroy = destroy; -end: - return ret; -} - void *bt_component_get_private_data(struct bt_component *component) { void *ret = NULL; @@ -160,28 +172,3 @@ bt_component_set_private_data(struct bt_component *component, end: return ret; } - -static -void bt_component_destroy(struct bt_ref *ref) -{ - struct bt_component *component = NULL; - - if (!ref) { - return; - } - - component = container_of(ref, struct bt_component, ref); - - /** - * User data is destroyed first, followed by the concrete component - * instance. - */ - assert(!component->user_data || component->user_destroy); - component->user_destroy(component->user_data); - - g_string_free(component->name, TRUE); - - assert(component->destroy); - component->destroy(component); - bt_component_class_put(component->class); -} diff --git a/plugins/iterator.c b/plugins/iterator.c index cfa143a3..4cfd1e49 100644 --- a/plugins/iterator.c +++ b/plugins/iterator.c @@ -27,22 +27,20 @@ */ #include +#include #include #include #include #include static -void bt_notification_iterator_destroy(struct bt_ref *ref) +void bt_notification_iterator_destroy(struct bt_object *obj) { struct bt_notification_iterator *iterator; - if (!ref) { - return; - } - - iterator = container_of(ref, struct bt_notification_iterator, - ref); + assert(obj); + iterator = container_of(obj, struct bt_notification_iterator, + base); assert(iterator->user_destroy || !iterator->user_data); iterator->user_destroy(iterator); g_free(iterator); @@ -64,7 +62,7 @@ struct bt_notification_iterator *bt_notification_iterator_create( goto end; } - bt_ref_init(&iterator->ref, bt_notification_iterator_destroy); + bt_object_init(iterator, bt_notification_iterator_destroy); end: return iterator; } @@ -84,24 +82,6 @@ end: return ret; } -void bt_notification_iterator_get(struct bt_notification_iterator *iterator) -{ - if (!iterator) { - return; - } - - bt_ref_get(&iterator->ref); -} - -void bt_notification_iterator_put(struct bt_notification_iterator *iterator) -{ - if (!iterator) { - return; - } - - bt_ref_put(&iterator->ref); -} - enum bt_notification_iterator_status bt_notification_iterator_set_get_cb( struct bt_notification_iterator *iterator, bt_notification_iterator_get_cb get) diff --git a/plugins/plugin.c b/plugins/plugin.c index 35056777..1a2eb115 100644 --- a/plugins/plugin.c +++ b/plugins/plugin.c @@ -27,6 +27,7 @@ */ #include +#include #include #include @@ -37,12 +38,13 @@ #define PLUGIN_SYMBOL_EXIT "__bt_plugin_exit" static -void bt_plugin_destroy(struct bt_ref *ref) +void bt_plugin_destroy(struct bt_object *obj) { struct bt_plugin *plugin; - assert(ref); - plugin = container_of(ref, struct bt_plugin, ref); + assert(obj); + plugin = container_of(obj, struct bt_plugin, base); + if (plugin->module) { if (!g_module_close(plugin->module)) { printf_error("Module close error: %s", @@ -67,7 +69,7 @@ struct bt_plugin *bt_plugin_create(GModule *module) goto error; } - bt_ref_init(&plugin->ref, bt_plugin_destroy); + bt_object_init(plugin, bt_plugin_destroy); if (!g_module_symbol(module, PLUGIN_SYMBOL_NAME, (gpointer *) &plugin->name)) { @@ -97,8 +99,8 @@ struct bt_plugin *bt_plugin_create(GModule *module) return plugin; error: - bt_plugin_put(plugin); - return NULL; + BT_PUT(plugin); + return plugin; } BT_HIDDEN @@ -108,23 +110,3 @@ enum bt_component_status bt_plugin_register_component_classes( assert(plugin && factory); return plugin->init(factory); } - -BT_HIDDEN -void bt_plugin_get(struct bt_plugin *plugin) -{ - if (!plugin) { - return; - } - - bt_ref_get(&plugin->ref); -} - -BT_HIDDEN -void bt_plugin_put(struct bt_plugin *plugin) -{ - if (!plugin) { - return; - } - - bt_ref_put(&plugin->ref); -} diff --git a/plugins/source.c b/plugins/source.c index 38fc3110..2dba33bd 100644 --- a/plugins/source.c +++ b/plugins/source.c @@ -26,6 +26,7 @@ * SOFTWARE. */ +#include #include #include #include @@ -102,6 +103,6 @@ struct bt_notification_iterator *bt_plugin_source_create_iterator( end: return iterator; error: - bt_notification_iterator_put(iterator); - return NULL; + BT_PUT(iterator); + return iterator; } -- 2.34.1