From: Simon Marchi Date: Mon, 6 Jan 2020 18:20:57 +0000 (-0500) Subject: cli: fix bt_plugin leak when using `-i ctf` X-Git-Tag: v2.0.0~30 X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=13aae0f4f15c5b2e5b02a1392723a66e5895527d cli: fix bt_plugin leak when using `-i ctf` When running: libtool --mode=execute valgrind --leak-check=full ./src/cli/babeltrace2 /home/smarchi/src/babeltrace/tests/data/ctf-traces/succeed/succeed1 -i ctf I get: 2,680 (168 direct, 2,512 indirect) bytes in 1 blocks are definitely lost in loss record 56 of 58 at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x5360B10: g_malloc0 (gmem.c:129) by 0x4E97502: bt_plugin_create_empty (plugin.h:178) by 0x4E9AD72: bt_plugin_so_create_empty (plugin-so.c:1239) by 0x4E9B0D1: bt_plugin_so_create_all_from_sections (plugin-so.c:1342) by 0x4E9BFBF: bt_plugin_so_create_all_from_file (plugin-so.c:1678) by 0x4E93FB4: bt_plugin_find_all_from_file (plugin.c:210) by 0x4E95300: nftw_append_all_from_dir (plugin.c:551) by 0x5956F13: process_entry (ftw.c:464) by 0x59573BA: ftw_dir (ftw.c:543) by 0x5957BEB: ftw_startup (ftw.c:768) by 0x4E95748: bt_plugin_create_append_all_from_dir (plugin.c:641) The call to find_loaded_plugin at babeltrace2-cfg-cli-args.c:4013 returns a new reference to the plugin, for which we don't have a corresponding put_ref. Looking at all the users of find_loaded_plugin, they only really need to borrow the plugin, so rather than add a put_ref, I've made it so find_loaded_plugin returns a borrowed reference instead of a new reference. For clarity, I've renamed that function to borrow_loaded_plugin_by_name. And for consistency, I've renamed borrow_loaded_plugin to borrow_loaded_plugin_by_index. Change-Id: I19234bda6e219efa3e55da760846d138c381fac4 Signed-off-by: Simon Marchi Reported-by: Valgrind Memcheck Reviewed-on: https://review.lttng.org/c/babeltrace/+/2731 Tested-by: jenkins --- diff --git a/src/cli/babeltrace2-cfg-cli-args.c b/src/cli/babeltrace2-cfg-cli-args.c index 381fc484..e6fce2f7 100644 --- a/src/cli/babeltrace2-cfg-cli-args.c +++ b/src/cli/babeltrace2-cfg-cli-args.c @@ -4010,7 +4010,7 @@ struct bt_config *bt_config_convert_from_args(int argc, const char *argv[], if (auto_source_discovery_restrict_plugin_name) { plugin_count = 1; - plugin = find_loaded_plugin(auto_source_discovery_restrict_plugin_name); + plugin = borrow_loaded_plugin_by_name(auto_source_discovery_restrict_plugin_name); plugins = &plugin; } else { plugin_count = get_loaded_plugins_count(); diff --git a/src/cli/babeltrace2-plugins.c b/src/cli/babeltrace2-plugins.c index 8fdfe3e7..223f09ad 100644 --- a/src/cli/babeltrace2-plugins.c +++ b/src/cli/babeltrace2-plugins.c @@ -44,7 +44,7 @@ void fini_loaded_plugins(void) g_ptr_array_free(loaded_plugins, TRUE); } -const bt_plugin *find_loaded_plugin(const char *name) +const bt_plugin *borrow_loaded_plugin_by_name(const char *name) { int i; const bt_plugin *plugin = NULL; @@ -69,7 +69,6 @@ const bt_plugin *find_loaded_plugin(const char *name) BT_LOGI("Cannot find plugin: name=\"%s\"", name); } - bt_plugin_get_ref(plugin); return plugin; } @@ -83,7 +82,7 @@ const bt_plugin **borrow_loaded_plugins(void) return (const bt_plugin **) loaded_plugins->pdata; } -const bt_plugin *borrow_loaded_plugin(size_t index) +const bt_plugin *borrow_loaded_plugin_by_index(size_t index) { BT_ASSERT(index < loaded_plugins->len); return g_ptr_array_index(loaded_plugins, index); @@ -102,7 +101,7 @@ void add_to_loaded_plugins(const bt_plugin_set *plugin_set) const bt_plugin *plugin = bt_plugin_set_borrow_plugin_by_index_const(plugin_set, i); const bt_plugin *loaded_plugin = - find_loaded_plugin(bt_plugin_get_name(plugin)); + borrow_loaded_plugin_by_name(bt_plugin_get_name(plugin)); BT_ASSERT(plugin); @@ -113,7 +112,6 @@ void add_to_loaded_plugins(const bt_plugin_set *plugin_set) bt_plugin_get_name(plugin), bt_plugin_get_path(plugin), bt_plugin_get_path(loaded_plugin)); - bt_plugin_put_ref(loaded_plugin); } else { /* Add to global array. */ BT_LOGD("Adding plugin to loaded plugins: plugin-path=\"%s\"", diff --git a/src/cli/babeltrace2-plugins.h b/src/cli/babeltrace2-plugins.h index 60354d37..2dd55d85 100644 --- a/src/cli/babeltrace2-plugins.h +++ b/src/cli/babeltrace2-plugins.h @@ -33,9 +33,10 @@ BT_HIDDEN void fini_loaded_plugins(void); BT_HIDDEN int require_loaded_plugins(const bt_value *plugin_paths); -BT_HIDDEN const bt_plugin *find_loaded_plugin(const char *name); BT_HIDDEN size_t get_loaded_plugins_count(void); BT_HIDDEN const bt_plugin **borrow_loaded_plugins(void); -BT_HIDDEN const bt_plugin *borrow_loaded_plugin(size_t index); +BT_HIDDEN const bt_plugin *borrow_loaded_plugin_by_index(size_t index); +BT_HIDDEN const bt_plugin *borrow_loaded_plugin_by_name(const char *name); + #endif /* CLI_BABELTRACE_PLUGINS_H */ diff --git a/src/cli/babeltrace2.c b/src/cli/babeltrace2.c index ad8ec4b7..0dfac7a8 100644 --- a/src/cli/babeltrace2.c +++ b/src/cli/babeltrace2.c @@ -146,14 +146,13 @@ const void *find_component_class_from_plugin(const char *plugin_name, BT_LOGI("Finding component class: plugin-name=\"%s\", " "comp-cls-name=\"%s\"", plugin_name, comp_class_name); - plugin = find_loaded_plugin(plugin_name); + plugin = borrow_loaded_plugin_by_name(plugin_name); if (!plugin) { goto end; } comp_class = plugin_borrow_comp_cls_func(plugin, comp_class_name); bt_object_get_ref(comp_class); - BT_PLUGIN_PUT_REF_AND_RESET(plugin); end: if (comp_class) { @@ -726,7 +725,7 @@ enum bt_cmd_status cmd_help(struct bt_config *cfg) const bt_plugin *plugin = NULL; const bt_component_class *needed_comp_cls = NULL; - plugin = find_loaded_plugin(cfg->cmd_data.help.cfg_component->plugin_name->str); + plugin = borrow_loaded_plugin_by_name(cfg->cmd_data.help.cfg_component->plugin_name->str); if (!plugin) { BT_CLI_LOGE_APPEND_CAUSE( "Cannot find plugin: plugin-name=\"%s\"", @@ -780,7 +779,6 @@ error: end: bt_component_class_put_ref(needed_comp_cls); - bt_plugin_put_ref(plugin); return cmd_status; } @@ -854,7 +852,7 @@ enum bt_cmd_status cmd_list_plugins(struct bt_config *cfg) } for (i = 0; i < plugins_count; i++) { - const bt_plugin *plugin = borrow_loaded_plugin(i); + const bt_plugin *plugin = borrow_loaded_plugin_by_index(i); component_classes_count += bt_plugin_get_source_component_class_count(plugin) + @@ -871,7 +869,7 @@ enum bt_cmd_status cmd_list_plugins(struct bt_config *cfg) bt_common_color_reset()); for (i = 0; i < plugins_count; i++) { - const bt_plugin *plugin = borrow_loaded_plugin(i); + const bt_plugin *plugin = borrow_loaded_plugin_by_index(i); printf("\n"); print_plugin_info(plugin);