[PATCH] perf stat: reject --field-separator and --json-output combination

[PATCH] perf stat: reject --field-separator and --json-output combination

[Ivan Lazaric]

Specifying --field-separator option is stating you want CSV output.
Passing both --field-separator and --json-output is then stating
you want output to be in CSV and JSON format at same time.

Currently this combination is not rejected, and the outcome
is a malformed combination of CSV and JSON output.
This is because of inconsistencies in various printing functions,
some of them have if-else chains that start with
"Should I print JSON?", and some start with "Should I print CSV?".

Example of current output:
$ tools/perf/perf stat -x , -j -e cpu-migrations true
{"counter-value" : "0.000000", "unit" : "", "event" : "cpu-migrations", "event-runtime" : 474817, "pcnt-running" : 100.00,,

Instead reject the option combination,
with a helpful error message and non-zero exit code.

Example of new output:
$ tools/perf/perf stat -x , -j true
cannot use both --field-separator and --json-output

 Usage: perf stat [<options>] [<command>]

    -x, --field-separator <separator>
                          print counts with custom separator
    -j, --json-output     print counts in JSON format

Signed-off-by: Ivan Lazaric <ivan.lazaric1@gmail.com>
---
 tools/perf/builtin-stat.c      |  7 +++++++
 tools/perf/tests/shell/stat.sh | 12 ++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 99d7db372b48..5565351db756 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2637,6 +2637,13 @@ int cmd_stat(int argc, const char **argv)
 
 	stat_config.aggr_mode = opt_aggr_mode_to_aggr_mode(&opt_mode);
 
+	if (stat_config.csv_sep && stat_config.json_output) {
+		fprintf(stderr, "cannot use both --field-separator and --json-output\n");
+		parse_options_usage(stat_usage, stat_options, "x", 1);
+		parse_options_usage(NULL, stat_options, "j", 1);
+		goto out;
+	}
+
 	if (stat_config.csv_sep) {
 		stat_config.csv_output = true;
 		if (!strcmp(stat_config.csv_sep, "\\t"))
diff --git a/tools/perf/tests/shell/stat.sh b/tools/perf/tests/shell/stat.sh
index 4edb04039036..c4cd249d8e6a 100755
--- a/tools/perf/tests/shell/stat.sh
+++ b/tools/perf/tests/shell/stat.sh
@@ -483,6 +483,17 @@ test_stat_pid() {
   wait $pid 2>/dev/null || true
 }
 
+test_csv_json_fail() {
+  echo "stat -x <sep> -j test"
+  if perf stat -x , -j true > /dev/null 2>&1
+  then
+    echo "stat -x <sep> -j test [Failed - command should have errored]"
+    err=1
+  else
+    echo "stat -x <sep> -j test [Success]"
+  fi
+}
+
 test_default_stat
 test_null_stat
 test_offline_cpu_stat
@@ -498,6 +509,7 @@ test_stat_no_aggr
 test_stat_detailed
 test_stat_repeat
 test_stat_pid
+test_csv_json_fail
 
 cleanup
 exit $err
-- 
2.43.0