From 4bf5c85fc4fe021489669155ae5de25e86397575 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 26 Sep 2019 11:28:51 -0400 Subject: [PATCH] Fix: cli: don't log error when using help command with a plugin name When using the help command with a plugin name, it works, but Babeltrace logs an error: 09-26 16:37:03.782 10054 10054 E CLI/CFG-CLI-ARGS plugin_comp_cls_names@babeltrace2-cfg-cli-args.c:189 Missing component class type (`source`, `filter`, or `sink`). This is problematic, because this is a valid use of the command. Logging an error is unnecessarily alarming. The current code takes the argument (in our case, a plugin name such as "ctf") and passes it through plugin_comp_cls_names. The function can't parse the string as a component class name, so logs the error mentioned above. The caller, bt_config_help_from_args, sees that the function has failed, so falls back to assuming the string is a plugin name, which then works correctly. To avoid this, the sophisticated solution I chose was to look for an unescaped dot in the argument. If there's one, we treat it as a component class name, if there isn't, we treat is as a plugin name. Change-Id: I76b791e5d44436fdc5b10a3c10e3579a0783b04d Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/2101 Tested-by: jenkins Reviewed-by: Philippe Proulx --- src/cli/babeltrace2-cfg-cli-args.c | 40 ++++++++--- tests/Makefile.am | 2 + tests/cli/test_help | 109 +++++++++++++++++++++++++++++ 3 files changed, 143 insertions(+), 8 deletions(-) create mode 100755 tests/cli/test_help diff --git a/src/cli/babeltrace2-cfg-cli-args.c b/src/cli/babeltrace2-cfg-cli-args.c index fa1c1f54..d7d34e46 100644 --- a/src/cli/babeltrace2-cfg-cli-args.c +++ b/src/cli/babeltrace2-cfg-cli-args.c @@ -1382,6 +1382,8 @@ struct bt_config *bt_config_help_from_args(int argc, const char *argv[], char *plugin_name = NULL, *comp_cls_name = NULL; struct bt_argpar_parse_ret argpar_parse_ret = { 0 }; struct bt_argpar_item_non_opt *non_opt; + GString *substring = NULL; + size_t end_pos; *retcode = 0; cfg = bt_config_help_create(plugin_paths, default_log_level); @@ -1423,18 +1425,36 @@ struct bt_config *bt_config_help_from_args(int argc, const char *argv[], } non_opt = argpar_parse_ret.items->pdata[0]; - plugin_comp_cls_names(non_opt->arg, NULL, &plugin_name, &comp_cls_name, - &cfg->cmd_data.help.cfg_component->type); - if (plugin_name && comp_cls_name) { - /* Component class help */ + + /* Look for unescaped dots in the argument. */ + substring = bt_common_string_until(non_opt->arg, ".\\", ".", &end_pos); + if (!substring) { + BT_CLI_LOGE_APPEND_CAUSE("Could not consume argument: arg=%s", + non_opt->arg); + goto error; + } + + if (end_pos == strlen(non_opt->arg)) { + /* Didn't find an unescaped dot, treat it as a plugin name. */ + g_string_assign(cfg->cmd_data.help.cfg_component->plugin_name, + non_opt->arg); + } else { + /* + * Found an unescaped dot, treat it as a component class name. + */ + plugin_comp_cls_names(non_opt->arg, NULL, &plugin_name, &comp_cls_name, + &cfg->cmd_data.help.cfg_component->type); + if (!plugin_name || !comp_cls_name) { + BT_CLI_LOGE_APPEND_CAUSE( + "Could not parse argument as a component class name: arg=%s", + non_opt->arg); + goto error; + } + g_string_assign(cfg->cmd_data.help.cfg_component->plugin_name, plugin_name); g_string_assign(cfg->cmd_data.help.cfg_component->comp_cls_name, comp_cls_name); - } else { - /* Fall back to plugin help */ - g_string_assign(cfg->cmd_data.help.cfg_component->plugin_name, - non_opt->arg); } goto end; @@ -1447,6 +1467,10 @@ end: g_free(plugin_name); g_free(comp_cls_name); + if (substring) { + g_string_free(substring, TRUE); + } + bt_argpar_parse_ret_fini(&argpar_parse_ret); return cfg; diff --git a/tests/Makefile.am b/tests/Makefile.am index 6142a03b..92ff89e1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -35,6 +35,7 @@ dist_check_SCRIPTS = \ cli/convert/test_auto_source_discovery_params \ cli/convert/test_auto_source_discovery_log_level \ cli/convert/test_convert_args \ + cli/test_help \ cli/test_intersection \ cli/test_output_ctf_metadata \ cli/test_output_path_ctf_non_lttng_trace \ @@ -59,6 +60,7 @@ TESTS_ARGPAR = \ TESTS_CLI = \ cli/convert/test_convert_args \ + cli/test_help \ cli/test_intersection \ cli/test_output_path_ctf_non_lttng_trace \ cli/test_packet_seq_num \ diff --git a/tests/cli/test_help b/tests/cli/test_help new file mode 100755 index 00000000..b867618e --- /dev/null +++ b/tests/cli/test_help @@ -0,0 +1,109 @@ +#!/bin/bash +# +# Copyright (C) 2019 EfficiOS Inc. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License, version 2 only, as +# published by the Free Software Foundation. +# +# This program 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 General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# this program; if not, write to the Free Software Foundation, Inc., 51 +# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +SH_TAP=1 + +if [ "x${BT_TESTS_SRCDIR:-}" != "x" ]; then + UTILSSH="$BT_TESTS_SRCDIR/utils/utils.sh" +else + UTILSSH="$(dirname "$0")/../utils/utils.sh" +fi + +# shellcheck source=../utils/utils.sh +source "$UTILSSH" + +plan_tests 21 + +stdout=$(mktemp test_help_stdout.XXXXXX) +stderr=$(mktemp test_help_stderr.XXXXXX) + +# Return 0 if file "$1" exists and is empty, non-0 otherwise. + +is_empty() +{ + [[ -f "$1" && ! -s "$1" ]] +} + +# Test with a working plugin name. +bt_cli "${stdout}" "${stderr}" help ctf +ok $? "help ctf plugin exit status" + +grep --silent 'Description: CTF input and output' "${stdout}" +ok $? "help ctf plugin expected output" + +is_empty "${stderr}" +ok $? "help ctf plugin produces no error" + +# Test with a working component class name. +bt_cli "${stdout}" "${stderr}" help src.ctf.fs +ok $? "help src.ctf.fs component class exit status" + +grep --silent 'Description: Read CTF traces from the file system.' "${stdout}" +ok $? "help src.ctf.fs component class expected output" + +is_empty "${stderr}" +ok $? "help src.ctf.fs component class produces no error" + +# Test without parameter. +bt_cli "${stdout}" "${stderr}" help +isnt $? 0 "help without parameter exit status" + +grep --silent "Missing plugin name or component class descriptor." "${stderr}" +ok $? "help without parameter produces expected error" + +is_empty "${stdout}" +ok $? "help without parameter produces no output" + +# Test with too many parameters. +bt_cli "${stdout}" "${stderr}" help ctf fs +isnt $? 0 "help with too many parameters exit status" + +grep --silent "Extraneous command-line argument specified to \`help\` command:" "${stderr}" +ok $? "help with too many parameters produces expected error" + +is_empty "${stdout}" +ok $? "help with too many parameters produces no output" + +# Test with unknown plugin name. +bt_cli "${stdout}" "${stderr}" help zigotos +isnt $? 0 "help with unknown plugin name" + +grep --silent 'Cannot find plugin: plugin-name="zigotos"' "${stderr}" +ok $? "help with unknown plugin name produces expected error" + +is_empty "${stdout}" +ok $? "help with unknown plugin name produces no output" + +# Test with unknown component class name (but known plugin). +bt_cli "${stdout}" "${stderr}" help src.ctf.bob +isnt $? 0 "help with unknown component class name" + +grep --silent 'Cannot find component class: plugin-name="ctf", comp-cls-name="bob", comp-cls-type=0' "${stderr}" +ok $? "help with unknown component class name produces expected error" + +grep --silent 'Description: CTF input and output' "${stdout}" +ok $? "help with unknown component class name prints plugin help" + +# Test with unknown component class plugin +bt_cli "${stdout}" "${stderr}" help src.bob.fs +isnt $? 0 "help with unknown component class plugin" + +grep --silent 'Cannot find plugin: plugin-name="bob"' "${stderr}" +ok $? "help with unknown component class plugin produces expected error" + +is_empty "${stdout}" +ok $? "help with unknown component class plugin produces no output" -- 2.34.1