Linux Perf Users
 help / color / mirror / Atom feed
* [PATCH] perf stat: reject --field-separator and --json-output combination
@ 2026-07-05  0:58 Ivan Lazaric
  2026-07-08  6:21 ` Namhyung Kim
  0 siblings, 1 reply; 6+ messages in thread
From: Ivan Lazaric @ 2026-07-05  0:58 UTC (permalink / raw)
  To: linux-perf-users; +Cc: irogers, 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] perf stat: reject --field-separator and --json-output combination
  2026-07-05  0:58 [PATCH] perf stat: reject --field-separator and --json-output combination Ivan Lazaric
@ 2026-07-08  6:21 ` Namhyung Kim
  2026-07-08 13:14   ` [PATCH v2] " Ivan Lazaric
  0 siblings, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2026-07-08  6:21 UTC (permalink / raw)
  To: Ivan Lazaric; +Cc: linux-perf-users, irogers

Hello,

On Sun, Jul 05, 2026 at 02:58:40AM +0200, Ivan Lazaric wrote:
> 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>

Thanks for the patch, but it doesn't apply cleanly.  Can you please
rebase it on to the latest perf-tools-next branch?

Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] perf stat: reject --field-separator and --json-output combination
  2026-07-08  6:21 ` Namhyung Kim
@ 2026-07-08 13:14   ` Ivan Lazaric
  2026-07-08 13:36     ` sashiko-bot
  2026-07-10  0:41     ` Namhyung Kim
  0 siblings, 2 replies; 6+ messages in thread
From: Ivan Lazaric @ 2026-07-08 13:14 UTC (permalink / raw)
  To: namhyung; +Cc: linux-perf-users, irogers, 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>
---
Changes since v1
 - Rebase on top of perf-tools-next

I was working off of master base branch mistakenly.

Applying the patch `perf build: Fix compiler errors with old capstone` to fix the build
(link: https://lore.kernel.org/linux-perf-users/20260706234836.815254-1-namhyung@kernel.org/ ),
this builds and passes the test shell/stat.sh (currently number 121). I have not tested other tests.

 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 92cdb2df7285..3f685beba384 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2655,6 +2655,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 1e17bee026bd..649de1166fed 100755
--- a/tools/perf/tests/shell/stat.sh
+++ b/tools/perf/tests/shell/stat.sh
@@ -535,6 +535,17 @@ test_stat_delay() {
   echo "stat -D test [Success]"
 }
 
+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
@@ -551,6 +562,7 @@ test_stat_detailed
 test_stat_repeat
 test_stat_pid
 test_stat_delay
+test_csv_json_fail
 
 cleanup
 exit $err
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] perf stat: reject --field-separator and --json-output combination
  2026-07-08 13:14   ` [PATCH v2] " Ivan Lazaric
@ 2026-07-08 13:36     ` sashiko-bot
  2026-07-08 23:43       ` Namhyung Kim
  2026-07-10  0:41     ` Namhyung Kim
  1 sibling, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-07-08 13:36 UTC (permalink / raw)
  To: Ivan Lazaric; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing explicit inclusion of `<stdio.h>` for standard I/O functions and types.
--

--- Patch [1]: [PATCH v2] perf stat: reject --field-separator and --json-output combination ---
commit 14ca4ce3700a93ed48a3336a633292f7e45ae462
Author: Ivan Lazaric <ivan.lazaric1@gmail.com>

perf stat: reject --field-separator and --json-output combination

This patch explicitly rejects using both the --field-separator and
--json-output options in perf stat. It adds a check to output an error
message and exit cleanly, preventing malformed mixed formats.

> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index a04466ea3b0a0..6ec82752d71c6 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -2643,6 +2643,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");
                ^^^^^^^

[Severity: Low]
Could this cause a build failure when compiling perf with musl libc?

While glibc often implicitly includes standard headers, musl has stricter
header separation.

Since this code introduces a new call to fprintf and uses stderr, does
builtin-stat.c need an explicit #include <stdio.h> in its global includes
block to guarantee musl compatibility?

> +		parse_options_usage(stat_usage, stat_options, "x", 1);
> +		parse_options_usage(NULL, stat_options, "j", 1);
> +		goto out;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708131359.71732-3-ivan.lazaric1@gmail.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] perf stat: reject --field-separator and --json-output combination
  2026-07-08 13:36     ` sashiko-bot
@ 2026-07-08 23:43       ` Namhyung Kim
  0 siblings, 0 replies; 6+ messages in thread
From: Namhyung Kim @ 2026-07-08 23:43 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Ivan Lazaric, linux-perf-users

On Wed, Jul 08, 2026 at 01:36:55PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Missing explicit inclusion of `<stdio.h>` for standard I/O functions and types.
> --
> 
> --- Patch [1]: [PATCH v2] perf stat: reject --field-separator and --json-output combination ---
> commit 14ca4ce3700a93ed48a3336a633292f7e45ae462
> Author: Ivan Lazaric <ivan.lazaric1@gmail.com>
> 
> perf stat: reject --field-separator and --json-output combination
> 
> This patch explicitly rejects using both the --field-separator and
> --json-output options in perf stat. It adds a check to output an error
> message and exit cleanly, preventing malformed mixed formats.
> 
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index a04466ea3b0a0..6ec82752d71c6 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -2643,6 +2643,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");
>                 ^^^^^^^
> 
> [Severity: Low]
> Could this cause a build failure when compiling perf with musl libc?
> 
> While glibc often implicitly includes standard headers, musl has stricter
> header separation.
> 
> Since this code introduces a new call to fprintf and uses stderr, does
> builtin-stat.c need an explicit #include <stdio.h> in its global includes
> block to guarantee musl compatibility?

It should be fine as it's used in other places already.

Thanks,
Namhyung

> 
> > +		parse_options_usage(stat_usage, stat_options, "x", 1);
> > +		parse_options_usage(NULL, stat_options, "j", 1);
> > +		goto out;
> > +	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260708131359.71732-3-ivan.lazaric1@gmail.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] perf stat: reject --field-separator and --json-output combination
  2026-07-08 13:14   ` [PATCH v2] " Ivan Lazaric
  2026-07-08 13:36     ` sashiko-bot
@ 2026-07-10  0:41     ` Namhyung Kim
  1 sibling, 0 replies; 6+ messages in thread
From: Namhyung Kim @ 2026-07-10  0:41 UTC (permalink / raw)
  To: Ivan Lazaric; +Cc: linux-perf-users, irogers

On Wed, 08 Jul 2026 15:14:01 +0200, Ivan Lazaric wrote:
> 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?".
> 
> [...]
Applied to perf-tools-next, thanks!

Best regards,
Namhyung



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-10  0:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-05  0:58 [PATCH] perf stat: reject --field-separator and --json-output combination Ivan Lazaric
2026-07-08  6:21 ` Namhyung Kim
2026-07-08 13:14   ` [PATCH v2] " Ivan Lazaric
2026-07-08 13:36     ` sashiko-bot
2026-07-08 23:43       ` Namhyung Kim
2026-07-10  0:41     ` Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox