Fix: cli: don't log error when using help command with a plugin name
authorSimon Marchi <simon.marchi@efficios.com>
Thu, 26 Sep 2019 15:28:51 +0000 (11:28 -0400)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Thu, 3 Oct 2019 14:45:11 +0000 (10:45 -0400)
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 <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2101
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
src/cli/babeltrace2-cfg-cli-args.c
tests/Makefile.am
tests/cli/test_help [new file with mode: 0755]

index fa1c1f54ee3ffb908e755ac31c258fce7a6e7dd8..d7d34e46f8516ff9c04420dbefa90a441f95e047 100644 (file)
@@ -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;
index 6142a03b9cca90e95dc7506fc55bfc0ff3dcd80f..92ff89e15287e42abd3f93378c68a88e35686b47 100644 (file)
@@ -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 (executable)
index 0000000..b867618
--- /dev/null
@@ -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"
This page took 0.028855 seconds and 4 git commands to generate.